Skip to content
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.

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

rustdoc: hide #[repr(transparent)] if it isn't part of the public ABI #115439

Merged

Conversation

fmease
Copy link
Member

@fmease fmease commented Sep 1, 2023

Fixes #90435.

This hides #[repr(transparent)] when the non-1-ZST field the struct is "transparent" over is private.

CC @RalfJung

Tentatively nominating it for the release notes, feel free to remove the nomination.
@rustbot label needs-fcp relnotes A-rustdoc-ui

@rustbot
Copy link
Collaborator

rustbot commented Sep 1, 2023

r? @oli-obk

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 1, 2023
@rustbot
Copy link
Collaborator

rustbot commented Sep 1, 2023

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

@fmease
Copy link
Member Author

fmease commented Sep 1, 2023

r? rustdoc

@@ -110,3 +110,23 @@ https://doc.rust-lang.org/stable/std/?search=%s&go_to_first=true

This URL adds the `go_to_first=true` query parameter which can be appended to any `rustdoc` search URL
to automatically go to the first result.

## `#[repr(transparent)]`: Documenting the transparent representation
Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't sure where to place this section, open to suggestions

Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably the best place to put it right now.

@@ -770,10 +791,9 @@ impl Item {
};
out.push(&int_s);
}
if out.is_empty() {
return Vec::new();
Copy link
Member Author

Choose a reason for hiding this comment

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

The early return wasn't correct. It resulted in us not rendering some attributes like #[non_exhaustive] in cross-crate re-export scenarios. I've added a regression test for this: tests/rustdoc/inline_cross/attributes.rs.

Comment on lines 116 to 128
You can read more about `#[repr(transparent)]` itself in the [Rust Reference][repr-trans-ref] and
in the [Rustonomicon][repr-trans-nomicon].

Since this representation is only considered part of the public ABI if the single field with non-trivial
size or alignment is public and if the documentation does not say otherwise, Rustdoc displays the
attribute if and only if the non-1-ZST field is public or at least one field is public in case all
fields are 1-ZST fields. The term *1-ZST* refers to types that are one-aligned and zero-sized.

If you would like to declare the representation as private even if the non-1-ZST field is public,
document it in prose. It would seem that you can manually hide the attribute with
`#[cfg_attr(not(doc), repr(transparent))]`. However, due to [current limitations][cross-crate-cfg-doc],
this does not work for types re-exported from another crate. Therefore, you should always additionally
explicitly write down the layout guarantees in such a case.
Copy link
Member Author

@fmease fmease Sep 1, 2023

Choose a reason for hiding this comment

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

Update: I've slightly tweaked this section, please see the latest revision.
I'm pretty sure that the phrasing in this section can be improved upon, it feels a bit awkward in some places.

Edit: And it's longer than one sentence, notriddle :P

@@ -41,7 +42,7 @@ impl JsonRenderer<'_> {
})
.collect();
let docs = item.opt_doc_value();
let attrs = item.attributes(self.tcx, true);
let attrs = item.attributes(self.tcx, self.cache(), true);
Copy link
Member Author

@fmease fmease Sep 1, 2023

Choose a reason for hiding this comment

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

Note that my patch doesn't affect Rustdoc JSON. We still pass through every inert attribute. Let me know if I that needs changing.

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member

