Hide #[repr(transparent)] where the field is non-public

@dtolnay

Note that repr(transparent) is not the same as permission to inspect or manipulate the internal representation of a type. That permission only exists if the field is also exposed as pub.

As such, we should consider hiding #[repr(transparent)] when the field is not otherwise visible. This should not be the case when --document-private-items is passed for reasons that should be obvious.

This will affect some stdlib structs such as NonZeroU8

@rustbot label +A-docs +C-enhancement +T-rustdoc

@jhpratt

Is there consensus that this is a good thing to do? I understand that the thing that @jhpratt wanted to do with PathBuf was advised against, but for types that already have repr(transparent) like NonZeroU8, why shouldn't it be advertised? Is there no scenario where a user both relies on the fact that a type is repr(transparent) and doesn't do anything "impermissible" with it?

The reason I was advised against it is because the attribute is only meaningful if you are able to know what the inner type is. While something like NonZeroU8 is logically represented as u8, there's nothing that strictly days that's the case. This is the quote I included.

The #[repr(transparent)] doesn't give a user sufficient promises to do anything. So why show it at all?

The #[repr(transparent)] doesn't give a user sufficient promises to do anything.

I mean, that's not true, is it? It makes some very explicit promises. Consider UnsafeCell. We have tons of code where I work that wraps up thread-safe C types in UnsafeCell so they can be accessed from multiple threads in Rust. When those types cross the FFI boundary, it seems critical for there to be a guarantee that the memory layout will be the same so we can transmute a value or reference to the C type into its Rust equivalent:

struct ffi_foo;

#[repr(transparent)]
pub struct Foo {
    inner: UnsafeCell<ffi_foo>,
}

impl Foo {
    pub fn go_kazoo(&self) {
        unsafe { ffi_foo_go_kazoo(self.inner.get()) }
    }
}

impl<'a> From<&'a ffi_foo> for &'a Foo {
    fn from(x: &'a ffi_foo) -> Self {
        // SAFETY: types have identical layout and ffi_foo is thread-safe
        unsafe { std::mem::transmute(x) }
    }
}

Maybe that example is misguided, but it still stands that repr(transparent) gives meaningful guarantees, right?

@dtolnay

Ignoring the fact that the example you provided is unsound, I see no reference to the inner type of UnsafeCell in its documentation. As a result I'm deferring to the comment by @dtolnay who knows more about this than me. He's indicated that barring the field being public it cannot be relied upon.

Which part is unsound?

Ignoring the details of my example, though, here's the Nomicon regarding repr(transparent):

The goal is to make it possible to transmute between the single field and the struct. An example of that is UnsafeCell, which can be transmuted into the type it wraps.

Also, passing the struct through FFI where the inner field type is expected on the other side is guaranteed to work. In particular, this is necessary for struct Foo(f32) to always have the same ABI as f32.

@jhpratt

Yeah @jhpratt is correct. Just a repr(transparent) attribute does not give any public-facing guarantee when the field isn't pub.

Individual data structures may document a commitment that their layout may be publicly relied on, but that is simply not repr(transparent). Currently this kind of thing is just written in prose but this came up as well in rust-lang/unsafe-code-guidelines#302 (comment) about whether there should be some machine-readable way.

Okay I see what you're saying. To put it in my own words, the set of all repr(transparent) types in the standard library can be split into two categories: "publicly transparent" (e.g., UnsafeCell, NonZero*) and "incidentally transparent" (e.g., PathBuf).

The unfortunate thing is that this split is currently not documented very well if at all. And as shown above, the Nomicon describes repr(transparent) as being equivalent to "publicly transparent." So at the very least, it seems that the Nomicon needs to be amended if this change is made.

But given that the Nomicon is written how it is, it seems possible that there are already crates that elide any additional prose and rely solely on the presence of repr(transparent) in their rustdocs to communicate that a type is "publicly transparent." So if this change is made without notification, perhaps existing documentation will lose information.

I disagree that the nomicon's description refers to publicly transparent. It's describing guarantees that the language makes to the module/crate that contains the type. Nothing in there is about what guarantees the module/crate makes to its downstream users.

Hm, maybe not explicitly. But when I read it, it seems strongly implied.

The goal is to make it possible to transmute between the single field and the struct. An example of that is UnsafeCell, which can be transmuted into the type it wraps.

Nothing on that page, nor in the parent page, nor even in the Reference page on layouts, suggests to me that these rules only apply to the module/crate where the type is defined. My own intuition additionally tells me that the layout of a type is a property that is true for any crate in which the type is used.

Perhaps there's some detail that I'm missing when I read it, but really, if this is an important distinction, then it should be made more clearly. I'm more than happy to have my misconceptions ironed out through discussion, but I would wager that other noobs like me will walk away thinking the exact same things given the current text, and that's the real problem to me.

I would wager that other noobs like me will walk away thinking the exact same things given the current text, and that's the real problem to me.

To me that's precisely why this issue was created and should be resolved.

Yeah, it's bad. It falls out of the way cross-crate inlining and compiling works.

Fixing it is actually going to be pretty complicated. I couldn't find a bug report for it (not very many people use cfg(doc) and not very many people use cross-crate inlining, so I suppose they don't hit it much).

#114952

@fmease

@fmease wrote

when working on a fix for #90435 (which is now sadly blocked on #114952 I guess), ...

I don't think this is blocked on #114952. Just the easy hack of using #[cfg_attr(not(doc), repr(transparent))] sadly doesn't entirely work, so a proper implementation in rustdoc might be needed.

Seems like I misinterpreted your comment. I'm gonna pick up the work on it again then.

I've now opened #115439.

Rollup merge of rust-lang#115439 - fmease:rustdoc-priv-repr-transpare…

Unrolled build for rust-lang#115439

Fix some `#[cfg_attr(not(doc), repr(..))]`

Rollup merge of rust-lang#128046 - GrigorenkoPV:90435, r=tgross35

Unrolled build for rust-lang#128046

Rollup merge of #107680 - dtolnay:docrepr, r=Amanieu

Navigation Menu

Repository navigation

Hide `#[repr(transparent)]` where the field is non-public #90435

40 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Hide #[repr(transparent)] where the field is non-public #90435

Description

Activity

BGR360 commented on Nov 27, 2021

jhpratt commented on Nov 27, 2021

BGR360 commented on Nov 27, 2021

jhpratt commented on Nov 27, 2021

BGR360 commented on Nov 27, 2021

dtolnay commented on Nov 27, 2021

BGR360 commented on Nov 27, 2021

dtolnay commented on Nov 27, 2021

BGR360 commented on Nov 27, 2021

jhpratt commented on Nov 27, 2021

40 remaining items

notriddle commented on Aug 17, 2023

RalfJung commented on Aug 29, 2023

fmease commented on Aug 29, 2023

fmease commented on Sep 1, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Hide `#[repr(transparent)]` where the field is non-public #90435