Skip to content

rustdoc JSON: Don't apply #[repr] privacy heuristics #141126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 23, 2025
Merged

rustdoc JSON: Don't apply #[repr] privacy heuristics #141126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 7 additions & 15 deletions src/librustdoc/clean/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -774,20 +774,11 @@ impl Item {
.filter_map(|attr| {
if is_json {
match attr {
hir::Attribute::Parsed(AttributeKind::Deprecation { .. }) => {
// rustdoc-json stores this in `Item::deprecation`, so we
// don't want it it `Item::attrs`.
None
}
rustc_hir::Attribute::Parsed(
rustc_attr_data_structures::AttributeKind::Repr(..),
) => {
// We have separate pretty-printing logic for `#[repr(..)]` attributes.
// For example, there are circumstances where `#[repr(transparent)]`
// is applied but should not be publicly shown in rustdoc
// because it isn't public API.
None
}
// rustdoc-json stores this in `Item::deprecation`, so we
// don't want it it `Item::attrs`.
hir::Attribute::Parsed(AttributeKind::Deprecation { .. }) => None,
// We have separate pretty-printing logic for `#[repr(..)]` attributes.
hir::Attribute::Parsed(AttributeKind::Repr(..)) => None,
_ => Some({
let mut s = rustc_hir_pretty::attribute_to_string(&tcx, attr);
assert_eq!(s.pop(), Some('\n'));
Expand Down Expand Up @@ -820,7 +811,8 @@ impl Item {
if repr.transparent() {
// Render `repr(transparent)` iff the non-1-ZST field is public or at least one
// field is public in case all fields are 1-ZST fields.
let render_transparent = cache.document_private
let render_transparent = is_json
|| cache.document_private
Comment on lines +814 to +815
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

If we felt like making this part of the cargo-semver-checks experience super nice, including the layout info of fields would be lovely:

  • It would let us have ABI-related lints like "this pub repr(C) type had field X change offset / alignment / size." These would be part of our ABI lint group, which isn't quite SemVer but people e.g. working on embedded systems have found useful nonetheless.
  • It would allow us to check the 1-ZST portion of the "is repr(transparent) public API" logic, which we can otherwise only approximate as "some field is pub so we'll assume it's the non-1-ZST one." That heuristic is generally fine in practice AFAICT, but doing better is always good :)

None of this is an emergency of course. Just wanted to give you info on what else in this general area would be useful info to have.

Copy link
Member

@aDotInTheVoid aDotInTheVoid May 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you put this in an issue (or on zulip)? I think it’ll be forgotten here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave a reminder for myself when I get back home. I'll likely be ~offline until Thursday or so due to travel, so that's the earliest I'd be able to open one — it's 11pm here at the moment and I'm going to bed now.

If someone else were to open an issue for this in the meantime, I wouldn't be upset at all. No pressure of course, just proactively opting in.

|| adt
.all_fields()
.find(|field| {
Expand Down
2 changes: 1 addition & 1 deletion src/rustdoc-json-types/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub type FxHashMap<K, V> = HashMap<K, V>; // re-export for use in src/librustdoc
/// This integer is incremented with every breaking change to the API,
/// and is returned along with the JSON blob as [`Crate::format_version`].
/// Consuming code should assert that this value matches the format version(s) that it supports.
pub const FORMAT_VERSION: u32 = 45;
pub const FORMAT_VERSION: u32 = 46;

/// The root of the emitted JSON blob.
///
Expand Down
4 changes: 4 additions & 0 deletions tests/rustdoc-json/attrs/repr_combination.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,7 @@ pub enum AlignedExplicitRepr {
pub enum ReorderedAlignedExplicitRepr {
First,
}

//@ is "$.index[?(@.name=='Transparent')].attrs" '["#[repr(transparent)]"]'
#[repr(transparent)]
pub struct Transparent(i64);
37 changes: 0 additions & 37 deletions tests/rustdoc-json/attrs/repr_transparent.rs
View file
Open in desktop

This file was deleted.

Loading