Auto merge of #116882 - fmease:rustdoc-generalized-priv-repr-heuristic, r=<try>

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

Follow-up to #115439.
Unblocks #116743, CC `@dtolnay.`
Fixes #128364.
Fixes #137440.

Only display the representation `#[repr(REPR)]` (where `REPR` is not `Rust` or `transparent`) of a given type if the type is not `#[non_exhaustive]`, if none of its variants (incl. the synthetic variants of structs) are `#[doc(hidden)]` or `#[non_exhaustive]` and if all of its fields are public and not `#[doc(hidden)]` since it's likely not meant to be considered part of the public ABI otherwise. `--document-{private,hidden}-items` works as expected in this context, too.

Moreover, we now also factor in the presence of `#[doc(hidden)]` and of `#[non_exhaustive]` when checking whether to show `repr(transparent)` or not.

Author: rust-bors[bot]

src/doc/rustdoc/src/advanced-features.md

@@ -89,20 +89,32 @@ 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
+
+Generally, rustdoc only displays the representation of a given type it's not `#[non_exhaustive]`,
+if none of its variants are `#[doc(hidden)]` or `#[non_exhaustive]` and
+if all of its fields are public and not `#[doc(hidden)]`
+since it's likely not meant to be considered part of the public ABI otherwise.
+
+Note that there's no way to overwrite that heuristic and force rustdoc to show the representation
+regardless.
+
+### `#[repr(transparent)]`
 
 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 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.
+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 and not `#[doc(hidden)]` or
+– in case all fields are 1-ZST fields — the type is not `#[non_exhaustive]` and
+at least one field is public and not `#[doc(hidden)]`.
+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 [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

src/librustdoc/clean/types.rs

@@ -1,3 +1,4 @@
+use std::borrow::Cow;
 use std::hash::Hash;
 use std::path::PathBuf;
 use std::sync::{Arc, OnceLock as OnceCell};
@@ -764,11 +765,12 @@ impl Item {
         Some(tcx.visibility(def_id))
     }
 
-    pub(crate) fn attributes_without_repr(&self, tcx: TyCtxt<'_>, is_json: bool) -> Vec<String> {
+    pub(crate) fn attributes(&self, tcx: TyCtxt<'_>, cache: &Cache, is_json: bool) -> Vec<String> {
         const ALLOWED_ATTRIBUTES: &[Symbol] =
             &[sym::export_name, sym::link_section, sym::no_mangle, sym::non_exhaustive];
 
-        self.attrs
+        let mut attrs: Vec<_> = self
+            .attrs
             .other_attrs
             .iter()
             .filter_map(|attr| {
@@ -786,36 +788,24 @@ impl Item {
                         }),
                     }
                 } else if attr.has_any_name(ALLOWED_ATTRIBUTES) {
-                    Some(
-                        rustc_hir_pretty::attribute_to_string(&tcx, attr)
-                            .replace("\\\n", "")
-                            .replace('\n', "")
-                            .replace("  ", " "),
-                    )
+                    let attr = rustc_hir_pretty::attribute_to_string(&tcx, attr)
+                        .replace("\\\n", "")
+                        .replace('\n', "")
+                        .replace("  ", " ");
+                    Some(attr)
                 } else {
                     None
                 }
             })
-            .collect()
-    }
+            .collect();
 
