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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Oct 14, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/doc/rustdoc/src/advanced-features.md
View file
Open in desktop
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
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.


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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,12 709,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 @@ -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;
Expand All @@ -773,10 795,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.

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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<a href=\"\" class=\"fn\">{".len() "</a>".len());
Expand Down Expand Up @@ -1035,13 1035,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 @@ -1050,8 1050,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
})
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 { "" },
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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 {}{}",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
});

Expand Down Expand Up @@ -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}",
Expand All @@ -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}}",
Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/json/conversions.rs
View file
Open in desktop
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);
Copy link
Member Author

@fmease fmease Sep 1, 2023 •
edited
Loading

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.

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading