Transmuting associated type of trait ref + data ref

[dhardy]

I haven't seen this pattern discussed:

pub trait Widget {
    type Data;

    fn as_node<'a>(&'a mut self, data: &'a Self::Data) -> Node<'a>;

    fn child_node<'n>(&'n mut self, data: &'n Self::Data, index: usize) -> Option<Node<'n>>;

    // ... other methods which may take `&Self::Data` but do not use this type in any other way
}

pub struct Node<'a>(
    &'a mut dyn Widget<Data = ()>,
    &'a (),
);

impl<'a> Node<'a> {
    /// Construct
    #[inline(always)]
    pub fn new<T>(widget: &'a mut dyn Widget<Data = T>, data: &'a T) -> Self {
        // Safety: since the vtable for dyn Widget<Data = T> only uses T as &T
        // and T: Sized, the vtable should be equivalent for all T.
        // We ensure here that the type of `data` matches that used by `widget`.
        // NOTE: This makes assumptions beyond Rust's specification.
        use std::mem::transmute;
        unsafe { Node(transmute(widget), transmute(data)) }
    }

    pub fn get_child(&mut self, index: usize) -> Option<Node<'_>> {
        self.0.child_node(self.1, index)
    }
}

I believe this falls into the valley of "not UB but also not specified sound behaviour"? It makes some assumptions about the vtable and about &() not being optimised out, but does appear to work.

Note: type Widget::Data does not have a lifetime bound, but this can be changed if recommended — I don't think this changes anything however.

Full code (including a safe implementation using boxing) can be found here: https://github.com/kas-gui/kas/blob/master/crates/kas-core/src/core/node.rs

[zachs18]

I don't think "not UB but also not specified sound behaviour" is an accurate way to describe this. This is definitely UB as currently described in the Reference and checked by Miri, since it creates &mut dyn Widget<()>s whose vtable is for &mut dyn Widget<T> where T != ().

The metadata of a wide reference, [..] must match the type of the unsized tail:

• dyn Trait metadata must be a pointer to a compiler-generated vtable for Trait. [..]

(Reference)

Miri example of safe code using this API (playground):

error: Undefined Behavior: constructing invalid value: wrong trait in wide pointer vtable: expected `Widget<Data = ()>`, but encountered `Widget<Data = std::string::String>`
  --> src/main.rs:35:23
   |
35 |         unsafe { Node(transmute(widget), transmute(data)) }
   |                       ^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: BACKTRACE:
   = note: inside `Node::<'_>::new::<std::string::String>` at src/main.rs:35:23: 35:40
note: inside `<u32 as Widget>::as_node`
  --> src/main.rs:14:9
   |
14 |         Node::new::<String>(self, data)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `main`
  --> src/main.rs:46:5
   |
46 |     x.as_node(&data);
   |     ^^^^^^^^^^^^^^^^

---

That said, I don't think this is likely to be "miscompiled" currently. However, if you want to be safe against future changes to the compiler, then you could make your own "vtable" something like this

[dhardy]

Interesting alternative using manually-generated vtables, thanks.

Aside: the vtable thunk does show up in backtraces:

thread 'main' panicked at src/main.rs:72:9:
not yet implemented
stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/std/src/panicking.rs:697:5
   1: core::panicking::panic_fmt
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/core/src/panicking.rs:75:14
   2: core::panicking::panic
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/core/src/panicking.rs:145:5
   3: <u32 as playground::Widget>::child_node
             at ./src/main.rs:72:9
   4: playground::WidgetVTable::for_widget::child_node
             at ./src/main.rs:31:32
   5: playground::Node::get_child
             at ./src/main.rs:125:13
   6: playground::main
             at ./src/main.rs:134:22
   7: core::ops::function::FnOnce::call_once
             at ./.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

(Equivalent to the boxed alternative using a hidden trait that I linked.)

That said, I don't think this is likely to be "miscompiled" currently.

So this is unspecified, undefined behaviour that still works.

While your implementation is very interesting (and could be implemented easily enough since macros already generate or augment nearly all Widget implementations), it appears to have little to offer over the box+internal-trait implementation used by default and adds a significant amount of unsafe code. So I think I'll stick with my current approach (zero-unsafe by default but retaining the unsafe_node feature).

I still appreciate the feedback on this.

---

Do you see any future where additional specification could lead to my approach above becoming UB-free?

(If not, I guess close this issue.)