-    pub(crate) fn attributes_and_repr(
-        &self,
-        tcx: TyCtxt<'_>,
-        cache: &Cache,
-        is_json: bool,
-    ) -> Vec<String> {
-        let mut attrs = self.attributes_without_repr(tcx, is_json);
-
-        if let Some(repr_attr) = self.repr(tcx, cache, is_json) {
-            attrs.push(repr_attr);
+        if let Some(def_id) = self.def_id()
+            && let Some(attr) = repr_attribute(tcx, cache, def_id, is_json)
+        {
+            attrs.push(attr);
         }
-        attrs
-    }
 
-    /// Returns a stringified `#[repr(...)]` attribute.
-    pub(crate) fn repr(&self, tcx: TyCtxt<'_>, cache: &Cache, is_json: bool) -> Option<String> {
-        repr_attributes(tcx, cache, self.def_id()?, self.type_(), is_json)
+        attrs
     }
 
     pub fn is_doc_hidden(&self) -> bool {
@@ -827,71 +817,123 @@ impl Item {
     }
 }
 
-pub(crate) fn repr_attributes(
-    tcx: TyCtxt<'_>,
+/// Compute the *public* `#[repr]` of the item given by `DefId`.
+///
+/// Read more about it here:
+/// <https://doc.rust-lang.org/nightly/rustdoc/advanced-features.html#repr-documenting-the-representation-of-a-type>.
+pub(crate) fn repr_attribute<'tcx>(
+    tcx: TyCtxt<'tcx>,
     cache: &Cache,
     def_id: DefId,
-    item_type: ItemType,
     is_json: bool,
 ) -> Option<String> {
-    use rustc_abi::IntegerType;
+    let adt = match tcx.def_kind(def_id) {
+        DefKind::Struct | DefKind::Enum | DefKind::Union => tcx.adt_def(def_id),
+        _ => return None,
+    };
+    let repr = adt.repr();
 
-    if !matches!(item_type, ItemType::Struct | ItemType::Enum | ItemType::Union) {
+    let is_visible = |def_id| cache.document_hidden || !tcx.is_doc_hidden(def_id);
+    let is_public_field = |field: &ty::FieldDef| {
+        (cache.document_private || field.vis.is_public()) && is_visible(field.did)
+    };
+    let is_exhaustive = |def_id| !tcx.has_attr(def_id, sym::non_exhaustive);
+
+    if repr.transparent() {
+        // The transparent repr is public iff the non-1-ZST field is public and visible or
+        // – in case all fields are 1-ZST fields — the type is exhaustive and at least
+        // one field is public and visible.
+        let is_public = 'is_public: {
+            if is_json {
+                break 'is_public true;
+            }
+
+            // `#[repr(transparent)]` can only be applied to structs and single-variant enums.
+            let var = adt.variant(rustc_abi::FIRST_VARIANT); // the first and only variant
+
+            // Therefore, if we have an enum, we don't care if it is `#[non_exhaustive]` or not
+            // since the user isn't allowed to add more variants to it later anyway.
+
+            if !is_visible(var.def_id) {
+                break 'is_public false;
+            }
+
+            // Side note: There can only ever be one or zero non-1-ZST fields.
+            let non_1zst_field = var.fields.iter().find(|field| {
+                let ty = ty::TypingEnv::post_analysis(tcx, field.did)
+                    .as_query_input(tcx.type_of(field.did).instantiate_identity());
+                tcx.layout_of(ty).is_ok_and(|layout| !layout.is_1zst())
+            });
+
+            match non_1zst_field {
+                // We don't care if the containing variant is `#[non_exhaustive]` or not as the
+                // user is only allowed to add more *1-ZST* fields which don't matter in the
+                // presence of this non-1-ZST field.
+                Some(field) => is_public_field(field),
+                None => {
+                    is_exhaustive(var.def_id)
+                        && (var.fields.is_empty() || var.fields.iter().any(is_public_field))
+                }
+            }
+        };
+
+        // Since the transparent repr can't have any other reprs or
+        // repr modifiers beside it, we can safely return early here.
+        return is_public.then(|| "#[repr(transparent)]".into());
+    }
+
+    // Fast path which avoids looking through the variants and fields in
+    // the common case of no `#[repr]` or in the case of `#[repr(Rust)]`.
+    // FIXME: This check is not very robust / forward compatible!
+    if !repr.c()
+        && !repr.simd()
+        && repr.int.is_none()
+        && repr.pack.is_none()
+        && repr.align.is_none()
+    {
         return None;
     }
-    let adt = tcx.adt_def(def_id);
-    let repr = adt.repr();
-    let mut out = Vec::new();
-    if repr.c() {
-        out.push("C");
+
+    // The repr is public iff all components are public, visible and exhaustive.
+    let is_public = is_json
+        || is_exhaustive(def_id)
+            && adt.variants().iter().all(|variant| {
+                is_exhaustive(variant.def_id)
+                    && is_visible(variant.def_id)
+                    && variant.fields.iter().all(is_public_field)
+            });
+    if !is_public {
+        return None;
     }
-    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
-            || is_json
-            || adt
-                .all_fields()
-                .find(|field| {
-                    let ty = field.ty(tcx, ty::GenericArgs::identity_for_item(tcx, field.did));
-                    tcx.layout_of(ty::TypingEnv::post_analysis(tcx, field.did).as_query_input(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");
-        }
+    let mut result = Vec::<Cow<'_, _>>::new();
+
+    if repr.c() {
+        result.push("C".into());
     }
     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);
+        result.push("simd".into());
     }
-    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' })
-            }
-            IntegerType::Fixed(size, is_signed) => {
-                format!("{}{}", if is_signed { 'i' } else { 'u' }, size.size().bytes() * 8)
+        let prefix = if int.is_signed() { 'i' } else { 'u' };
+        let int = match int {
+            rustc_abi::IntegerType::Pointer(_) => format!("{prefix}size"),
+            rustc_abi::IntegerType::Fixed(int, _) => {
+                format!("{prefix}{}", int.size().bytes() * 8)
             }
         };
-        out.push(&int_s);
+        result.push(int.into());
     }
-    if !out.is_empty() { Some(format!("#[repr({})]", out.join(", "))) } else { None }
+
+    // Render modifiers last.
+    if let Some(pack) = repr.pack {
+        result.push(format!("packed({})", pack.bytes()).into());
+    }
+    if let Some(align) = repr.align {
+        result.push(format!("align({})", align.bytes()).into());
+    }
+
+    (!result.is_empty()).then(|| format!("#[repr({})]", result.join(", ")))
 }
 
 #[derive(Clone, Debug)]

src/librustdoc/html/render/mod.rs

@@ -1194,7 +1194,7 @@ fn render_assoc_item(
 // a whitespace prefix and newline.
 fn render_attributes_in_pre(it: &clean::Item, prefix: &str, cx: &Context<'_>) -> impl fmt::Display {
     fmt::from_fn(move |f| {
-        for a in it.attributes_and_repr(cx.tcx(), cx.cache(), false) {
+        for a in it.attributes(cx.tcx(), cx.cache(), false) {
             writeln!(f, "{prefix}{a}")?;
         }
         Ok(())
@@ -1210,19 +1210,14 @@ fn render_code_attribute(code_attr: CodeAttribute, w: &mut impl fmt::Write) {
 // When an attribute is rendered inside a <code> tag, it is formatted using
 // a div to produce a newline after it.
 fn render_attributes_in_code(w: &mut impl fmt::Write, it: &clean::Item, cx: &Context<'_>) {
-    for attr in it.attributes_and_repr(cx.tcx(), cx.cache(), false) {
+    for attr in it.attributes(cx.tcx(), cx.cache(), false) {
         render_code_attribute(CodeAttribute(attr), w);
     }
 }
 
-/// used for type aliases to only render their `repr` attribute.
-fn render_repr_attributes_in_code(
-    w: &mut impl fmt::Write,
-    cx: &Context<'_>,
-    def_id: DefId,
-    item_type: ItemType,
-) {
-    if let Some(repr) = clean::repr_attributes(cx.tcx(), cx.cache(), def_id, item_type, false) {
+/// Render the `repr` attribute.
+fn render_repr_attribute_in_code(w: &mut impl fmt::Write, cx: &Context<'_>, def_id: DefId) {
+    if let Some(repr) = clean::repr_attribute(cx.tcx(), cx.cache(), def_id, false) {
         render_code_attribute(CodeAttribute(repr), w);
     }
 }

src/librustdoc/html/render/print_item.rs

@@ -20,7 +20,7 @@ use super::{
     collect_paths_for_type, document, ensure_trailing_slash, get_filtered_impls_for_reference,
     item_ty_to_section, notable_traits_button, notable_traits_json, render_all_impls,
     render_assoc_item, render_assoc_items, render_attributes_in_code, render_attributes_in_pre,
-    render_impl, render_repr_attributes_in_code, render_rightside, render_stability_since_raw,
+    render_impl, render_repr_attribute_in_code, render_rightside, render_stability_since_raw,
     render_stability_since_raw_with_extra, write_section_heading,
 };
 use crate::clean;
@@ -1480,18 +1480,14 @@ impl<'a, 'cx: 'a> ItemUnion<'a, 'cx> {
     fn render_attributes_in_pre(&self) -> impl fmt::Display {
         fmt::from_fn(move |f| {
             if self.is_type_alias {
-                // For now the only attributes we render for type aliases are `repr` attributes.
-                if let Some(repr) = clean::repr_attributes(
-                    self.cx.tcx(),
-                    self.cx.cache(),
-                    self.def_id,
-                    ItemType::Union,
-                    false,
-                ) {
+                // For now the only attribute we render for type aliases is the `repr` attribute.
+                if let Some(repr) =
+                    clean::repr_attribute(self.cx.tcx(), self.cx.cache(), self.def_id, false)
+                {
                     writeln!(f, "{repr}")?;
                 };
             } else {
-                for a in self.it.attributes_and_repr(self.cx.tcx(), self.cx.cache(), false) {
+                for a in self.it.attributes(self.cx.tcx(), self.cx.cache(), false) {
                     writeln!(f, "{a}")?;
                 }
             }
@@ -1558,8 +1554,8 @@ impl<'clean> DisplayEnum<'clean> {
 
         wrap_item(w, |w| {
             if is_type_alias {
-                // For now the only attributes we render for type aliases are `repr` attributes.
-                render_repr_attributes_in_code(w, cx, self.def_id, ItemType::Enum);
+                // For now the only attribute we render for type aliases is the `repr` attribute.
+                render_repr_attribute_in_code(w, cx, self.def_id);
             } else {
                 render_attributes_in_code(w, it, cx);
             }
@@ -2016,8 +2012,8 @@ impl<'a> DisplayStruct<'a> {
     ) -> fmt::Result {
         wrap_item(w, |w| {
             if is_type_alias {
-                // For now the only attributes we render for type aliases are `repr` attributes.
-                render_repr_attributes_in_code(w, cx, self.def_id, ItemType::Struct);
+                // For now the only attribute we render for type aliases is the `repr` attribute.
+                render_repr_attribute_in_code(w, cx, self.def_id);
             } else {
                 render_attributes_in_code(w, it, cx);
             }

src/librustdoc/json/conversions.rs

@@ -41,7 +41,7 @@ impl JsonRenderer<'_> {
             })
             .collect();
         let docs = item.opt_doc_value();
-        let attrs = item.attributes_and_repr(self.tcx, self.cache(), true);
+        let attrs = item.attributes(self.tcx, self.cache(), true);
         let span = item.span(self.tcx);
         let visibility = item.visibility(self.tcx);
         let clean::ItemInner { name, item_id, .. } = *item.inner;

tests/rustdoc-gui/src/test_docs/lib.rs

@@ -455,10 +455,10 @@ pub fn safe_fn() {}
 
 #[repr(C)]
 pub struct WithGenerics<T: TraitWithNoDocblocks, S = String, E = WhoLetTheDogOut, P = i8> {
-    s: S,
-    t: T,
-    e: E,
-    p: P,
+    pub s: S,
+    pub t: T,
+    pub e: E,
+    pub p: P,
 }
 
 pub struct StructWithPublicUndocumentedFields {