Changes look good to me overall (didn't go into details yet). It'll need to go through FCP and to have someone from the compiler team to take a look as well. But otherwise, nice work! 👍

@fmease fmease force-pushed the rustdoc-priv-repr-transparent-heuristic branch 3 times, most recently from a32b08d to a7c9170 Compare September 2, 2023 13:48
@rustbot rustbot added A-rustdoc-ui Area: rustdoc UI (generated HTML) needs-fcp This change is insta-stable, so needs a completed FCP to proceed. relnotes Marks issues that should be documented in the release notes of the next release. labels Sep 2, 2023
@fmease fmease removed the A-rustdoc-json Area: Rustdoc JSON backend label Sep 6, 2023
@bors
Copy link
Contributor

bors commented Sep 7, 2023

☔ The latest upstream changes (presumably #114855) made this pull request unmergeable. Please resolve the merge conflicts.

@fmease fmease force-pushed the rustdoc-priv-repr-transparent-heuristic branch from a7c9170 to f797064 Compare September 8, 2023 17:28
@rustbot rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Sep 8, 2023
@fmease fmease added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed A-rustdoc-json Area: Rustdoc JSON backend labels Sep 8, 2023
@notriddle
Copy link
Contributor

Good call. The FCP is for the change in behavior mentioned in the book docs.

Since this representation is only considered part of the public ABI if the single field with non-trivial size or alignment is public and if the documentation does not state otherwise, Rustdoc helpfully displays the attribute if and only if the non-1-ZST field is public or at least one field is public in case all fields are 1-ZST fields. The term 1-ZST refers to types that are one-aligned and zero-sized.

It would seem that one can manually hide the attribute with #[cfg_attr(not(doc), repr(transparent))] if one wishes to declare the representation as private even if the non-1-ZST field is public. However, due to #114952, this method is not always guaranteed to work. Therefore, if you would like to do so, you should always write it down in prose independently of whether you use cfg_attr or not.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Oct 2, 2023
@rfcbot
Copy link

rfcbot commented Oct 2, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Oct 12, 2023
@rfcbot
Copy link

rfcbot commented Oct 12, 2023

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@fmease fmease removed the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Oct 13, 2023
@GuillaumeGomez
Copy link
Member

Thanks everyone!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 14, 2023

📌 Commit 64fa12a has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 14, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 14, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#115439 (rustdoc: hide `#[repr(transparent)]` if it isn't part of the public ABI)
 - rust-lang#116591 (Don't accidentally detect the commit hash as an `fadd` instruction)
 - rust-lang#116603 (Reorganize `bootstrap/Cargo.toml`)
 - rust-lang#116715 (Prevent more spurious unreachable pattern lints)
 - rust-lang#116723 (Fix broken build on ESP-IDF caused by rust-lang#115108)
 - rust-lang#116730 (Add some unsoundness tests for opaques capturing hidden regions not in substs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4dd4d9b into rust-lang:master Oct 14, 2023
11 checks passed
@rustbot rustbot added this to the 1.75.0 milestone Oct 14, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 14, 2023
Rollup merge of rust-lang#115439 - fmease:rustdoc-priv-repr-transparent-heuristic, r=GuillaumeGomez

rustdoc: hide `#[repr(transparent)]` if it isn't part of the public ABI

Fixes rust-lang#90435.

This hides `#[repr(transparent)]` when the non-1-ZST field the struct is "transparent" over is private.

CC `@RalfJung`

Tentatively nominating it for the release notes, feel free to remove the nomination.
`@rustbot` label needs-fcp relnotes A-rustdoc-ui
@fmease fmease deleted the rustdoc-priv-repr-transparent-heuristic branch October 14, 2023 20:37
@dtolnay
Copy link
Member

dtolnay commented Oct 14, 2023

Thank you!

I created #116743 to follow up by deleting cfg_attr(not(doc), repr(...))-based workarounds from the standard library.

bors-ferrocene bot added a commit to ferrocene/ferrocene that referenced this pull request Oct 17, 2023
54: Pull upstream master 2023 10 17 r=pietroalbini a=Veykril

* rust-lang/rust#116196
* rust-lang/rust#116824
* rust-lang/rust#116822
* rust-lang/rust#116477
* rust-lang/rust#116826
* rust-lang/rust#116820
  * rust-lang/rust#116811
  * rust-lang/rust#116808
  * rust-lang/rust#116805
  * rust-lang/rust#116800
  * rust-lang/rust#116798
  * rust-lang/rust#116754
* rust-lang/rust#114370
* rust-lang/rust#116804
  * rust-lang/rust#116802
  * rust-lang/rust#116790
  * rust-lang/rust#116786
  * rust-lang/rust#116709
  * rust-lang/rust#116430
  * rust-lang/rust#116257
  * rust-lang/rust#114157
* rust-lang/rust#116731
* rust-lang/rust#116550
* rust-lang/rust#114330
* rust-lang/rust#116724
* rust-lang/rust#116782
  * rust-lang/rust#116776
  * rust-lang/rust#115955
  * rust-lang/rust#115196
* rust-lang/rust#116775
* rust-lang/rust#114589
* rust-lang/rust#113747
* rust-lang/rust#116772
  * rust-lang/rust#116771
  * rust-lang/rust#116760
  * rust-lang/rust#116755
  * rust-lang/rust#116732
  * rust-lang/rust#116522
  * rust-lang/rust#116341
  * rust-lang/rust#116172
* rust-lang/rust#110604
* rust-lang/rust#110729
* rust-lang/rust#116527
* rust-lang/rust#116688
* rust-lang/rust#116757
  * rust-lang/rust#116753
  * rust-lang/rust#116748
  * rust-lang/rust#116741
  * rust-lang/rust#116594
* rust-lang/rust#116691
* rust-lang/rust#116643
* rust-lang/rust#116683
* rust-lang/rust#116635
* rust-lang/rust#115515
* rust-lang/rust#116742
  * rust-lang/rust#116661
  * rust-lang/rust#116576
  * rust-lang/rust#116540
* rust-lang/rust#116352
* rust-lang/rust#116737
  * rust-lang/rust#116730
  * rust-lang/rust#116723
  * rust-lang/rust#116715
  * rust-lang/rust#116603
  * rust-lang/rust#116591
  * rust-lang/rust#115439
* rust-lang/rust#116264
* rust-lang/rust#116727
  * rust-lang/rust#116704
  * rust-lang/rust#116696
  * rust-lang/rust#116695
  * rust-lang/rust#116644
  * rust-lang/rust#116630
* rust-lang/rust#116728
  * rust-lang/rust#116689
  * rust-lang/rust#116679
  * rust-lang/rust#116618
  * rust-lang/rust#116577
  * rust-lang/rust#115653
* rust-lang/rust#116702
* rust-lang/rust#116015
* rust-lang/rust#115822
* rust-lang/rust#116407
* rust-lang/rust#115719
* rust-lang/rust#115524
* rust-lang/rust#116705
* rust-lang/rust#116645
* rust-lang/rust#116233
* rust-lang/rust#115108
* rust-lang/rust#116670
* rust-lang/rust#116676
* rust-lang/rust#116666



Co-authored-by: Benoît du Garreau <bdgdlm@outlook.com>
Co-authored-by: Colin Finck <colin@reactos.org>
Co-authored-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com>
Co-authored-by: León Orell Valerian Liehr <me@fmease.dev>
Co-authored-by: Trevor Gross <tmgross@umich.edu>
Co-authored-by: Evan Merlock <evan@merlock.dev>
Co-authored-by: joboet <jonasboettiger@icloud.com>
Co-authored-by: Ralf Jung <post@ralfj.de>
Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Co-authored-by: Mark Rousskov <mark.simulacrum@gmail.com>
Co-authored-by: onur-ozkan <work@onurozkan.dev>
Co-authored-by: Nicholas Nethercote <n.nethercote@gmail.com>
Co-authored-by: The 8472 <git@infinite-source.de>
Co-authored-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Co-authored-by: reez12g <reez12g@gmail.com>
Co-authored-by: Jakub Beránek <berykubik@gmail.com>
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Oct 19, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Mar 3, 2024
Pkgsrc changes:
 * Adjust patches and cargo checksums to new versions.
 * For an external LLVM, set dependency of llvm >= 16, in accordance
   with the upstream changes.
 * Mark that on NetBSD we now need >= 9.0, so 8.x is no longer supported.
 * On NetBSD/sparc64 10.x, we now need GCC 12 to build the embedded
   LLVM, which is version 17; apparently GCC 10.4 or 10.5 mis-compiles it,
   resulting in an illegal instruction fault during the build.
   Ref. rust-lang/rust#117231

Upstream changes:

Version 1.75.0 (2023-12-28)
==========================

- [Stabilize `async fn` and return-position `impl Trait` in traits.]
  (rust-lang/rust#115822)
- [Allow function pointer signatures containing `&mut T` in `const` contexts.]
  (rust-lang/rust#116015)
- [Match `usize`/`isize` exhaustively with half-open ranges.]
  (rust-lang/rust#116692)
- [Guarantee that `char` has the same size and alignment as `u32`.]
  (rust-lang/rust#116894)
- [Document that the null pointer has the 0 address.]
  (rust-lang/rust#116988)
- [Allow partially moved values in `match`.]
  (rust-lang/rust#103208)
- [Add notes about non-compliant FP behavior on 32bit x86 targets.]
  (rust-lang/rust#113053)
- [Stabilize ratified RISC-V target features.]
  (rust-lang/rust#116485)

Compiler
--------

- [Rework negative coherence to properly consider impls that only
  partly overlap.] (rust-lang/rust#112875)
- [Bump `COINDUCTIVE_OVERLAP_IN_COHERENCE` to deny, and warn in dependencies.]
  (rust-lang/rust#116493)
- [Consider alias bounds when computing liveness in NLL.]
  (rust-lang/rust#116733)
- [Add the V (vector) extension to the `riscv64-linux-android` target spec.]
  (rust-lang/rust#116618)
- [Automatically enable cross-crate inlining for small functions]
  (rust-lang/rust#116505)
- Add several new tier 3 targets:
    - [`csky-unknown-linux-gnuabiv2hf`]
      (rust-lang/rust#117049)
    - [`i586-unknown-netbsd`]
      (rust-lang/rust#117170)
    - [`mipsel-unknown-netbsd`]
      (rust-lang/rust#117356)

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

Libraries
---------

- [Override `Waker::clone_from` to avoid cloning `Waker`s unnecessarily.]
  (rust-lang/rust#96979)
- [Implement `BufRead` for `VecDeque<u8>`.]
  (rust-lang/rust#110604)
- [Implement `FusedIterator` for `DecodeUtf16` when the inner iterator does.]
  (rust-lang/rust#110729)
- [Implement `Not, Bit{And,Or}{,Assign}` for IP addresses.]
  (rust-lang/rust#113747)
- [Implement `Default` for `ExitCode`.]
  (rust-lang/rust#114589)
- [Guarantee representation of None in NPO]
  (rust-lang/rust#115333)
- [Document when atomic loads are guaranteed read-only.]
  (rust-lang/rust#115577)
- [Broaden the consequences of recursive TLS initialization.]
  (rust-lang/rust#116172)
- [Windows: Support sub-millisecond sleep.]
  (rust-lang/rust#116461)
- [Fix generic bound of `str::SplitInclusive`'s `DoubleEndedIterator` impl]
  (rust-lang/rust#100806)
- [Fix exit status / wait status on non-Unix `cfg(unix)` platforms.]
  (rust-lang/rust#115108)

Stabilized APIs
---------------

- [`Atomic*::from_ptr`]
  (https://doc.rust-lang.org/stable/core/sync/atomic/struct.AtomicUsize.html#method.from_ptr)
- [`FileTimes`]
  (https://doc.rust-lang.org/stable/std/fs/struct.FileTimes.html)
- [`FileTimesExt`]
  (https://doc.rust-lang.org/stable/std/os/windows/fs/trait.FileTimesExt.html)
- [`File::set_modified`]
  (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.set_modified)
- [`File::set_times`]
  (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.set_times)
- [`IpAddr::to_canonical`]
  (https://doc.rust-lang.org/stable/core/net/enum.IpAddr.html#method.to_canonical)
- [`Ipv6Addr::to_canonical`]
  (https://doc.rust-lang.org/stable/core/net/struct.Ipv6Addr.html#method.to_canonical)
- [`Option::as_slice`]
  (https://doc.rust-lang.org/stable/core/option/enum.Option.html#method.as_slice)
- [`Option::as_mut_slice`]
  (https://doc.rust-lang.org/stable/core/option/enum.Option.html#method.as_mut_slice)
- [`pointer::byte_add`]
  (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_add)
- [`pointer::byte_offset`]
  (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_offset)
- [`pointer::byte_offset_from`]
  (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_offset_from)
- [`pointer::byte_sub`]
  (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_sub)
- [`pointer::wrapping_byte_add`]
  (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_add)
- [`pointer::wrapping_byte_offset`]
  (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_offset)
- [`pointer::wrapping_byte_sub`]
  (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_sub)

These APIs are now stable in const contexts:

- [`Ipv6Addr::to_ipv4_mapped`]
  (https://doc.rust-lang.org/stable/core/net/struct.Ipv6Addr.html#method.to_ipv4_mapped)
- [`MaybeUninit::assume_init_read`]
  (https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#method.assume_init_read)
- [`MaybeUninit::zeroed`]
  (https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#method.zeroed)
- [`mem::discriminant`]
  (https://doc.rust-lang.org/stable/core/mem/fn.discriminant.html)
- [`mem::zeroed`]
  (https://doc.rust-lang.org/stable/core/mem/fn.zeroed.html)

Cargo
-----

- [Add new packages to `[workspace.members]` automatically.]
  (rust-lang/cargo#12779)
- [Allow version-less `Cargo.toml` manifests.]
  (rust-lang/cargo#12786)
- [Make browser links out of HTML file paths.]
  (rust-lang/cargo#12889)

Rustdoc
-------

- [Accept less invalid Rust in rustdoc.]
  (rust-lang/rust#117450)
- [Document lack of object safety on affected traits.]
  (rust-lang/rust#113241)
- [Hide `#[repr(transparent)]` if it isn't part of the public ABI.]
  (rust-lang/rust#115439)
- [Show enum discriminant if it is a C-like variant.]
  (rust-lang/rust#116142)

Compatibility Notes
-------------------

- [FreeBSD targets now require at least version 12.]
  (rust-lang/rust#114521)
- [Formally demote tier 2 MIPS targets to tier 3.]
  (rust-lang/rust#115238)
- [Make misalignment a hard error in `const` contexts.]
  (rust-lang/rust#115524)
- [Fix detecting references to packed unsized fields.]
  (rust-lang/rust#115583)
- [Remove support for compiler plugins.]
  (rust-lang/rust#116412)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: rustdoc UI (generated HTML) disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide #[repr(transparent)] where the field is non-public