Untagged unions (tracking issue for RFC 1444)
nikomatsakis opened this issue · 210 comments
Tracking issue for rust-lang/rfcs#1444.
Unresolved questions:
- Does assigning directly to a union field trigger a drop of the previous contents?
- When moving out of one field of a union, are the others considered invalidated? (1, 2, 3, 4)
- Answered by rust-lang/rfcs#2514.
- Under what conditions can you implement
Copyfor a union? For example, what if some variants are of non-Copy type? All variants? - What interaction is there between unions and enum layout optimizations? (#36394)
Open issues of high import:
- #47412 -- MIR-based unsafety checker sometimes accepts unsafe accesses to union fields in presence of uninhabited fields
I may have missed it in the discussion on the RFC, but am I correct in thinking that destructors of union variants are never run? Would the destructor for the Box::new(1) run in this example?
union Foo {
f: i32,
g: Box<i32>,
}
let mut f = Foo { g: Box::new(1) };
f.g = Box::new(2);
@sfackler My current understanding is that f.g = Box::new(2) will run the destructor but f = Foo { g: Box::new(2) } would not. That is, assigning to a Box<i32> lvalue will cause a drop like always, but assigning to a Foo lvalue will not.
So an assignment to a variant is like an assertion that the field was previously "valid"?
@sfackler For Drop types, yeah, that's my understanding. If they weren't previously valid you need to use the Foo constructor form or ptr::write. From a quick grep, it doesn't seem like the RFC is explicit about this detail, though. I see it as an instantiation of the general rule that writing to a Drop lvalue causes a destructor call.
Should a &mut union with Drop variants be a lint?
On Friday, 8 April 2016, Scott Olson notifications@github.com wrote:
@sfackler https://github.com/sfackler For Drop types, yeah, that's my
understanding. If they weren't previously valid you need to use the Foo
constructor form or ptr::write. From a quick grep, it doesn't seem like
the RFC is explicit about this detail, though.—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#32836 (comment)
On April 8, 2016 3:36:22 PM PDT, Scott Olson notifications@github.com wrote:
@sfackler For
Droptypes, yeah, that's my understanding. If they
weren't previously valid you need to use theFooconstructor form or
ptr::write. From a quick grep, it doesn't seem like the RFC is
explicit about this detail, though.
I should have covered that case explicitly. I think both behaviors are defensible, but I think it'd be far less surprising to never implicitly drop a field. The RFC already recommends a lint for union fields with types that implement Drop. I don't think assigning to a field implies that field was previously valid.
Yeah, that approach seems a bit less dangerous to me as well.
Not dropping when assigning to a union field would make f.g = Box::new(2) act differently from let p = &mut f.g; *p = Box::new(2), because you can't make the latter case not drop. I think my approach is less surprising.
It's not a new problem, either; unsafe programmers already have to deal with other situations where foo = bar is UB if foo is uninitialized and Drop.
I personally don't plan to use Drop types with unions at all. So I'll defer entirely to people who have worked with analogous unsafe code on the semantics of doing so.
I also don't intend to use Drop types in unions so either way doesn't matter to me as long as it is consistent.
I don't intend to use mutable references to unions, and probably
just "weirdly-tagged" ones with Into
On Friday, 8 April 2016, Peter Atashian notifications@github.com wrote:
I also don't intend to use Drop types in unions so either way doesn't
matter to me as long as it is consistent.—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#32836 (comment)
Seems like this is a good issue to raise up as an unresolved question. I'm not sure yet which approach I prefer.
@nikomatsakis As much as I find it awkward for assigning to a union field of a type with Drop to require previous validity of that field, the reference case @tsion mentioned seems almost unavoidable. I think this might just be a gotcha associated with code that intentionally disables the lint for putting a type with Drop in a union. (And a short explanation of it should be in the explanatory text for that lint.)
And I'd like to reiterate that unsafe programmers must already generally know that a = b means drop_in_place(&mut a); ptr::write(&mut a, b) to write safe code. Not dropping union fields would be one more exception to learn, not one less.
(NB: the drop doesn't happen when a is statically known to already be uninitialized, like let a; a = b;.)
But I support having a default warning against Drop variants in unions that people have to #[allow(..)] since this is a fairly non-obvious detail.
@tsion this is not true for a = b and maybe only sometimes true for a.x = b but it is certainly true for *a = b. This uncertainty is what made me hesitant about it. For example, this compiles:
fn main() {
let mut x: (i32, i32);
x.0 = 2;
x.1 = 3;
}(though trying to print x later fails, but I consider that a bug)
@nikomatsakis That example is new to me. I guess I would have considered it a bug that that example compiles, given my previous experience.
But I'm not sure I see the relevance of that example. Why is what I said not true for a = b and only sometimes for a.x = b?
Say, if x.0 had a type with a destructor, surely that destructor is called:
fn main() {
let mut x: (Box<i32>, i32);
x.0 = Box::new(2); // x.0 statically know to be uninit, destructor not called
x.0 = Box::new(3); // x.0 destructor is called before writing new value
}Maybe just lint against that kind of write?
My point is only that = does not always run the destructor; it
uses some knowledge about whether the target is known to be
initialized.
On Tue, Apr 12, 2016 at 04:10:39PM -0700, Scott Olson wrote:
@nikomatsakis That example new to me. I guess I would have considered it a bug that that example compiles, given my previous experience.
But I'm not sure I see the relevance of that example. Why is what I said not true for
a = band only sometimes for 'a.x = b'?Say, if
x.0had a type with a destructor, surely that destructor is called:fn main() { let mut x: (Box<i32>, i32); x.0 = Box::new(2); // x.0 statically know to be uninit, destructor not called x.0 = Box::new(3); // x.0 destructor is called }
It runs the destructor if the drop flag is set.
But I think that kind of write is confusing anyway, so why not just forbid it? You can always do *(&mut u.var) = val.
My point is only that
=does not always run the destructor; it uses some knowledge about whether the target is known to be initialized.
@nikomatsakis I already mentioned that:
(NB: the drop doesn't happen when a is statically known to already be uninitialized, like let a; a = b;.)
But I didn't account for dynamic checking of drop flags, so this is definitely more complicated than I considered.
Drop flags are only semi-dynamic - after zeroing drop is gone, they are a part of codegen. I say we forbid that kind of write because it does more confusion than good.
Should Drop types even be allowed in unions? If I'm understanding things correctly, the main reason to have unions in Rust is to interface with C code that has unions, and C doesn't even have destructors. For all other purposes, it seems that it's better to just use an enum in Rust code.
There is a valid use case for using a union to implement a NoDrop type which inhibits drop.
As well as invoking such code manually via drop_in_place or similar.
To me dropping a field value while writing to it is definitely wrong because the previous option type is undefined.
Would it be possible to prohibit field setters but require full union replacement? In this case if the union implements Drop full union drop would be called for the value replaced as expected.
I don't think it makes sense to prohibit field setters; most uses of unions should have no problem using those, and fields without a Drop implementation will likely remain the common case. Unions with fields that implement Drop will produce a warning by default, making it even less likely to hit this case accidentally.
For the sake of discussion, I intend to expose mutable references to fields in unions and put arbitrary (possibly Drop) types into them. Basically, I would like to use unions to write custom space-efficient enums. For example,
union SlotInner<V> {
next_empty: usize, /* index of next empty slot */
value: V,
}
struct Slot<V> {
inner: SlotInner<V>,
version: u64 /* even version -> is_empty */
}@nikomatsakis I'd like to propose a concrete answer to the question currently listed as unresolved here.
To avoid unnecessarily complex semantics, assigning to a union field should act like assigning to a struct field, which means dropping the old contents. It's easy enough to avoid this if you know about it, by assigning to the whole union instead. This is still slightly surprising behavior, but having a union field that implements Drop at all will produce a warning, and the text of that warning can explicitly mention this as a caveat.
Would it make sense to provide an RFC pull request amending RFC1444 to document this behavior?
@joshtriplett Since @nikomatsakis is away on vacation, I'll answer: I think it's great form to file an amendment RFC for resolving questions like this. We'd often fast-track such RFC PRs when appropriate.
@aturon Thanks. I've filed the new RFC PR rust-lang/rfcs#1663 with these clarifications.to RFC1444, to resolve this issue.
I have some preliminary implementation in https://github.com/petrochenkov/rust/tree/union.
Status: Implemented (modulo bugs), PR submitted (#36016).
@petrochenkov Awesome! Looks great so far.
I'm not quite sure how to treat unions with non-Copy fields in move checker.
Suppose u is an initialized value of union U { a: A, b: B } and now we move out of one of the fields:
-
A: !Copy, B: !Copy, move_out_of(u.a)
This is simple,u.bis also put into uninitialized state.
Sanity check:union U { a: T, b: T }should behave exactly likestruct S { a: T }+ field alias. -
A: Copy, B: !Copy, move_out_of(u.a)
Supposedlyu.bshould still be initialized, becausemove_out_of(u.a)is simply amemcpyand doesn't changeu.bin any way. -
A: !Copy, B: Copy, move_out_of(u.a)
This is the strangest case; supposedlyu.bshould also be put into uninitialized state despite beingCopy.Copyvalues can be uninitialized (e.g.let a: u8;), but changing their state from initialized to uninitialized is something new, AFAIK.
@retep998
I know this is completely irrelevant to FFI needs :)
The good news is that it's not a blocker, I'm going to implement whatever behavior is simpler and submit PR this weekend.
@petrochenkov my instinct is that unions are a "bit-bucket", essentially. You are responsible for tracking whether the data is initialized or not and what it's true type is. This is very similar to the referent of a raw pointer.
This is why we can't drop the data for you, and also why any access to the fields is unsafe (even if, say, there is only one variant).
By these rules, I would expect unions to implement Copy if copy is implemented for them. Unlike structs/enums, however, there would be no internal sanity checks: you can always implement copy for a union type if you like.
Let me give some examples to clarify:
union Foo { ... } // contents don't matterThis union is affine, because Copy has not been implemented.
union Bar { x: Rc<String> }
impl Copy for Bar { }
impl Clone for Bar { fn clone(&self) -> Self { *self } }This union type Bar is copy, because Copy has been implemented.
Note that if Bar were a struct, it would be an error to implement Copy because of the type of the field x.
Huh, I guess I'm not actually answering your question though, now that I re-read it. =)
OK, so, I realize I was not answering your question at all. So let me try again. Following the "bit bucket" principle, I would still expect that we can move out from a union at will. But of course another option would be to treat it like we treat a *mut T, and require you to use ptr::read to move out.
EDIT: I'm not actually entire sure why we would prohibit such moves. It might have had to do w/ moving drop -- or perhaps just because it's easy to make a mistake and it seems better to make "moves out" more explicit? I am having trouble remembering the history here.
my instinct is that unions are a "bit-bucket", essentially.
Ha, I, on the contrary, would like to give as many guarantees about union's content as we can for such a dangerous construct.
The interpretation is that union is an enum for which we don't know the discriminant, i.e. we can guarantee that at any moment of time at least one of union's variants has valid value (unless unsafe code is involved).
All the borrow/move rules in the current implementation support this guarantee, simultaneously this is the most conservative interpretation, that allows us to go either the "safe" way (for example, allowing safe access to unions with equally typed fields, this can be useful) or the "bit bucket" way in the future, when more experience with Rust unions is gathered.
Actually, I'd like to make it even more conservative as described in #36016 (comment)
The interpretation is that union is an enum for which we don't know the discriminant, i.e. we can guarantee that at any moment of time at least one of union's variants has valid value (unless unsafe code is involved).
Note that unsafe code is always involved, when working with a union, since every access to a field is unsafe.
The way I think of it is, I think, similar. Basically a union is like an enum but it can be in more than one variant simultaneously. The set of valid variants is not known to the compiler at any point, though sometimes we can figure out that the set is empty (i.e., the enum is uninitialized).
So I see any use of some_union.field as basically an implicit (and unsafe) assertion that the set of valid variants currently includes field. This seems compatible with how the borrow-checker integration works; if you borrow field x and then try to use y, you are getting an error because you are basically saying that the data is simultaneously x and y (and it is borrowed). (In contrast, with a regular enum, it is not possible to inhabit more than one variant at a time, and you can see this in how the borrowck rules play out).
Anyway, the point is, when we "move" from one field of a union, the question at hand I guess is whether we can deduce that this implies that interpreting the value as the other variants is no longer valid. I think it'd be not so hard to argue either way, though. I consider this a grey zone.
The danger of being conservative is that we might well rule out unsafe code that would otherwise make sense and be valid. But I'm ok with starting out tighter and deciding whether to loosen later.
We should discuss the matter of what conditions are needed to implement Copy on a union -- also, we should make sure we have a complete list of these grey areas listed above to make sure we address and document before stabilization!
Basically a union is like an enum but it can be in more than one variant simultaneously.
One argument against the "more than one variant" interpretation is how unions behave in constant expressions - for these unions we always know the single active variant and also can't access inactive variants because transmuting at compile time is generally bad (unless we are trying to turn the compiler into some kind of partial target emulator).
My interpretation is that at runtime inactive variants are still inactive but can be accessed if they are layout compatible with the union's active variant (more restrictive definition) or rather with union's fragment assignment history (more vague, but more useful).
we should make sure we have a complete list of these grey areas
I'm going to amend the union RFC in some not-so-remote future! The "enum" interpretation has pretty fun consequences.
transmuting at compile time is generally bad (unless we are trying to turn the compiler into some kind of partial target emulator)
@petrochenkov This is one of the goals of my Miri project. Miri can already do transmutes and various raw pointer shenanigans. It would be a small amount of work to make Miri handle unions (nothing new on the raw memory handling side).
And @eddyb is pushing to replace rustc constant evaluation with a version of Miri.
One argument against the "more than one variant" interpretation is how unions behave in constant expressions...
How to best support the use of unions in constants is an interesting question, but I see no problem with restricting constant expressions to a subset of runtime behavior (this is what we always do, anyhow). That is, just because we may not be able to fully support some particular transmute at compilation time doesn't mean it's illegal at runtime.
My interpretation is that at runtime inactive variants are still inactive but can be accessed if they are layout compatible with the union's active variant
Hmm, I'm trying to think how this is different from saying that the union belongs to all of those variants simultaneously. I don't really see a difference yet. :)
I feel like this interpretation has odd interactions with moves in general. For example, if the data is "really" an X, and you interpret it as a Y, but Y is affine, then is it still an X?
Regardless, I think it's fine that having a move of any field consume the entire union can be seen as consistent with any of these interpretations. For example, in the "set of variants" approach, the idea is just that moving the value deinitializes all existing variants (and of course the variant you used must be one of the valid set). In your version, it would seem to "transmute" into that variant (and consume the original).
I'm going to amend the union RFC in some not-so-remote future! The "enum" interpretation has pretty fun consequences.
Such confidence! You're going to try ;)
Care to shed a few more details on what concrete changes you have in mind?
Care to shed a few more details on what concrete changes you have in mind?
More detailed description of the implementation (i.e. better documentation), some small extensions (like empty unions and .. in union patterns), two main (contradicting) alternatives of union evolution - more unsafe and less restrictive "scratch space" interpretation and more safe and more restrictive "enum with unknown discriminant" interpretation - and their consequences for move/initialization checker, Copy impls, unsafety of field access, etc.
It would also be useful to define when accessing an inactive union field is UB, e.g.
union U { a: u8, b: () }
let u = U { b: () };
let a = u.a; // most probably an UB, equivalent to reading from `mem::uninitialized()`
but this is an infinitely tricky area.
Sounds likely, cross-field semantics are basically a pointer cast right?
(() as *u8)
On Thursday, 1 September 2016, Vadim Petrochenkov notifications@github.com
wrote:
It would also be useful to define when accessing an inactive union field
is UB, e.g.union U { a: u8, b: () }
let u = U { b: () };
let a = u.a; // most probably an UB, equivalent to reading frommem::uninitialized()but this is an infinitely tricky area.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#32836 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABxXhi68qRITTFW5iJn6omZQQBQgzweNks5qlw4qgaJpZM4IDXsj
.
Isn't field access always unsafe?
On Thursday, 1 September 2016, Vadim Petrochenkov notifications@github.com
wrote:
Care to shed a few more details on what concrete changes you have in mind?
More detailed description of the implementation (i.e. better
documentation), some small extensions (like empty unions and .. in union
patterns), two main (contradicting) alternatives of union evolution - more
unsafe and less restrictive "scratch space" interpretation and more safe
and more restrictive "enum with unknown discriminant" interpretation - and
their consequences for move/initialization checker, Copy impls, unsafety
of field access, etc.—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#32836 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABxXhuHStN8AFhR3KYDU27U29MiMpN5Bks5qlws9gaJpZM4IDXsj
.
Isn't field access always unsafe?
It can be made safe sometimes, e.g.
- assignment to trivially-destructible union fields is safe.
- any access to fields of a
union U { f1: T, f2: T, ..., fN: T }(i.e. all fields have the same type) is safe in "enum with unknown discriminant" interpretation.
It seems better not to apply special conditions to this, from a user point of view. Just call it unsafe, always.
Currently testing out the support for unions in the latest rustc from git. Everything I've tried works perfectly.
I ran into an interesting case in the dead field checker. Try the following code:
#![feature(untagged_unions)]
union U {
i: i32,
f: f32,
}
fn main() {
println!("{}", std::mem::size_of::<U>());
let u = U { f: 1.0 };
println!("{:#x}", unsafe { u.i });
}You'll get this error:
warning: struct field is never used: `f`, #[warn(dead_code)] on by default
Looks like the dead_code checker didn't notice the initialization.
(I already filed PR #36252 about the use of "struct field", changing it to just "field".)
Unions cannot currently contain dynamically sized fields, but the RFC doesn't specify this behavior either way:
#![feature(untagged_unions)]
union Foo<T: ?Sized> {
value: T,
}Output:
error[E0277]: the trait bound `T: std::marker::Sized` is not satisfied
--> <anon>:4:5
|
4 | value: T,
| ^^^^^^^^ trait `T: std::marker::Sized` not satisfied
|
= help: consider adding a `where T: std::marker::Sized` bound
= note: only the last field of a struct or enum variant may have a dynamically sized type
Contextual keyword does not work outside the module/crate root context:
fn main() {
// all work
struct Peach {}
enum Pineapple {}
trait Mango {}
impl Mango for () {}
type Strawberry = ();
fn woah() {}
mod even_modules {
union WithUnions {}
}
use std;
// does not work
union Banana {}
}Seems like a pretty nasty consistency wart.
@nagisa
Are you using some older version of rustc by accident?
I've just checked your example on playpen and it works (modulo "empty union" errors).
There's also a run-pass test checking for this specific situation - https://github.com/rust-lang/rust/blob/master/src/test/run-pass/union/union-backcomp.rs.
@petrochenkov ah, I used play.rlo, but it seems like it might have reverted to stable or something. Never mind me, then.
I think unions will eventually need to support safe fields, the evil twins of unsafe fields from this proposal.
cc rust-lang/rfcs#381 (comment)
I do think it would make sense to have a way of declaring "safe unions", based on various criteria.
For instance, a union containing exclusively Copy non-Drop fields, all with the same size, seems safe; no matter how you access the fields, you might get unexpected data, but you can't encounter a memory safety problem or undefined behavior.
@joshtriplett You would also need to make sure there are no "holes" in the types from which you could read uninitialized data. As I like to say: "Uninitialized data is either unpredictable random data or your SSH private key, whichever is worse."
Isn't &T Copy and non-Drop? Put that in a "safe" union with usize, and you've got a rogue reference generator. So the rules will have to be a bit more strict than that.
For instance, a union containing exclusively Copy non-Drop fields, all with the same size, seems safe; no matter how you access the fields, you might get unexpected data, but you can't encounter a memory safety problem or undefined behavior.
I realize this is just an off-the-cuff example, not a serious proposal, but here are some examples to illustrate how tricky this is:
u8andboolhave the same size, but mostu8values are invalid forbooland ignoring this triggers UB&Tand&Uhave the same size and areCopy + !Dropfor allTandU(as long as both or neither areSized)- transmuting between
uN/iNandfNis currently only possible in unsafe code. I believe such transmutes are always safe but this does enlarge the safe language so it may be controverial. - Violating privacy (e.g. punning between
struct Foo(Bar);andBar) is a big no no as wel, since privacy might be used to uphold invariants relevant to safety.
@Amanieu When I was writing that, I'd meant to include a note about not having any internal padding, and somehow forgot to do so. Thanks for catching that.
@cuviper I was trying to define "plain old data", as in something that contains zero pointers. You're right, the definition would need to exclude references. Probably easier to whitelist a set of allowed types and combinations of those types.
u8 and bool have the same size, but most u8 values are invalid for bool and ignoring this triggers UB
Good point; the same issue applies to enums.
&T and &U have the same size and are Copy + !Drop for all T and U (as long as both or neither are Sized)
I'd forgotten that.
transmuting between uN/iN and fN is currently only possible in unsafe code. I believe such transmutes are always safe but this does enlarge the safe language so it may be controverial.
Agreed on both points; this seems acceptable to allow.
Violating privacy (e.g. punning between struct Foo(Bar); and Bar) is a big no no as wel, since privacy might be used to uphold invariants relevant to safety.
If you don't know the internals of the type, you can't know if the internals meet the requirements (such as no internal padding). So you could exclude this by requiring that all components be plain-old-data, recursively, and that you have sufficient visibility to verify that.
transmuting between uN/iN and fN is currently only possible in unsafe code. I believe such transmutes are always safe but this does enlarge the safe language so it may be controverial.
Floating point numbers have signalling NaN which is a trap representation which results in UB.
@retep998 Does Rust run on any platforms that don't support disabling floating-point traps? (That doesn't change the UB issue, but in theory we could make that problem go away.)
The interpretation is that union is an enum for which we don't know the discriminant, i.e. we can guarantee that at any moment of time at least one of union's variants has valid value
I think I've come around to this interpretation -- well, not exactly this. I still think of it as there is some set of legal variants which is determined at the point where you store, as I always did. I think of storing a value into a union as a bit like putting it into a "quantum state" -- it could now potentially be transmuted into one of many legal interpretations. But I agree that when you move out from one of these variants, you have "forced" it into just one of those, and consumed the value. Therefore, you shouldn't be able to use the enum again (if that type is not Copy). So 👍, basically.
Question about #[repr(C)]: as @pnkfelix recently pointed out to me, the current spec states that if a union is not #[repr(C)], it is illegal to store with field x and read with field y. Presumably this is because we are not required to start all fields at the same offset.
I can see some utility in this: for example, a sanitizer might implement unions by storing them like a normal enum (or even a struct...?) and checking that you use the same variant that you put in.
But it seems like a kind of footgun, and also one of those repr guarantees we would never be able to actually change in practice, because too many people will be relying on it in the wild.
Thoughts?
The interpretation is that union is an enum for which we don't know the discriminant, i.e. we can guarantee that at any moment of time at least one of union's variants has valid value
The worst part is variant/field fragments, which are directly accessible for unions.
Consider this code:
union U {
a: (u8, bool),
b: (bool, u8),
}
fn main() {
unsafe {
let mut u = U { a: (2, false) };
u.b.1 = 2; // turns union's memory into (2, 2)
}
}
All fields are Copy, no ownership involved and move checker is happy, but the partial assignment to the inactive field b turns the union into state with 0 valid variants. I haven't thought how to deal with it yet. Make such assignments UB? Change the interpretation? Something else?
Make such assignments UB?
This would be my assumption, yes. When you assigned with a, the variant b was not in the set of valid variants, and hence later using u.b.1 (whether to read or assign) is invalid.
Question about #[repr(C)]: as @pnkfelix recently pointed out to me, the current spec states that if a union is not #[repr(C)], it is illegal to store with field x and read with field y. Presumably this is because we are not required to start all fields at the same offset.
I think the appropriate wording here is that 1) Reading from fields that are not "layout compatible" (this is vague) with previously written fields/field-fragments is UB 2) For #[repr(C)] unions users know what layouts are (from ABI docs) so they can discern between UB and non-UB 3) For #[repr(Rust)] union layouts are unspecified so users can't say what is UB and what is not, but WE (rustc/libstd + their tests) have this sacred knowledge, so we can separate the wheat from the chaff and use #[repr(Rust)] in non-UB fashion.
- After size/stride and field reordering questions are decided upon I'd expect struct and union layouts to be set in stone and specified, so users will know the layouts as well and will be able to use
#[repr(Rust)]unions as freely as#[repr(C)]and the problem will go away.
@nikomatsakis In the discussion of the union RFC, people mentioned wanting to have native Rust code that uses unions to build compact data structures.
Is there anything stopping from people using #[repr(C)]? If not, then I don't see the need to provide any sort of guarantees for #[repr(Rust)], just leave it as "here be dragons". Probably would be best to have a lint that is warn by default for unions that aren't #[repr(C)].
@retep998 It seems reasonable to me that repr(Rust) doesn't guarantee any particular layout or overlap. I would just suggest that repr(Rust) should not in practice break people's assumptions about the memory usage of a union ("no larger than the largest member").
Does Rust run on any platforms that don't support disabling floating-point traps?
That isn’t really a valid question to ask. First of all, optimiser itself can rely on the UB-ness of trap representations and rewrite the program in unexpected ways. Moreover, Rust does not really support altering FP environment either.
But it seems like a kind of footgun, and also one of those repr guarantees we would never be able to actually change in practice, because too many people will be relying on it in the wild.
Thoughts?
Adding a lint or some such which inspects the program flow and throws a complaint at the user if read is done from a field when the enum was provably written in some other field would help with this¹. MIR-based lint would make a short work of that. If a CFG does not allow making any conclusions on legality of the union field load and the user makes a mistake, the undefined behaviour is the best we can specify without having specified the Rust repr itself IMO.
¹: Especially effective if people begin using union as a poor man’s transmute for some reason.
should not in practice break people's assumptions about the memory usage of a union ("no larger than the largest member").
I disagree. It might make a lot sense to extend repr(Rust) stuff to the size of a machine word on some architectures, for example.
One issue that may want to be considered before stabilization is #37479. Looks like with the most recent version of LLDB debugging unions may not work :(
@alexcrichton Does it work with GDB?
From what I can tell, yes. The Linux bots seem to be running the test just fine.
Then that means Rust provides all the right debugging information, and LLDB just has a bug here. I don't think a bug in one of multiple debuggers, not present in another, should block stabilizing this. LLDB just needs fixing.
It would be cool to see if we could get this feature into FCP for the 1.17 cycle (that's the March 16th beta). Can anyone give a summary of the outstanding questions and the current situation of the feature so we can see if we can reach consensus and resolve everything?
@withoutboats
My plans are
- Wait for the upcoming release (Feb 3).
- Propose stabilization of unions with
Copyfields. This will cover all FFI needs - FFI libraries will be able to use unions on stable. "POD" unions are used for decades in C/C++ and well understood (modulo type based aliasing, but Rust doesn't have it), there are also no known blockers. - Write "Unions 1.2" RFC until Feb 3. It will describe the current implementation of unions and outline future directions. The future of unions with non-
Copyfields will be decided in the process of discussing this RFC.
Note, that exposing something like ManuallyDrop or NoDrop from the standard library doesn't require stabilizing unions.
STATUS UPDATE (Feb 4): I'm writing the RFC, but I'm having writer's block after each sentence, as usual, so there's a chance I'll finish it the next weekend (Feb 11-12) and not this weekend (Feb 4-5).
STATUS UPDATE (Feb 11): The text is 95% ready, I'll submit it tomorrow.
@petrochenkov that seems like a very reasonable course of action.
@petrochenkov That sounds reasonable to me. I also reviewed your unions 1.2 proposal, and provided some comments; overall, it looks good to me.
@joshtriplett I was thinking that, while in the @rust-lang/lang meeting we talked about keeping the check-lists up to date, I would actually like to see that -- for each of these points -- we make an affirmative decision (i.e., ideally with @rfcbot). This would probably suggest a distinct issue (or even amendment RFC). We could do this over time, but until then I don't feel like we've "settled" the answers to the open questions definitively. Along those lines, extracting and summarizing the relevant conversation into an amendment RFC or even just an issue we can link to from here seems like an excellent step to helping ensure that everyone is on the same page -- and something anyone who is interested can do, of course, not just @rust-lang/lang members or shepherds.
So, I have submitted the "Unions 1.2" RFC - rust-lang/rfcs#1897.
Now I'd like to propose stabilization of a conservative subset of union - all fields of the union should be Copy, the number of fields should be non-zero and the union should not implement Drop.
(I'm not sure the last requirement is viable though, because it may be easily circumvent by wrapping the union into a struct and implementing Drop for that struct.)
Such unions cover all the needs of FFI libraries, which are supposed to be the primary consumer of this language feature.
The text of "Unions 1.2" RFC doesn't really tell anything new about FFI-style unions, except that it explicitly confirms that type punning is permitted.
EDIT: "Unions 1.2" RFC is also going to make assignments to trivially-destructibleCopy fields safe (see #32836 (comment), #32836 (comment)), this affects FFI-style unions as well.
This text also provides documentation necessary for the stabilization.
The "Overview" section can be copy-pasted into the book and "Detailed design" into the reference.
ping @nikomatsakis
Does something like this really need to be added as part of the language? It took me about 20 minutes to knock up an implementation of a union using a little unsafe and ptr::write().
use std::mem;
use std::ptr;
/// A union of `f64`, `bool`, and `i32`.
#[derive(Default, Clone, PartialEq, Debug)]
struct Union {
data: [u8; 8],
}
impl Union {
pub unsafe fn get<T>(&self) -> &T {
&*(&self.data as *const _ as *const T)
}
pub unsafe fn set<T>(&mut self, value: T) {
// "transmute" our pointer to self.data into a &mut T so we can
// use ptr::write()
let data_ptr: &mut T = &mut *(&mut self.data as *mut _ as *mut T);
ptr::write(data_ptr, value);
}
}
fn main() {
let mut u = Union::default();
println!("data: {0:?} ({0:#p})", &u.data);
{
let as_i32: &i32 = unsafe { u.get() };
println!("as i32: {0:?} ({0:#p})", as_i32);
}
unsafe {
u.set::<f64>(3.14);
}
println!("As an f64: {:?}", unsafe { u.get::<f64>() });
}I feel like it wouldn't be difficult for someone to write a macro which can generate something like that except ensuring that the internal array is the size of the largest type. Then instead of my completely generic (and hideously unsafe) get::<T>() they could add a trait bound to limit the types you can get and set. You might even add specific getter and setter methods if you want named fields.
I'm thinking they might write something like this:
union! { Foo(u64, Vec<u8>, String) };My point is, this is something you could quite feasibly do as part of a library instead of adding extra syntax and complexity to an already fairly complex language. Plus with proc macros it's already quite possible, even if that hasn't fully hit stable yet.
@Michael-F-Bryan We don't have constant size_of yet though.
@Michael-F-Bryan It's not just enough to have a [u8] array, you also need to get the alignment correct. I in fact already use macros to handle unions but due to the lack of constant size_of and align_of I have to manually allocate the correct space, plus because there is no usable ident concatenation in declarative macros I have to manually specify the names for both the getters and setters. Even just initializing a union is hard at the moment because I have to first initialize it with some default value and then set the value to the variant I want (or add another set of methods to construct the union which is even more verbosity in the definition of the union). It's overall a lot more work and prone to error and uglier than native support for unions. Maybe you should read the RFC and the discussion that went with it so you can understand why this feature is so important.
And the same for alignment.
I imagine ident concatenation shouldn't be too difficult now syn exists. It allows you to do operations on the AST passed in, so you could take two Idents, extract their string representation (Ident implements AsRef<str>), then create a new Ident which is the concatenation of the two using Ident::From<String>().
The RFC mentions a lot about how existing macro implementations are cumbersome to use, however with the recent creation of crates like syn and quote, it's now a lot easier to do proc macros. I feel like that'd go a long way towards improving ergonomics and make things less error prone.
For example, you could have a MyUnion::default() which just zero's the union's internal buffer, then a fn MyUnion::new<T>(value:T) -> MyUnion, where T has a trait bound ensuring you can only initialize with the correct types.
In terms of alignment and size, are you able to use the mem module from the standard library (i.e. std::mem::align_of() and friends)? I guess everything I'm proposing would depend on being able to use those at macro expansion time to figure out the size and alignment required. 99.9% of the times unions are used it's done with primitive types anyway, so I feel like you'd be able to write a helper function which takes a type's name and returns its alignment or size (possibly asking the compiler, although that's more of an implementation detail).
I admit, built in pattern matching would be very nice, but most of the time any unions you use in FFI would get wrapped in a thin abstraction layer anyway. So you might be able to get away with a couple if/else statements or using a helper function.
In terms of alignment and size, are you able to use the mem module from the standard library (i.e. std::mem::align_of() and friends)?
That is not going to work in any cross compilation context.
@Michael-F-Bryan All of these discussions and many more were had in the history of rust-lang/rfcs#1444 . To summarize the responses to your specific concerns, in addition to those already mentioned: you'd have to reimplement the padding and alignment rules of every target platform/compiler, and use awkward syntax throughout your FFI code (which @retep998 has in fact done extensively for Windows bindings and can vouch for the awkwardness of). Also, proc macros currently only work for deriving; you can't extend syntax elsewhere.
Also:
99.9% of the times unions are used it's done with primitive types anyway
Not true at all. C code extensively uses a "struct of unions of structs" pattern, where most of the union fields consist of different struct types.
@rfcbot fcp merge per @petrochenkov's comment #32836 (comment)
I have nothing to add, just triggering the bot
Team member @withoutboats has proposed to merge this. The next step is review by the rest of the tagged teams:
No concerns currently listed.
Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
PSA: I'm going to update the "Unions 1.2" RFC with one more change affecting FFI-style unions - I'll move safe assignments to trivially-destructible fields of unions from "Future directions" to the RFC proper.
union.trivially_destructible_field = 10; // safeWhy:
- Assignments to trivially-destructible union fields are unconditionally safe, regardless of interpretation of unions.
- It will remove roughly a half of union-related
unsafeblocks. - It will be harder to do later due to potential large number of
unused_unsafewarnings/errors in stable code.
@petrochenkov Do you mean "trivially destructible union fields", or "unions with entirely trivially destructible fields"?
Are you proposing that all the unsafe behavior occurs on read, where you choose a interpretation? For instance, having a union containing an enum and other fields, where the union value contains an invalid discriminant?
At a high level that seems plausible. It does allow some things I'd consider unsafe, but Rust in general doesn't, such as bypassing destructors or leaking memory. At a low level, I'd hesitate to consider that sound.
I feel ok about stabilizing that subset. I don't know my opinion about this Unions 1.2 RFC yet as I haven't had time to read it! I'm not sure what I think about allowing safe access to fields in some cases. I feel like our efforts to make a "minimal" notion of what is unsafe (just dereferencing pointers) was a mistake, in retrospect, and we should have declared a wider swath of things unsafe (e.g., a lot of casts), since they interact in complex ways with LLVM. I feel like this may be the case here too. Put another way, I might rather pull back the rules about unsafe in concert with more progress on unsafe code guidelines.
@joshtriplett
"trivially destructible fields", I've tweaked the wording.
Are you proposing that all the unsafe behavior occurs on read, where you choose a interpretation?
Yes. The write alone can't cause anything dangerous without a subsequent read.
EDIT:
I feel like our efforts to make a "minimal" notion of what is unsafe (just dereferencing pointers) was a mistake, in retrospect
Oh.
Safe writes are completely in line with the current approach to unsafety, but if you are going to change it, then I should probably wait.
I don't feel great about stabilising this subset. Usually when we stabilise a subset it is a syntactic subset or at least fairly obvious subset. This subset feels a bit complex to me. If there is so much undecided about the feature that we aren't ready to stabilise the current implementation, then I'd rather leave the whole thing unstable for a while longer.
@nrc
Well, the subset is rather obvious - "FFI unions", or "C unions", or "pre-C++11 unions" - even if it's not syntactic. My initial goal was to stabilize this subset ASAP (this cycle, ideally) so it could be used in libraries like winapi.
There's nothing especially dubious about the remaining subset and its implementation, it's just not urgent and need to wait for unclear amount of time until the process for "Unions 1.2" RFC completes. My expectations would be to stabilize the remaining parts in 1, 2 or 3 cycles after stabilization of the initial subset.
I think I have an ultimate argument for safe field assignments.
Unsafe field assignment
unsafe {
u.trivially_destructible_field = value;
}is equivalent to safe full union assignment
u = U { trivially_destructible_field: value };except that the safe version is paradoxically less safe because it will overwrite u's bytes outside of trivially_destructible_field with undefs, while field assignment have guarantee about leaving them intact.
@petrochenkov The extreme of that is size_of_val(&value) == 0, right?