From 64fa12a4fb1447e3368ae2cd08cf75ea576997ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 15 Aug 2023 10:10:35 +0200 Subject: [PATCH] rustdoc: hide repr(transparent) if it isn't part of the public ABI --- src/doc/rustdoc/src/advanced-features.md | 20 ++++++++ src/librustdoc/clean/types.rs | 51 +++++++++++++------ src/librustdoc/html/render/mod.rs | 16 +++--- src/librustdoc/html/render/print_item.rs | 23 ++++----- src/librustdoc/json/conversions.rs | 3 +- tests/rustdoc/inline_cross/attributes.rs | 7 +++ .../inline_cross/auxiliary/attributes.rs | 2 + tests/rustdoc/inline_cross/auxiliary/repr.rs | 22 +++++++- tests/rustdoc/inline_cross/repr.rs | 21 ++++++-- tests/rustdoc/repr.rs | 29 +++++++++++ 10 files changed, 152 insertions(+), 42 deletions(-) create mode 100644 tests/rustdoc/inline_cross/attributes.rs create mode 100644 tests/rustdoc/inline_cross/auxiliary/attributes.rs create mode 100644 tests/rustdoc/repr.rs diff --git a/src/doc/rustdoc/src/advanced-features.md b/src/doc/rustdoc/src/advanced-features.md index dbf0baec04c03..1733c8fc9a246 100644 --- a/src/doc/rustdoc/src/advanced-features.md +++ b/src/doc/rustdoc/src/advanced-features.md @@ -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 + +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. + +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 +you use `cfg_attr` or not. + +[repr-trans-ref]: https://doc.rust-lang.org/reference/type-layout.html#the-transparent-representation +[repr-trans-nomicon]: https://doc.rust-lang.org/nomicon/other-reprs.html#reprtransparent +[cross-crate-cfg-doc]: https://github.com/rust-lang/rust/issues/114952 diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index b276745f3170b..ecbe7017bf199 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -709,12 +709,16 @@ impl Item { Some(tcx.visibility(def_id)) } - pub(crate) fn attributes(&self, tcx: TyCtxt<'_>, keep_as_is: bool) -> Vec { + pub(crate) fn attributes( + &self, + tcx: TyCtxt<'_>, + cache: &Cache, + keep_as_is: bool, + ) -> Vec { const ALLOWED_ATTRIBUTES: &[Symbol] = - &[sym::export_name, sym::link_section, sym::no_mangle, sym::repr, sym::non_exhaustive]; + &[sym::export_name, sym::link_section, sym::no_mangle, sym::non_exhaustive]; use rustc_abi::IntegerType; - use rustc_middle::ty::ReprFlags; let mut attrs: Vec = self .attrs @@ -735,20 +739,38 @@ impl Item { } }) .collect(); - if let Some(def_id) = self.def_id() && - !def_id.is_local() && - // This check is needed because `adt_def` will panic if not a compatible type otherwise... - matches!(self.type_(), ItemType::Struct | ItemType::Enum | ItemType::Union) + if !keep_as_is + && let Some(def_id) = self.def_id() + && let ItemType::Struct | ItemType::Enum | ItemType::Union = self.type_() { - let repr = tcx.adt_def(def_id).repr(); + let adt = tcx.adt_def(def_id); + let repr = adt.repr(); let mut out = Vec::new(); - if repr.flags.contains(ReprFlags::IS_C) { + if repr.c() { out.push("C"); } - if repr.flags.contains(ReprFlags::IS_TRANSPARENT) { - out.push("transparent"); + 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.flags.contains(ReprFlags::IS_SIMD) { + if repr.simd() { out.push("simd"); } let pack_s; @@ -773,10 +795,9 @@ impl Item { }; out.push(&int_s); } - if out.is_empty() { - return Vec::new(); + if !out.is_empty() { + attrs.push(format!("#[repr({})]", out.join(", "))); } - attrs.push(format!("#[repr({})]", out.join(", "))); } attrs } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index f70f59d3be389..04889ba710ce9 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -856,10 +856,10 @@ fn assoc_method( let (indent, indent_str, end_newline) = if parent == ItemType::Trait { header_len += 4; let indent_str = " "; - write!(w, "{}", render_attributes_in_pre(meth, indent_str, tcx)); + write!(w, "{}", render_attributes_in_pre(meth, indent_str, cx)); (4, indent_str, Ending::NoNewline) } else { - render_attributes_in_code(w, meth, tcx); + render_attributes_in_code(w, meth, cx); (0, "", Ending::Newline) }; w.reserve(header_len + "{".len() + "".len()); @@ -1035,13 +1035,13 @@ fn render_assoc_item( // When an attribute is rendered inside a `
` tag, it is formatted using
 // a whitespace prefix and newline.
-fn render_attributes_in_pre<'a, 'b: 'a>(
+fn render_attributes_in_pre<'a, 'tcx: 'a>(
     it: &'a clean::Item,
     prefix: &'a str,
-    tcx: TyCtxt<'b>,
-) -> impl fmt::Display + Captures<'a> + Captures<'b> {
+    cx: &'a Context<'tcx>,
+) -> impl fmt::Display + Captures<'a> + Captures<'tcx> {
     crate::html::format::display_fn(move |f| {
-        for a in it.attributes(tcx, false) {
+        for a in it.attributes(cx.tcx(), cx.cache(), false) {
             writeln!(f, "{prefix}{a}")?;
         }
         Ok(())
@@ -1050,8 +1050,8 @@ fn render_attributes_in_pre<'a, 'b: 'a>(
 
 // When an attribute is rendered inside a  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, tcx: TyCtxt<'_>) {
-    for attr in it.attributes(tcx, false) {
+fn render_attributes_in_code(w: &mut impl fmt::Write, it: &clean::Item, cx: &Context<'_>) {
+    for attr in it.attributes(cx.tcx(), cx.cache(), false) {
         write!(w, "
{attr}
").unwrap();
     }
 }
diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs
index c6751c9585ee9..209d71c4f3ba7 100644
--- a/src/librustdoc/html/render/print_item.rs
+++ b/src/librustdoc/html/render/print_item.rs
@@ -117,8 +117,7 @@ macro_rules! item_template_methods {
         fn render_attributes_in_pre<'b>(&'b self) -> impl fmt::Display + Captures<'a> + 'b + Captures<'cx> {
             display_fn(move |f| {
                 let (item, cx) = self.item_and_mut_cx();
-                let tcx = cx.tcx();
-                let v = render_attributes_in_pre(item, "", tcx);
+                let v = render_attributes_in_pre(item, "", &cx);
                 write!(f, "{v}")
             })
         }
@@ -656,7 +655,7 @@ fn item_function(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, f: &cle
             w,
             "{attrs}{vis}{constness}{asyncness}{unsafety}{abi}fn \
                 {name}{generics}{decl}{notable_traits}{where_clause}",
-            attrs = render_attributes_in_pre(it, "", tcx),
+            attrs = render_attributes_in_pre(it, "", cx),
             vis = visibility,
             constness = constness,
             asyncness = asyncness,
@@ -691,7 +690,7 @@ fn item_trait(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean:
         write!(
             w,
             "{attrs}{vis}{unsafety}{is_auto}trait {name}{generics}{bounds}",
-            attrs = render_attributes_in_pre(it, "", tcx),
+            attrs = render_attributes_in_pre(it, "", cx),
             vis = visibility_print_with_space(it.visibility(tcx), it.item_id, cx),
             unsafety = t.unsafety(tcx).print_with_space(),
             is_auto = if t.is_auto(tcx) { "auto " } else { "" },
@@ -1170,7 +1169,7 @@ fn item_trait_alias(
         write!(
             w,
             "{attrs}trait {name}{generics}{where_b} = {bounds};",
-            attrs = render_attributes_in_pre(it, "", cx.tcx()),
+            attrs = render_attributes_in_pre(it, "", cx),
             name = it.name.unwrap(),
             generics = t.generics.print(cx),
             where_b = print_where_clause(&t.generics, cx, 0, Ending::Newline),
@@ -1198,7 +1197,7 @@ fn item_opaque_ty(
         write!(
             w,
             "{attrs}type {name}{generics}{where_clause} = impl {bounds};",
-            attrs = render_attributes_in_pre(it, "", cx.tcx()),
+            attrs = render_attributes_in_pre(it, "", cx),
             name = it.name.unwrap(),
             generics = t.generics.print(cx),
             where_clause = print_where_clause(&t.generics, cx, 0, Ending::Newline),
@@ -1223,7 +1222,7 @@ fn item_type_alias(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &c
             write!(
                 w,
                 "{attrs}{vis}type {name}{generics}{where_clause} = {type_};",
-                attrs = render_attributes_in_pre(it, "", cx.tcx()),
+                attrs = render_attributes_in_pre(it, "", cx),
                 vis = visibility_print_with_space(it.visibility(cx.tcx()), it.item_id, cx),
                 name = it.name.unwrap(),
                 generics = t.generics.print(cx),
@@ -1408,7 +1407,7 @@ fn item_enum(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, e: &clean::
     let tcx = cx.tcx();
     let count_variants = e.variants().count();
     wrap_item(w, |w| {
-        render_attributes_in_code(w, it, tcx);
+        render_attributes_in_code(w, it, cx);
         write!(
             w,
             "{}enum {}{}",
@@ -1644,7 +1643,7 @@ fn item_primitive(w: &mut impl fmt::Write, cx: &mut Context<'_>, it: &clean::Ite
 fn item_constant(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, c: &clean::Constant) {
     wrap_item(w, |w| {
         let tcx = cx.tcx();
-        render_attributes_in_code(w, it, tcx);
+        render_attributes_in_code(w, it, cx);
 
         write!(
             w,
@@ -1693,7 +1692,7 @@ fn item_constant(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, c: &cle
 
 fn item_struct(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, s: &clean::Struct) {
     wrap_item(w, |w| {
-        render_attributes_in_code(w, it, cx.tcx());
+        render_attributes_in_code(w, it, cx);
         render_struct(w, it, Some(&s.generics), s.ctor_kind, &s.fields, "", true, cx);
     });
 
@@ -1753,7 +1752,7 @@ fn item_fields(
 
 fn item_static(w: &mut impl fmt::Write, cx: &mut Context<'_>, it: &clean::Item, s: &clean::Static) {
     wrap_item(w, |buffer| {
-        render_attributes_in_code(buffer, it, cx.tcx());
+        render_attributes_in_code(buffer, it, cx);
         write!(
             buffer,
             "{vis}static {mutability}{name}: {typ}",
@@ -1771,7 +1770,7 @@ fn item_static(w: &mut impl fmt::Write, cx: &mut Context<'_>, it: &clean::Item,
 fn item_foreign_type(w: &mut impl fmt::Write, cx: &mut Context<'_>, it: &clean::Item) {
     wrap_item(w, |buffer| {
         buffer.write_str("extern {\n").unwrap();
-        render_attributes_in_code(buffer, it, cx.tcx());
+        render_attributes_in_code(buffer, it, cx);
         write!(
             buffer,
             "    {}type {};\n}}",
diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs
index 0886501596056..47e50107b520c 100644
--- a/src/librustdoc/json/conversions.rs
+++ b/src/librustdoc/json/conversions.rs
@@ -18,6 +18,7 @@ use rustdoc_json_types::*;
 
 use crate::clean::{self, ItemId};
 use crate::formats::item_type::ItemType;
+use crate::formats::FormatRenderer;
 use crate::json::JsonRenderer;
 use crate::passes::collect_intra_doc_links::UrlFragment;
 
@@ -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);
         let span = item.span(self.tcx);
         let visibility = item.visibility(self.tcx);
         let clean::Item { name, item_id, .. } = item;
diff --git a/tests/rustdoc/inline_cross/attributes.rs b/tests/rustdoc/inline_cross/attributes.rs
new file mode 100644
index 0000000000000..c0b75c48fee91
--- /dev/null
+++ b/tests/rustdoc/inline_cross/attributes.rs
@@ -0,0 +1,7 @@
+// aux-crate:attributes=attributes.rs
+// edition:2021
+#![crate_name = "user"]
+
+// @has 'user/struct.NonExhaustive.html'
+// @has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[non_exhaustive]'
+pub use attributes::NonExhaustive;
diff --git a/tests/rustdoc/inline_cross/auxiliary/attributes.rs b/tests/rustdoc/inline_cross/auxiliary/attributes.rs
new file mode 100644
index 0000000000000..c6f155d4ba5a9
--- /dev/null
+++ b/tests/rustdoc/inline_cross/auxiliary/attributes.rs
@@ -0,0 +1,2 @@
+#[non_exhaustive]
+pub struct NonExhaustive;
diff --git a/tests/rustdoc/inline_cross/auxiliary/repr.rs b/tests/rustdoc/inline_cross/auxiliary/repr.rs
index 4a6648a643980..35f08c11b7b3a 100644
--- a/tests/rustdoc/inline_cross/auxiliary/repr.rs
+++ b/tests/rustdoc/inline_cross/auxiliary/repr.rs
@@ -10,7 +10,7 @@ pub struct ReprSimd {
 }
 #[repr(transparent)]
 pub struct ReprTransparent {
-    field: u8,
+    pub field: u8,
 }
 #[repr(isize)]
 pub enum ReprIsize {
@@ -20,3 +20,23 @@ pub enum ReprIsize {
 pub enum ReprU8 {
     Bla,
 }
+
+#[repr(transparent)] // private
+pub struct ReprTransparentPrivField {
+    field: u32, // non-1-ZST field
+}
+
+#[repr(transparent)] // public
+pub struct ReprTransparentPriv1ZstFields {
+    marker0: Marker,
+    pub main: u64, // non-1-ZST field
+    marker1: Marker,
+}
+
+#[repr(transparent)] // private
+pub struct ReprTransparentPrivFieldPub1ZstFields {
+    main: [u16; 0], // non-1-ZST field
+    pub marker: Marker,
+}
+
+pub struct Marker; // 1-ZST
diff --git a/tests/rustdoc/inline_cross/repr.rs b/tests/rustdoc/inline_cross/repr.rs
index 9e107cee9e91b..2f3d8f0038846 100644
--- a/tests/rustdoc/inline_cross/repr.rs
+++ b/tests/rustdoc/inline_cross/repr.rs
@@ -9,21 +9,32 @@ extern crate repr;
 
 // @has 'foo/struct.ReprC.html'
 // @has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(C, align(8))]'
-#[doc(inline)]
 pub use repr::ReprC;
 // @has 'foo/struct.ReprSimd.html'
 // @has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(simd, packed(2))]'
-#[doc(inline)]
 pub use repr::ReprSimd;
 // @has 'foo/struct.ReprTransparent.html'
 // @has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]'
-#[doc(inline)]
 pub use repr::ReprTransparent;
 // @has 'foo/enum.ReprIsize.html'
 // @has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(isize)]'
-#[doc(inline)]
 pub use repr::ReprIsize;
 // @has 'foo/enum.ReprU8.html'
 // @has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(u8)]'
-#[doc(inline)]
 pub use repr::ReprU8;
+
+// 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.
+
+// @has 'foo/struct.ReprTransparentPrivField.html'
+// @!has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]'
+pub use repr::ReprTransparentPrivField;
+
+// @has 'foo/struct.ReprTransparentPriv1ZstFields.html'
+// @has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]'
+pub use repr::ReprTransparentPriv1ZstFields;
+
+// @has 'foo/struct.ReprTransparentPrivFieldPub1ZstFields.html'
+// @!has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]'
+pub use repr::ReprTransparentPrivFieldPub1ZstFields;
diff --git a/tests/rustdoc/repr.rs b/tests/rustdoc/repr.rs
new file mode 100644
index 0000000000000..fbb46e126ba6a
--- /dev/null
+++ b/tests/rustdoc/repr.rs
@@ -0,0 +1,29 @@
+// 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.
+
+// @has 'repr/struct.ReprTransparentPrivField.html'
+// @!has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]'
+#[repr(transparent)] // private
+pub struct ReprTransparentPrivField {
+    field: u32, // non-1-ZST field
+}
+
+// @has 'repr/struct.ReprTransparentPriv1ZstFields.html'
+// @has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]'
+#[repr(transparent)] // public
+pub struct ReprTransparentPriv1ZstFields {
+    marker0: Marker,
+    pub main: u64, // non-1-ZST field
+    marker1: Marker,
+}
+
+// @has 'repr/struct.ReprTransparentPub1ZstField.html'
+// @has - '//*[@class="rust item-decl"]//*[@class="code-attribute"]' '#[repr(transparent)]'
+#[repr(transparent)] // public
+pub struct ReprTransparentPub1ZstField {
+    marker0: Marker,
+    pub marker1: Marker,
+}
+
+struct Marker; // 1-ZST