From dc27ca1dc73e4f924ec096701cd26badbff22fd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Wed, 18 Oct 2023 11:03:28 +0200 Subject: [PATCH] rustdoc: hide `#[repr(...)]` if it isn't part of the public ABI --- src/doc/rustdoc/src/advanced-features.md | 10 +- src/librustdoc/clean/types.rs | 175 ++++++++++++------- tests/rustdoc-gui/src/test_docs/lib.rs | 8 +- tests/rustdoc/inline_cross/auxiliary/repr.rs | 50 +++++- tests/rustdoc/inline_cross/repr.rs | 25 +++ tests/rustdoc/repr.rs | 69 ++++++++ 6 files changed, 262 insertions(+), 75 deletions(-) diff --git a/src/doc/rustdoc/src/advanced-features.md b/src/doc/rustdoc/src/advanced-features.md index c02c9aebe7eef..f97b9a1dc49b7 100644 --- a/src/doc/rustdoc/src/advanced-features.md +++ b/src/doc/rustdoc/src/advanced-features.md @@ -89,7 +89,13 @@ 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 +## `#[repr(...)]`: Documenting the representation of a type + + + +### `#[repr(transparent)]` + + You can read more about `#[repr(transparent)]` itself in the [Rust Reference][repr-trans-ref] and in the [Rustonomicon][repr-trans-nomicon]. @@ -102,7 +108,7 @@ fields are 1-ZST fields. The term *1-ZST* refers to types that are one-aligned a 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 [current limitations][cross-crate-cfg-doc], 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 +Therefore, if you would like to do so, you should always write that down in prose independently of whether you use `cfg_attr` or not. [repr-trans-ref]: https://doc.rust-lang.org/reference/type-layout.html#the-transparent-representation diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 1ec5f38b6ec0f..b4b343f7ac2eb 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -709,17 +709,15 @@ impl Item { Some(tcx.visibility(def_id)) } - pub(crate) fn attributes( + pub(crate) fn attributes<'tcx>( &self, - tcx: TyCtxt<'_>, + tcx: TyCtxt<'tcx>, cache: &Cache, keep_as_is: bool, ) -> Vec { const ALLOWED_ATTRIBUTES: &[Symbol] = &[sym::export_name, sym::link_section, sym::no_mangle, sym::non_exhaustive]; - use rustc_abi::IntegerType; - let mut attrs: Vec = self .attrs .other_attrs @@ -739,67 +737,122 @@ impl Item { } }) .collect(); - if !keep_as_is - && let Some(def_id) = self.def_id() - && let ItemType::Struct | ItemType::Enum | ItemType::Union = self.type_() - { - let adt = tcx.adt_def(def_id); - let repr = adt.repr(); - let mut out = Vec::new(); - if repr.c() { - out.push("C"); - } - 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 - || adt - .all_fields() - .find(|field| { - let ty = - field.ty(tcx, ty::GenericArgs::identity_for_item(tcx, field.did)); - tcx.layout_of(tcx.param_env(field.did).and(ty)) - .is_ok_and(|layout| !layout.is_1zst()) - }) - .map_or_else( - || adt.all_fields().any(|field| field.vis.is_public()), - |field| field.vis.is_public(), - ); - if render_transparent { - out.push("transparent"); - } - } - if repr.simd() { - out.push("simd"); - } - let pack_s; - if let Some(pack) = repr.pack { - pack_s = format!("packed({})", pack.bytes()); - out.push(&pack_s); - } - let align_s; - if let Some(align) = repr.align { - align_s = format!("align({})", align.bytes()); - out.push(&align_s); - } - let int_s; - if let Some(int) = repr.int { - int_s = match int { - IntegerType::Pointer(is_signed) => { - format!("{}size", if is_signed { 'i' } else { 'u' }) + if !keep_as_is && let Some(repr) = self.repr(tcx, cache) { + attrs.push(repr); + } + + attrs + } + + /// Compute the *public* `#[repr]` of this item. + /// + /// Read more about it here: + /// https://doc.rust-lang.org/nightly/rustdoc/advanced-features.html#repr-documenting-the-representation-of-a-type + fn repr<'tcx>(&self, tcx: TyCtxt<'tcx>, cache: &Cache) -> Option { + let def_id = self.def_id()?; + let (ItemType::Struct | ItemType::Enum | ItemType::Union) = self.type_() else { + return None; + }; + + let adt = tcx.adt_def(def_id); + let repr = adt.repr(); + + let is_visible = |def_id| cache.document_hidden || !tcx.is_doc_hidden(def_id); + let is_field_public = + |field: &'tcx ty::FieldDef| field.vis.is_public() && is_visible(field.did); + + if repr.transparent() { + // `repr(transparent)` is public iff the non-1-ZST field is public or + // at least one field is public in case all fields are 1-ZST fields. + let is_public = cache.document_private + || adt.variants().iter().all(|variant| { + if !is_visible(variant.def_id) { + return false; } - IntegerType::Fixed(size, is_signed) => { - format!("{}{}", if is_signed { 'i' } else { 'u' }, size.size().bytes() * 8) + + let field = variant.fields.iter().find(|field| { + let args = ty::GenericArgs::identity_for_item(tcx, field.did); + let ty = field.ty(tcx, args); + tcx.layout_of(tcx.param_env(field.did).and(ty)) + .is_ok_and(|layout| !layout.is_1zst()) + }); + + if let Some(field) = field { + return is_field_public(field); } - }; - out.push(&int_s); - } - if !out.is_empty() { - attrs.push(format!("#[repr({})]", out.join(", "))); - } + + adt.variants().iter().all(|variant| { + variant.fields.is_empty() || variant.fields.iter().any(is_field_public) + }) + }); + + // Since `repr(transparent)` can't have any other reprs or + // repr modifiers beside it, we can safely return early here. + return is_public.then(|| "#[repr(transparent)]".into()); } - attrs + + // Fast path which avoids looking through the variants and fields in + // the common case of no `#[repr]` or in the case of `#[repr(Rust)]`. + if !repr.c() + && !repr.simd() + && repr.int.is_none() + && repr.pack.is_none() + && repr.align.is_none() + { + return None; + } + + let is_public = cache.document_private + || if adt.is_enum() { + // FIXME(fmease): Should we take the visibility of fields of variants into account? + // FIXME(fmease): `any` or `all`? + adt.variants().is_empty() + || adt.variants().iter().any(|variant| is_visible(variant.def_id)) + } else { + // FIXME(fmease): `all` or `any`? + adt.all_fields().all(is_field_public) + }; + if !is_public { + return None; + } + + let mut result = Vec::new(); + + if repr.c() { + result.push("C"); + } + if repr.simd() { + result.push("simd"); + } + let int_s; + if let Some(int) = repr.int { + int_s = match int { + rustc_abi::IntegerType::Pointer(is_signed) => { + format!("{}size", if is_signed { 'i' } else { 'u' }) + } + rustc_abi::IntegerType::Fixed(size, is_signed) => { + format!("{}{}", if is_signed { 'i' } else { 'u' }, size.size().bytes() * 8) + } + }; + result.push(&int_s); + } + let pack_s; + if let Some(pack) = repr.pack { + pack_s = format!("packed({})", pack.bytes()); + result.push(&pack_s); + } + let align_s; + if let Some(align) = repr.align { + align_s = format!("align({})", align.bytes()); + result.push(&align_s); + } + + if result.is_empty() { + return None; + } + + Some(format!("#[repr({})]", result.join(", "))) } pub fn is_doc_hidden(&self) -> bool { diff --git a/tests/rustdoc-gui/src/test_docs/lib.rs b/tests/rustdoc-gui/src/test_docs/lib.rs index 7397992c0ab0f..27daf6ffba8fd 100644 --- a/tests/rustdoc-gui/src/test_docs/lib.rs +++ b/tests/rustdoc-gui/src/test_docs/lib.rs @@ -454,10 +454,10 @@ pub fn safe_fn() {} #[repr(C)] pub struct WithGenerics { - s: S, - t: T, - e: E, - p: P, + pub s: S, + pub t: T, + pub e: E, + pub p: P, } pub struct StructWithPublicUndocumentedFields { diff --git a/tests/rustdoc/inline_cross/auxiliary/repr.rs b/tests/rustdoc/inline_cross/auxiliary/repr.rs index 35f08c11b7b3a..40fa9d20ffa18 100644 --- a/tests/rustdoc/inline_cross/auxiliary/repr.rs +++ b/tests/rustdoc/inline_cross/auxiliary/repr.rs @@ -1,26 +1,60 @@ #![feature(repr_simd)] -#[repr(C, align(8))] +#[repr(Rust)] +pub struct ReprRust; + +#[repr(C, align(8))] // public pub struct ReprC { - field: u8, + pub field: u8, } -#[repr(simd, packed(2))] + +#[repr(C)] // private +pub struct ReprCPrivField { + private: u8, + pub public: i8, +} + +#[repr(align(4))] // private +pub struct ReprAlignHiddenField { + #[doc(hidden)] + pub hidden: i16, +} + +#[repr(simd, packed(2))] // public pub struct ReprSimd { - field: u8, + pub field: u8, } -#[repr(transparent)] + +#[repr(transparent)] // public pub struct ReprTransparent { - pub field: u8, + pub field: u8, // non-1-ZST field } -#[repr(isize)] + +#[repr(isize)] // public pub enum ReprIsize { Bla, } -#[repr(u8)] + +#[repr(u8)] // public pub enum ReprU8 { Bla, } +#[repr(u32)] // public +pub enum ReprU32 { + #[doc(hidden)] + Hidden, + Public, +} + +#[repr(u64)] // private +pub enum ReprU64HiddenVariants { + #[doc(hidden)] + A, + #[doc(hidden)] + B, +} + #[repr(transparent)] // private pub struct ReprTransparentPrivField { field: u32, // non-1-ZST field diff --git a/tests/rustdoc/inline_cross/repr.rs b/tests/rustdoc/inline_cross/repr.rs index d13e560b8d77f..eb4d9ff29c98d 100644 --- a/tests/rustdoc/inline_cross/repr.rs +++ b/tests/rustdoc/inline_cross/repr.rs @@ -7,22 +7,47 @@ extern crate repr; +// Never display `repr(Rust)` since it's the default anyway. +//@ has 'foo/struct.ReprRust.html' +//@ !has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(Rust)]' +pub use repr::ReprRust; + //@ has 'foo/struct.ReprC.html' //@ has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(C, align(8))]' pub use repr::ReprC; + +//@ has 'foo/struct.ReprCPrivField.html' +//@ !has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(C)]' +pub use repr::ReprCPrivField; + +//@ has 'foo/struct.ReprAlignHiddenField.html' +//@ !has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(align(4))]' +pub use repr::ReprAlignHiddenField; + //@ has 'foo/struct.ReprSimd.html' //@ has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(simd, packed(2))]' pub use repr::ReprSimd; + //@ has 'foo/struct.ReprTransparent.html' //@ has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]' pub use repr::ReprTransparent; + //@ has 'foo/enum.ReprIsize.html' //@ has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(isize)]' pub use repr::ReprIsize; + //@ has 'foo/enum.ReprU8.html' //@ has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(u8)]' pub use repr::ReprU8; +//@ has 'foo/enum.ReprU32.html' +//@ has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(u32)]' +pub use repr::ReprU32; + +//@ has 'foo/enum.ReprU64HiddenVariants.html' +//@ !has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(u64)]' +pub use repr::ReprU64HiddenVariants; + // Regression test for . // Check that we show `#[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. diff --git a/tests/rustdoc/repr.rs b/tests/rustdoc/repr.rs index f4f683b3d81a2..50bd8a3176790 100644 --- a/tests/rustdoc/repr.rs +++ b/tests/rustdoc/repr.rs @@ -1,3 +1,32 @@ +// Never display `repr(Rust)` since it's the default anyway. +//@ has 'repr/struct.ReprRust.html' +//@ !has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(Rust)]' +#[repr(Rust)] +pub struct ReprRust; + +//@ has 'repr/struct.ReprCPubFields.html' +//@ has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(C)]' +#[repr(C)] // public +pub struct ReprCPubFields { + pub a: u32, + pub b: u32, +} + +//@ has 'repr/struct.ReprCPrivFields.html' +//@ !has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(C)]' +#[repr(C)] // private +pub struct ReprCPrivFields { + a: u32, + b: u32, +} + +//@ has 'repr/enum.ReprU32Align.html' +//@ has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(u32, align(8))]' +#[repr(u32, align(8))] // public +pub enum ReprU32Align { + Variant(u16), +} + // Regression test for . // Check that we show `#[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. @@ -26,4 +55,44 @@ pub struct ReprTransparentPub1ZstField { pub marker1: Marker, } +//@ has 'repr/struct.ReprTransparentUnitStruct.html' +//@ has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]' +#[repr(transparent)] // public +pub struct ReprTransparentUnitStruct; + +//@ has 'repr/enum.ReprTransparentEnumUnitVariant.html' +//@ has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]' +#[repr(transparent)] // public +pub enum ReprTransparentEnumUnitVariant { + Variant, +} + +//@ has 'repr/enum.ReprTransparentEnumHiddenUnitVariant.html' +//@ !has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]' +#[repr(transparent)] // private +pub enum ReprTransparentEnumHiddenUnitVariant { + #[doc(hidden)] Variant, +} + +//@ has 'repr/enum.ReprTransparentEnumPub1ZstField.html' +//@ has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]' +#[repr(transparent)] // public +pub enum ReprTransparentEnumPub1ZstField { + Variant { + field: u64, // non-1-ZST field + #[doc(hidden)] + marker: Marker, + }, +} + +//@ has 'repr/enum.ReprTransparentEnumHidden1ZstField.html' +//@ !has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]' +#[repr(transparent)] // private +pub enum ReprTransparentEnumHidden1ZstField { + Variant { + #[doc(hidden)] + field: u64, // non-1-ZST field + }, +} + struct Marker; // 1-ZST