Formally define repr(u32, i8, etc...) and repr(C) on enums with payloads

text/0000-really-tagged-unions.md

@@ -0,0 +1,362 @@
+- Feature Name: really_tagged_unions
+- Start Date: 2017-10-30
+- RFC PR: (leave this empty)
+- Rust Issue: (leave this empty)
+
+# Summary
+[summary]: #summary
+
+Formally define the enum `#[repr(X)]` attributes to force a non-C-like enum to be equivalent to a tag followed by a union. This serves two purposes: allowing low-level Rust code to independently initialize the tag and payload, and allowing C(++) to safely manipulate these types.
+
+
+# Motivation
+[motivation]: #motivation
+
+Enums that contain data are very good and useful. Unfortunately, their layout is currently purposefully unspecified, which makes these kinds of enums unusable for FFI and for low-level code. To demonstrate this, this RFC will look at two examples from firefox development where this has been a problem.
+
+
+
+## C(++) FFI
+
+
+Consider a native Rust API for drawing a line, that uses a C-like LineStyle enum:
+
+```rust
+// In native Rust crate
+
+pub fn draw_line(&mut self, bounds: &Rect, color: &Color, style: LineStyle) {
+    ...
+}
+
+#[repr(u8)]
+pub enum LineStyle {
+    Solid,
+    Dotted,
+    Dashed,
+}
+
+#[repr(C)]
+pub struct Rect { x: f32, y: f32, width: f32, height: f32 }
+
+#[repr(C)]
+pub struct Color { r: f32, g: f32, b: f32, a: f32 }
+```
+
+
+This API is fairly easy for us to write a machine-checked shim for C++ code to invoke:
+
+```rust
+// In Rust shim crate
+
+#[no_mangle]
+pub extern "C" fn wr_draw_line(
+    state: &mut State, 
+    bounds: &Rect, 
+    color: &Color,
+    style: LineStyle,
+) {
+    state.draw_line(bounds, color, style);
+} 
+```
+
+```cpp
+// In C++ shim header
+
+
+// Autogenerated by cbindgen
+extern "C" {
+namespace wr {
+struct State; // opaque
+
+struct Rect { float x; float y; float width; float height; }
+struct Color { float r; float g; float b; float a; }
+
+enum class LineStyle: uint8_t {
+    Solid,
+    Dotted,
+    Dashed,
+}
+
+void wr_draw_line(WrState *state,
+                  const Rect *bounds,
+                  const ColorF *aColor,
+                  LineStyle aStyle);
+} // namespace wr
+} // extern
+
+
+
+// Hand-written
+void WrDrawLine(
+    wr::State* aState, 
+    const wr::Rect* aRect, 
+    const wr::Color* aColor, 
+    wr::LineStyle aStyle
+) {
+    wr_draw_line(aState, aRect, aColor, aStyle);
+}
+```
+
+
+This works well, and we're happy.
+
+Now consider adding a WavyLine style, which requires an extra thickness value:
+
+```rust
+// Native Rust crate
+
+#[repr(u8)]         // Doesn't actually do anything we can rely on now
+enum LineStyle {
+    Solid,
+    Dotted,
+    Dashed,
+    Wavy { thickness: f32 },
+}
+```
+
+We cannot safely pass this to/from C(++), nor can we manipulate it there. As
+such, we're forced to take the thickness as an extra argument that is just
+ignored most of the time:
+
+```rust
+// Native Rust crate
+
+pub fn draw_line(
+    &mut self, 
+    bounds: &Rect, 
+    color: &Color, 
+    style: LineStyle, 
+    wavy_line_thickness: f32
+) { ... }
+
+#[repr(u8)]
+enum LineStyle {
+    Solid,
+    Dotted,
+    Dashed,
+    Wavy,
+}
+```
+
+This produces a worse API for everyone, while also throwing away the type-safety benefits of enums. This trick also doesn't scale: if you have many nested enums, the combinatorics eventually become completely intractable.
+
+
+
+## In-Place Construction
+
+Popular deserialization APIs in Rust generally have a signature like `deserialize<T>() -> Result<T, Error>`. This works well for small values, but optimizes very poorly for large values, as Rust ends up copying the `T` many times. Further, in many cases we just want to overwrite an old value that we no longer care about.
+
+In those cases, we could potentially use an API like `deserialize_from<T>(&mut T) -> Result<(), Error>`. However Rust currently requires enums to be constructed "atomically", so we can't actually take advantage of this API if our large value is an enum.
+
+That is, we must do something like:
+
+```rust
+fn deserialize_from(dest: &mut MyBigEnum) -> Result<(), Error> {
+    let tag = deserialize_tag()?;
+    match tag {
+        A => {
+            let payload = deserialize_a()?
+            *dest = A(payload);
+        }
+        ..
+    }
+    Ok(())
+}
+```
+
+We must construct the entire payload out-of-place, and then move it into place at the end, even though our API is specifically designed to let us construct in-place.
+
+Now, this is pretty important for memory-safety in the *general* case, but there are many cases where this can be done safely. For instance, this is safe to do if the entire payload is plain-old-data, like `[u8; 200]`, or if the code catches panics and fixes up the value.
+
+Note that one cannot do something like:
+
+```rust
+*dest = A(mem::uninitialized())
+if let A(ref mut payload_dest) = *dest {
+    deserialize_a(payload_dest);
+} else { unreachable!() }
+```
+
+because enum optimizations make it unsound to put `mem::uninitialized` in an enum. That is, checking if `dest = A` can require inspecting the payload.
+
+To accomplish this task, we need dedicated support from the language.
+
+
+
+
+
+# Guide-level explanation
+[guide-level-explanation]: #guide-level-explanation
+
+An enum can currently be adorned with `#[repr(X)]` where `X` is one of Rust's integer types (u8, isize, etc). For C-like enums -- enums which have no variants with associated data -- this specifies that the enum should have the ABI of that integer type (size, alignment, and calling convention). `#[repr(C)]` just tells Rust to try to pick whatever integer type that a C compiler for the target platform would use for an enum.
+
+With this RFC, `#[repr(X)]` on a non-C-like enum will also have a meaning: the enum must be represented by a struct with two members: a tag followed by a union. The tag is a C-like enum with `#[repr(X)]`, and every composite member of the union is `#[repr(C)]`.
+
+Specifically, this definition:
+
+```rust
+#[repr(X)]
+enum MyEnum {
+    A(u32),
+    B(f32, u64),
+    C { x: u32, y: u8 },
+    D,
+}
+```
+
+Is equivalent to the following:
+
+```rust
+#[repr(C)]
+struct MyEnumRepr {
+    tag: MyEnumTag,
+    payload: MyEnumPayload,
+}
+
+#[repr(X)]
+enum MyEnumTag { A, B, C, D }
+
+#[repr(C)]
+union MyEnumPayload {
+   A: u32,
+   B: MyEnumPayloadB,
+   C: MyEnumPayloadC,
+   D: (),
+}
+
+#[repr(C)]
+struct MyEnumPayloadB(f32, u64);
+
+#[repr(C)]
+struct MyEnumPayloadC { x: u32, y: u8 }
+```
+
+C++ can also correctly manipulate this enum with the following definition:
+
+```cpp
+#include <stdint.h>
+
+enum class MyEnumTag: CppEquivalentOfX { A, B, C, D };
+struct MyEnumPayloadB { float _0; uint64_t _1;  };
+struct MyEnumPayloadC { uint32_t x; uint8_t y; };
+
+union MyEnumPayload {
+   uint32_t A;
+   MyEnumPayloadB B;   
+   MyEnumPayloadC C;
+};
+
+struct MyEnum {
+    MyEnumTag tag;
+    MyEnumPayload payload;
+};
+```
+
+The correct C definition is essentially the same, but with the `enum class` replaced with a plain integer of the appropriate type.
+
+In addition, it is defined for Rust programs to cast/reinterpret/transmute such an enum into the equivalent tag+union Rust definition above. Seperately manipulating the tag and payload is also defined. The tag and payload need only be in a consistent/initialized state when the value is matched on (which includes Dropping it). This means that the contents of a `#[repr(X)]` enum cannot be inspected by outer enums for storage optimizations -- `Option<MyEnum>` must be larger than `MyEnum`.
+
+For instance, this code is valid (using the same definitions above):
+
+```rust
+/// Tries to parse a MyEnum from a custom binary format, overwriting `dest`.
+/// On Err, `dest` may be partially overwritten (but will be in a memory-safe state)
+fn parse_my_enum_from<'a>(dest: &'a mut MyEnum, input: &mut &[u8]) -> Result<(), &'static str> {
+    unsafe {
+        // Convert to raw repr
+        let dest: &'a mut MyEnumRepr = mem::transmute(dest);
+
+        // If MyEnum was non-trivial, we might match on the tag and 
+        // drop_in_place the payload here to start.
+
+        // Read the tag
+        let tag = input.get(0).ok_or("Couldn't Read Tag")?;
+        dest.tag = match tag {
+            0 => MyEnumTag::A,
+            1 => MyEnumTag::B,
+            2 => MyEnumTag::C,
+            3 => MyEnumTag::D,
+            _ => { return Err("Invalid Tag Value"); }
+        };
+        *input = &input[1..];
+
+        // Note: it would be very bad if we panicked past this point, or if
+        // the following methods didn't initialize the payload on Err!
+
+        // Read the payload
+        match dest.tag {
+            MyEnumTag::A => parse_my_enum_a_from(&mut dest.payload.A, input),
+            MyEnumTag::B => parse_my_enum_b_from(&mut dest.payload.B, input),
+            MyEnumTag::C => parse_my_enum_c_from(&mut dest.payload.C, input),
+            MyEnumTag::D => { Ok(()) /* do nothing */ }
+        }
+    }
+}
+```
+
+
+# Reference-level explanation
+[reference-level-explanation]: #reference-level-explanation
+
+Since the whole point of this proposal is to enable low-level control, the guide-level explanation should cover all the relevant corner-cases and details in sufficient detail. All that remains is to discuss implementation details. Thankfully, this is very simple: unless there's been an error, this should exactly match the current behaviour of the compiler.
+
+It was [informally decided earilier this year](https://github.com/rust-lang/rust/issues/40029) that we should have this behaviour, as it was being relied on and it made sense. There is even a test in the rust-lang repo that was added to ensure that this behaviour doesn't regress.
+
+We may need to double-check that the sub-structs are actually being marked as `repr(C)`.
+
+
+
+# Drawbacks
+[drawbacks]: #drawbacks
+
+There aren't really any drawbacks: we already decided to do this. This is just making it official.
+
+
+
+# Rationale and alternatives
+[alternatives]: #alternatives
+
+There are a few alternative interpretations of `repr(X)` on a non-C-like enum.
+
+
+
+## It should do nothing 
+
+In which case it should probably become an error/warning. This isn't particularly desirable, as was discussed [when we decided to maintain this behaviour](https://github.com/rust-lang/rust/issues/40029).
+
+
+
+## The tag should come after the union, and/or order should be manually specified
+
+There isn't a particularly compelling reason to move the tag around because of how padding and alignment are handled: you can't actually save space by putting the tag after, as long as your tag is a reasonable size.
+
+It's possible positioning the tag afterwards could be desirable to interoperate with a definition that is provided by a third party (hardware spec or some existing C library). However there are tons of other tag packing strategies that we also can't handle, so we'd probably want a more robust solution for those kinds of cases anyway.
+
+
+
+## Compound variants shouldn't automatically be marked as `repr(C)`
+
+It's possible in the pure-Rust case to only want the outer-most level to be well-defined and to still let the inner contents be aggressively optimized. This interpretation and the RFC's primary interpretation can always be implemented on top of the other one by just making all compound variants into outline structs, just like how the desugarrings in the previous sections were written.
+
+So this is just a matter of "what is a good default". The FFI case clearly wants fully defined layouts, while the pure-Rust case seems like a toss up. It seems like `repr(C)` is therefore the more ergonomic default.
+
+
+## The tag shouldn't be opaque to outer enums
+
+So for instance `Option<MyEnum>` could be the same size as `MyEnum`. The author hasn't fully thought this issue out. It seems like it might be fine? It just seems safest to completely abandon optimization. Feedback is welcome here.
+
+
+
+
+# Unresolved questions
+[unresolved]: #unresolved-questions
+
+The unresolved questions basically amount to future extensions of this design. Here are some basic sketches/notes for things which are possible, but which have been left out for simplicity.
+
+* A field/method for the tag/payload (my_enum.tag, my_enum.payload)
+    * Probably should be a field to avoid conflicts with user-defined methods
+    * Might need `#[repr(pub(X))]` for API design reasons
+* Compiler-generated definitions for the Repr types
+    * With inherent type aliases on the enum? (`MyEnum::Tag`, `MyEnum::Payload`, `MyEnum::PayloadA`, etc.)
+* As discussed in the previous section, more advanced tag placement strategies?
+* Allow specifying tag's value: `#[repr(u32)] MyEnum { A(u32) = 2, B = 5 }`