Skip to content

Commit

Permalink
Rollup merge of #115439 - fmease:rustdoc-priv-repr-transparent-heuris…
Browse files Browse the repository at this point in the history
…tic, r=GuillaumeGomez

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

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
  • Loading branch information
matthiaskrgr authored Oct 14, 2023
2 parents 0233608 + 64fa12a commit 4dd4d9b
Show file tree
Hide file tree
Showing 10 changed files with 152 additions and 42 deletions.
20 changes: 20 additions & 0 deletions src/doc/rustdoc/src/advanced-features.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
51 changes: 36 additions & 15 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -713,12 +713,16 @@ impl Item {
Some(tcx.visibility(def_id))
}

pub(crate) fn attributes(&self, tcx: TyCtxt<'_>, keep_as_is: bool) -> Vec<String> {
pub(crate) fn attributes(
&self,
tcx: TyCtxt<'_>,
cache: &Cache,
keep_as_is: bool,
) -> Vec<String> {
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<String> = self
.attrs
Expand All @@ -739,20 +743,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;
Expand All @@ -777,10 +799,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
}
Expand Down
16 changes: 8 additions & 8 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -868,10 +868,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 + "<a href=\"\" class=\"fn\">{".len() + "</a>".len());
Expand Down Expand Up @@ -1047,13 +1047,13 @@ fn render_assoc_item(

// When an attribute is rendered inside a `<pre>` 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(())
Expand All @@ -1062,8 +1062,8 @@ fn render_attributes_in_pre<'a, 'b: 'a>(

// 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, 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, "<div class=\"code-attribute\">{attr}</div>").unwrap();
}
}
Expand Down
23 changes: 11 additions & 12 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,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}")
})
}
Expand Down Expand Up @@ -659,7 +658,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,
Expand Down Expand Up @@ -694,7 +693,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 { "" },
Expand Down Expand Up @@ -1173,7 +1172,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),
Expand Down Expand Up @@ -1201,7 +1200,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),
Expand All @@ -1226,7 +1225,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),
Expand Down Expand Up @@ -1415,7 +1414,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 {}{}",
Expand Down Expand Up @@ -1734,7 +1733,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,
Expand Down Expand Up @@ -1783,7 +1782,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);
});

Expand Down Expand Up @@ -1843,7 +1842,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}",
Expand All @@ -1861,7 +1860,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}}",
Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/json/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions tests/rustdoc/inline_cross/attributes.rs
Original file line number Diff line number Diff line change
@@ -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;
2 changes: 2 additions & 0 deletions tests/rustdoc/inline_cross/auxiliary/attributes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#[non_exhaustive]
pub struct NonExhaustive;
22 changes: 21 additions & 1 deletion tests/rustdoc/inline_cross/auxiliary/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub struct ReprSimd {
}
#[repr(transparent)]
pub struct ReprTransparent {
field: u8,
pub field: u8,
}
#[repr(isize)]
pub enum ReprIsize {
Expand All @@ -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
21 changes: 16 additions & 5 deletions tests/rustdoc/inline_cross/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/rust-lang/rust/issues/90435>.
// 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;
29 changes: 29 additions & 0 deletions tests/rustdoc/repr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Regression test for <https://github.com/rust-lang/rust/issues/90435>.
// 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

0 comments on commit 4dd4d9b

Please sign in to comment.