Stream: t-lang/wg-unsafe-code-guidelines

Topic: enum repr


nikomatsakis (Dec 19 2018 at 18:24, on Zulip):

Finally finished my write-up of enum representation:

https://github.com/rust-rfcs/unsafe-code-guidelines/pull/57

briansmith (Dec 19 2018 at 21:34, on Zulip):

@nikomatsakis I was surprised to see you propose that non-C-compatible layouts are guaranteed to have the tag at the beginning, IIUC. I have several cases where it would be better for the tag to be put at the end. In particular, when the enum has a variant with large alignment, forcing the tag to be at the beginning wastes a huge amount of space (127 bits of space, in my cases).

briansmith (Dec 19 2018 at 21:34, on Zulip):

Is it really important to guarantee the layout of these?

rkruppe (Dec 19 2018 at 21:37, on Zulip):

@briansmith Are you talking about repr(Int) on enums whose variants have fields? That was explicitly RFC'd to do what that PR documents (https://github.com/rust-lang/rfcs/pull/2195)

briansmith (Dec 19 2018 at 21:39, on Zulip):

@rkruppe I'm just talking about regular enums like enum E { A(A), B(B) } where types A and/or B have large alignment.

rkruppe (Dec 19 2018 at 21:39, on Zulip):

Huh, does the PR specify that? If so, I must have missed it

Matthew Jasper (Dec 19 2018 at 21:40, on Zulip):

How does it waste space? You still need padding at the end.

nikomatsakis (Dec 19 2018 at 21:41, on Zulip):

@briansmith I believe that I explicitly said that the layout without a #[repr] is not defined

nikomatsakis (Dec 19 2018 at 21:41, on Zulip):

if not, then I made a mistake, or perhaps I wrote it in a confusing way

nikomatsakis (Dec 19 2018 at 21:41, on Zulip):

(I did document the current behavior, which might not be worth doing)

briansmith (Dec 19 2018 at 21:42, on Zulip):

Oh, I see: I interpreted "Non-C-compatible layouts" to mean "all Non-C-compatible-layouts" but it's really only Non-C-compatible layouts that have a #[repr(Int)].

briansmith (Dec 19 2018 at 21:42, on Zulip):

I agree that everything is OK.

briansmith (Dec 19 2018 at 21:43, on Zulip):

@Matthew Jasper Whether you need padding at the end depends on the alignment of the object that follows it. That object mighty have alignment 1

Matthew Jasper (Dec 19 2018 at 21:46, on Zulip):

No? In struct A { f: i64, g: i8 }, struct B { a: A, h: i8} B must be at least 24 bytes.

briansmith (Dec 19 2018 at 21:51, on Zulip):

@Matthew Jasper Really? I thought that Rust reserved the right to not only pack things into padding, but also even reorder members of structs that don't have explicit #[repr(...)].

Matthew Jasper (Dec 19 2018 at 22:03, on Zulip):

It can reorder members, but all orders are the same here. It has to guarantee that one can write to all mem::size_of::<A>() bytes of a &mut A that refers to a.

nikomatsakis (Dec 19 2018 at 22:14, on Zulip):

Oh, I see: I interpreted "Non-C-compatible layouts" to mean "all Non-C-compatible-layouts" but it's really only Non-C-compatible layouts that have a #[repr(Int)].

I tried to make it more clear

RalfJ (Dec 20 2018 at 07:47, on Zulip):

@Matthew Jasper Really? I thought that Rust reserved the right to not only pack things into padding, but also even reorder members of structs that don't have explicit #[repr(...)].

packing into padding is impossible unfortunately; mem::swap would break: it overrides all bytes of both objects. so e.g. whatever &mut b.a points to cannot overlap with anything packed

RalfJ (Dec 20 2018 at 07:47, on Zulip):

though... I wonder if there is any chance that we could ever have a type whose size is unequal to its stride in an array?

RalfJ (Dec 20 2018 at 07:48, on Zulip):

I see no fundamental reason why such types couldn't exist, except that tons of unsafe code could rely on such types not existing...

Nicole Mazzuca (Dec 20 2018 at 09:33, on Zulip):

Yeah, it's invalid not because it's invalid as a concept, but because it would break so much code.

Nicole Mazzuca (Dec 20 2018 at 09:33, on Zulip):

Swift does this, for example

RalfJ (Dec 20 2018 at 09:42, on Zulip):

shame that we didn't do this from the start, to avoid such code :/

nikomatsakis (Dec 20 2018 at 16:31, on Zulip):

btw @Alan Jeffrey I can't tell how to respond to this on github but I don't quite understand this comment:

Which indexing types do you mean? Option<usize> takes 128 bits according to the playground.

This is because usize does not define any niche values -- all bitpatterns are valid.

RalfJ (Dec 20 2018 at 16:33, on Zulip):

I responded on GH

Alan Jeffrey (Dec 20 2018 at 16:36, on Zulip):

@nikomatsakis what @RalfJ said.

nikomatsakis (Dec 20 2018 at 16:37, on Zulip):

I can't figure out what @RalfJ said

