Skip to content

Commit

Permalink
rustdoc: hide #[repr(...)] if it isn't part of the public ABI
Browse files Browse the repository at this point in the history
  • Loading branch information
fmease committed Oct 18, 2023
1 parent ee8c9d3 commit 79bb50a
Show file tree
Hide file tree
Showing 6 changed files with 262 additions and 75 deletions.
10 changes: 8 additions & 2 deletions src/doc/rustdoc/src/advanced-features.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,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

<!-- FIXME(fmease): Fill in this section once the semantics have been decided upon. -->

### `#[repr(transparent)]`

<!-- FIXME(fmease): Extract most parts into the section above. -->

You can read more about `#[repr(transparent)]` itself in the [Rust Reference][repr-trans-ref] and
in the [Rustonomicon][repr-trans-nomicon].
Expand All @@ -124,7 +130,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
Expand Down
175 changes: 114 additions & 61 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -713,17 +713,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<String> {
const ALLOWED_ATTRIBUTES: &[Symbol] =
&[sym::export_name, sym::link_section, sym::no_mangle, sym::non_exhaustive];

use rustc_abi::IntegerType;

let mut attrs: Vec<String> = self
.attrs
.other_attrs
Expand All @@ -743,67 +741,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<String> {
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 {
Expand Down
8 changes: 4 additions & 4 deletions tests/rustdoc-gui/src/test_docs/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,10 +424,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 {
Expand Down
50 changes: 42 additions & 8 deletions tests/rustdoc/inline_cross/auxiliary/repr.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
25 changes: 25 additions & 0 deletions tests/rustdoc/inline_cross/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https:/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.
Expand Down
Loading

0 comments on commit 79bb50a

Please sign in to comment.