rust-lang / unsafe-code-guidelines

Forum for discussion about what unsafe code can and can't do
https://rust-lang.github.io/unsafe-code-guidelines
Apache License 2.0
667 stars 58 forks source link

Does the repr(transparent) guarantee work for fields that are empty types? #485

Open Manishearth opened 11 months ago

Manishearth commented 11 months ago

Context: https://github.com/hyperium/hyper/issues/3500

The repr(transparent) RFC states:

his attribute can be placed on newtypes, i.e. structures (and structure tuples) with a single field, and on structures that are logically equivalent to a newtype, i.e. structures with multiple fields where only a single one of them has a non-zero size.

The reference states:

The transparent representation can only be used on a struct or an enum with a single variant that has:

  • a single field with non-zero size, and
  • any number of fields with size 0 and alignment 1 (e.g. PhantomData<T>).

Structs and enums with this representation have the same layout and ABI as the single non-zero sized field.

It's unclear if this applies to situations where a contained type is an empty type (enum Empty {})

For example, what are the implications of this?

#[repr(transparent)]
struct Foo(String, Empty);

enum Empty {}

From the RFC, this type is not "logically equvialent to a newtype", and repr(transparent) should not apply. From the reference, the other field is size 0 alignment 1, so repr(transparent) should make the repr the same as String (I believe this is the current behavior in rustc), but it does feel like an edge case.

It would be nice to have this explicitly documented, rather than inferred.

RalfJung commented 10 months ago

That's a good question. I don't even know what the intention is here; this needs to be brought up with the lang team. (This is not a t-opsem question.)

Currently, we give Foo the Uninhabited ABI, so this does not act like a proper newtype. So the reality today is that repr(transparent) does not work when one of the 1-ZST is an empty type, but the compiler fails to diagnose this properly.

chorman0773 commented 10 months ago

It probably does not matter, though, because the ABI for an empty type is "Does not exist", and even if it did match ABI, it would be immediate UB to call a function with Foo at either the call- or def-site.

RalfJung commented 10 months ago

However this might impact layout, e.g. Result<(), Foo> vs Result<(), String> might be different if enum discriminant encoding takes into account which variants are inhabited.

Manishearth commented 10 months ago

@chorman0773 it does matter when determining the impact of the transparent type on the repr of an enum wrapping it

though that also gets us into questions of transitivity https://github.com/rust-lang/unsafe-code-guidelines/issues/486

chorman0773 commented 10 months ago

However this might impact layout, e.g. Result<(), Foo> vs Result<(), String> might be different if enum discriminant encoding takes into account which variants are inhabited.

#[repr(transparent)] types don't necessarily propagate internal layout anyways. The trivial case is that UnsafeCell<T> and MaybeUninit<T> is (by-spec) #[repr(transparent)] over T, but Option<T> and Option<UnsafeCell<T>>/Option<MaybeUninit<T>> aren't the same layout. Also works in reverse, NonZero{U,I}* are all transparent over the corresponding {u,i}* type by spec, but Option<NonZeroU32> is not the same layout as Option<u32>

RalfJung commented 10 months ago

They don't always propagate, true. (NonZero are not just transparent though, they also add a niche with special magic, so they are out-of-scope here.)

But there's no fundamental reason why they shouldn't propagate through enum. We anyway can't say this propagates through arbitrary wrappers since Wrapper<T> may use a trait bound on T and then use associated types and then obviously things can be arbitrarily different.

Manishearth commented 10 months ago

#[repr(transparent)] types don't necessarily propagate internal layout anyways.

Oh, that's something that probably ought to be documented carefully.

But there's no fundamental reason why they shouldn't propagate through enum. We anyway can't say this propagates through arbitrary wrappers since Wrapper<T> may use a trait bound on T and then use associated types and then obviously things can be arbitrarily different.

This would be a good guarantee to pin down!