nikomatsakis (Dec 20 2018 at 16:37, on Zulip):

GH reviews can be very weird

nikomatsakis (Dec 20 2018 at 16:37, on Zulip):

ah, I see

nikomatsakis (Dec 20 2018 at 16:37, on Zulip):

that was in the context of another thread earlier up, ok

nikomatsakis (Dec 20 2018 at 16:38, on Zulip):

no wonder it seemed so random

Alan Jeffrey (Dec 20 2018 at 16:38, on Zulip):

TIL about rustc_layout_scalar_valid_range_end

Alan Jeffrey (Dec 20 2018 at 16:38, on Zulip):

Do we want to say something about that in the discussion of niches?

nikomatsakis (Dec 20 2018 at 16:40, on Zulip):

I have been assuming that is the next area -- i.e., when we discuss validity invariants (although I agree with you it is potentially useful to peel off a "representation invariant" or "bitstring invariant" or something)

Alan Jeffrey (Dec 20 2018 at 16:41, on Zulip):

Can we peel off "niche", to make the representation section self-contained?

nikomatsakis (Dec 20 2018 at 16:42, on Zulip):

Might make sense. I'd still be a bit reluctant to discuss #[rustc_layout_scalar_valid_range_end] -- well, I guess it's ok, it's just that it's pretty wildly unstable.

Alan Jeffrey (Dec 20 2018 at 16:44, on Zulip):

Might be worth saying something vague then, about mechanisms for user-defined niches still being unstable.

RalfJ (Dec 20 2018 at 16:45, on Zulip):

I think we want to merge the rep + invariant documents eventually

RalfJ (Dec 20 2018 at 16:45, on Zulip):

but for discussion, niche is really where rep exploits the invariant

RalfJ (Dec 20 2018 at 16:45, on Zulip):

if we peel off that we might as well merge the discussions

nikomatsakis (Dec 20 2018 at 16:46, on Zulip):

btw, I think that enums are a clear use-case for some form of "first-field-has-zero-offset" #[repr]

nikomatsakis (Dec 20 2018 at 16:46, on Zulip):

it is silly that we use #[repr(C)] on all the substructs and hency fully order the fields of the variants

nikomatsakis (Dec 20 2018 at 16:46, on Zulip):

when all we need is the ability to read the discriminant

RalfJ (Dec 20 2018 at 16:46, on Zulip):

there are so many possible repr

Alan Jeffrey (Dec 20 2018 at 16:46, on Zulip):

@RalfJ Would merging rep + invariant end up putting concepts like lifetimes etc. into the rep chapter?

nikomatsakis (Dec 20 2018 at 16:46, on Zulip):

there are so many possible repr

I don't think this is a useful argument against any individual one

RalfJ (Dec 20 2018 at 16:47, on Zulip):

validity invariants have nothing to do with lifetimes IMO

nikomatsakis (Dec 20 2018 at 16:47, on Zulip):

but I suppose this is worth having a separate topic, and anyway it's not that imp't, the other reprs don't exist now

RalfJ (Dec 20 2018 at 16:47, on Zulip):

there are so many possible repr

I don't think this is a useful argument against any individual one

sure, but we could instead have a "most general" one? have an attribute at each field where the user gives an offset

nikomatsakis (Dec 20 2018 at 16:47, on Zulip):

that's fine

RalfJ (Dec 20 2018 at 16:47, on Zulip):

hm I guess we'd need a DSL to compute the offset based on size+alignment of other types... yeah that'll be complicated^^

RalfJ (Dec 20 2018 at 16:48, on Zulip):

@RalfJ Would merging rep + invariant end up putting concepts like lifetimes etc. into the rep chapter?

as far as I can see, merging them leaves nothing to be said for validity invariants

RalfJ (Dec 20 2018 at 16:48, on Zulip):

that's why I think we shouldn't

gnzlbg (Dec 21 2018 at 09:02, on Zulip):

@Nicole Mazzuca @RalfJ would it be possible to start linting such code ?

RalfJ (Dec 21 2018 at 09:03, on Zulip):

which code?

gnzlbg (Dec 21 2018 at 09:34, on Zulip):

packing into padding is impossible unfortunately; mem::swap would break: it overrides all bytes of both objects. so e.g. whatever &mut b.a points to cannot overlap with anything packed

gnzlbg (Dec 21 2018 at 09:34, on Zulip):

i wish zulip would allow replying to messages within a thread, linking the message that its being replied to (EDIT: now there is an issue for that: https://github.com/zulip/zulip/issues/11105)

Nicole Mazzuca (Dec 21 2018 at 09:49, on Zulip):

absolutely zero way

RalfJ (Dec 21 2018 at 14:37, on Zulip):

yeah no, that just breaks what references mean. I don't think it can be linted, even dynamically checking for it might be hard.

nikomatsakis (Dec 26 2018 at 16:50, on Zulip):

Are we feeling ok about the current state of https://github.com/rust-rfcs/unsafe-code-guidelines/pull/57 ?

Last update: Nov 20 2019 at 13:25UTC