vtable crate: Some refactoring and simplifications

api/sixtyfps-rs/lib.rs

@@ -3,7 +3,7 @@ pub use sixtyfps_rs_macro::sixtyfps;
 /// internal re_exports used by the macro generated
 pub mod re_exports {
     pub use const_field_offset::FieldOffsets;
-    pub use corelib::abi::datastructures::{Component, ComponentVTable, ItemTreeNode, ComponentTO};
+    pub use corelib::abi::datastructures::{Component, ComponentTO, ComponentVTable, ItemTreeNode};
     pub use corelib::abi::primitives::{Image, ImageVTable, Rectangle, RectangleVTable};
     pub use gl::sixtyfps_runtime_run_component_with_gl_renderer;
     pub use once_cell::sync::Lazy;

helper_crates/vtable/macro/macro.rs

@@ -52,7 +52,6 @@ pub fn vtable(_attr: TokenStream, item: TokenStream) -> TokenStream {
 
     let trait_name = Ident::new(&vtable_name[..vtable_name.len() - 6], input.ident.span());
     let to_name = quote::format_ident!("{}TO", trait_name);
-    let impl_name = quote::format_ident!("{}Impl", trait_name);
     let module_name = quote::format_ident!("{}_vtable_mod", trait_name);
     let ref_name = quote::format_ident!("{}Ref", trait_name);
     let refmut_name = quote::format_ident!("{}RefMut", trait_name);
@@ -209,10 +208,10 @@ pub fn vtable(_attr: TokenStream, item: TokenStream) -> TokenStream {
                     }));
                     let self_ty = &param.ty;
                     let const_or_mut = mutability.map_or_else(|| quote!(const), |x| quote!(#x));
-                    call_code = Some(quote!(#call_code <#self_ty>::from_inner(*self),));
+                    call_code =
-                    self_call = Some(
+                        Some(quote!(#call_code <#self_ty>::from_raw(self.vtable, self.ptr),));
-                        quote!(&#mutability (*(<#self_ty>::get_ptr(&#arg_name).as_ptr() as *#const_or_mut T)),),
+                    self_call =
-                    );
+                        Some(quote!(&#mutability (*(#arg_name.as_ptr() as *#const_or_mut T)),));
                     has_self = true;
                     continue;
                 }
@@ -271,10 +270,12 @@ pub fn vtable(_attr: TokenStream, item: TokenStream) -> TokenStream {
 
                 drop_impl = Some(quote! {
                     impl VTableMetaDrop for #vtable_name {
-                        unsafe fn drop(ptr: #to_name) {
+                        unsafe fn drop(ptr: *mut #to_name) {
                             // Safety: The vtable is valid and inner is a type corresponding to the vtable,
                             // which was allocated such that drop is expected.
-                            unsafe { (ptr.vtable.as_ref().#ident)(VRefMut::from_inner(ptr)) }
+                            unsafe {
+                                let ptr = &*ptr;
+                                (ptr.vtable.as_ref().#ident)(VRefMut::from_raw(ptr.vtable, ptr.ptr)) }
                         }
                     }
 
@@ -291,7 +292,7 @@ pub fn vtable(_attr: TokenStream, item: TokenStream) -> TokenStream {
 
             generated_to_fn_trait.push(ImplItemMethod {
                 attrs: vec![],
-                vis: Visibility::Inherited,
+                vis: Visibility::Public(VisPublic { pub_token: Default::default() }),
                 defaultness: None,
                 sig: sig.clone(),
                 block: parse2(if has_self {
@@ -412,36 +413,28 @@ pub fn vtable(_attr: TokenStream, item: TokenStream) -> TokenStream {
             #generated_trait
             #generated_trait_assoc_const
 
-            struct #impl_name { _private: [u8; 0] }
+            /// Invariant, same as vtable::Inner: vtable and ptr has to be valid and ptr an instance macthcin the vtable
-
-            /// This structure is highly unsafe, as it just has pointers. One could call trait functions
-            /// directly.  However, it should not be possible, in safe code, to construct or to obtain a reference
-            /// to this structure, as it cannot be constructed safely. And none of the safe api allow accessing
-            /// a reference or a copy of this structure
             #[doc(hidden)]
-            #[derive(Clone, Copy)]
             #[repr(C)]
             pub struct #to_name {
                 vtable: core::ptr::NonNull<#vtable_name>,
-                ptr: core::ptr::NonNull<#impl_name>,
+                ptr: core::ptr::NonNull<u8>,
+            }
+            impl #to_name {
+                #(#generated_to_fn_trait)*
+
+                pub fn get_vtable(&self) -> &#vtable_name {
+                    unsafe { self.vtable.as_ref() }
+                }
+
+                pub fn as_ptr(&self) -> *const u8 {
+                    self.ptr.as_ptr()
+                }
             }
-            impl #trait_name for #to_name { #(#generated_to_fn_trait)* }
 
             unsafe impl VTableMeta for #vtable_name {
-                type Trait = dyn #trait_name;
                 type VTable = #vtable_name;
-                type TraitObject = #to_name;
+                type Target = #to_name;
-                #[inline]
-                unsafe fn map_to(from: &Self::TraitObject) -> &Self::Trait { from }
-                #[inline]
-                unsafe fn map_to_mut(from: &mut Self::TraitObject) -> &mut Self::Trait { from }
-                #[inline]
-                unsafe fn get_ptr(from: &Self::TraitObject) -> core::ptr::NonNull<u8> { from.ptr.cast() }
-                #[inline]
-                unsafe fn get_vtable(from: &Self::TraitObject) -> &Self::VTable { from.vtable.as_ref() }
-                #[inline]
-                unsafe fn from_raw(vtable: core::ptr::NonNull<Self::VTable>, ptr: core::ptr::NonNull<u8>) -> Self::TraitObject
-                { #to_name { vtable, ptr : ptr.cast() } }
             }
 
             #drop_impl

helper_crates/vtable/src/lib.rs

@@ -4,86 +4,72 @@ use core::ptr::NonNull;
 pub use vtable_macro::*;
 
 pub unsafe trait VTableMeta {
-    /// that's the rust trait.  (e.g: `Hello`)
+    /// That's the trait object that implements the functions
-    type Trait: ?Sized;
+    /// NOTE: the size must be 2*size_of::<usize>
-    /// that's the vtable struct `HelloVTable`
+    /// and a repr(C) with (vtable, ptr) so it has the same layout as
+    /// the inner and VBox/VRef/VRefMut
+    type Target;
+
+    /// That's the VTable itself (so most likely Self)
     type VTable;
-
-    /// That's the trait object that implements the trait.
-    /// NOTE: the size must be 2*size_of<usize>
-    type TraitObject: Copy;
-
-    /// That maps from the tait object from the trait iteself
-    /// (In other word, return 'to' since 'to' implements trait,
-    /// but we can't represent that in rust right now, hence these helper)
-    ///
-    /// Safety: the trait object need to be pointing to valid pointer / vtable
-    unsafe fn map_to(to: &Self::TraitObject) -> &Self::Trait;
-    /// Same as map_to, but mutable
-    unsafe fn map_to_mut(to: &mut Self::TraitObject) -> &mut Self::Trait;
-
-    /// Return a raw pointer to the inside of the impl
-    unsafe fn get_ptr(from: &Self::TraitObject) -> NonNull<u8>;
-
-    /// Create a trait object from its raw parts
-    unsafe fn from_raw(vtable: NonNull<Self::VTable>, ptr: NonNull<u8>) -> Self::TraitObject;
-
-    /// return a reference to the vtable
-    unsafe fn get_vtable(from: &Self::TraitObject) -> &Self::VTable;
 }
 
 pub trait VTableMetaDrop: VTableMeta {
-    /// Safety: the traitobject need to be pointing to a valid allocated pointer
+    /// Safety: the Target need to be pointing to a valid allocated pointer
-    unsafe fn drop(ptr: Self::TraitObject);
+    unsafe fn drop(ptr: *mut Self::Target);
 }
 
-// These checks are not enough to ensure that this is not unsafe.
+#[derive(Copy, Clone)]
-fn sanity_checks<T: ?Sized + VTableMeta>() {
+/// The inner structure of VRef, VRefMut, and VBox.
-    debug_assert_eq!(core::mem::size_of::<T::TraitObject>(), 2 * core::mem::size_of::<usize>());
+///
+/// Invariant: _vtable and _ptr are valid pointer for the lifetime of the container.
+/// _ptr is an instance of the object represented by _vtable
+#[allow(dead_code)]
+struct Inner {
+    vtable: *const u8,
+    ptr: *const u8,
+}
+
+impl Inner {
+    /// Transmute a reference to self into a reference to T::Target.
+    fn deref<T: ?Sized + VTableMeta>(&self) -> *const T::Target {
+        debug_assert_eq!(core::mem::size_of::<T::Target>(), core::mem::size_of::<Inner>());
+        self as *const Inner as *const T::Target
+    }
 }
 
 #[repr(C)]
 pub struct VBox<T: ?Sized + VTableMetaDrop> {
-    inner: T::TraitObject,
+    inner: Inner,
+    phantom: PhantomData<T::Target>,
 }
 
 impl<T: ?Sized + VTableMetaDrop> Deref for VBox<T> {
-    type Target = T::Trait;
+    type Target = T::Target;
     fn deref(&self) -> &Self::Target {
-        sanity_checks::<T>();
+        unsafe { &*self.inner.deref::<T>() }
-        unsafe { T::map_to(&self.inner) }
     }
 }
 impl<T: ?Sized + VTableMetaDrop> DerefMut for VBox<T> {
     fn deref_mut(&mut self) -> &mut Self::Target {
-        sanity_checks::<T>();
+        unsafe { &mut *(self.inner.deref::<T>() as *mut _) }
-        unsafe { T::map_to_mut(&mut self.inner) }
     }
 }
 
 impl<T: ?Sized + VTableMetaDrop> Drop for VBox<T> {
     fn drop(&mut self) {
         unsafe {
-            T::drop(self.inner);
+            T::drop(self.inner.deref::<T>() as *mut _);
         }
     }
 }
 
 impl<T: ?Sized + VTableMetaDrop> VBox<T> {
-    pub unsafe fn from_inner(inner: T::TraitObject) -> Self {
-        Self { inner }
-    }
-    pub unsafe fn inner(x: &Self) -> T::TraitObject {
-        x.inner
-    }
-    pub unsafe fn get_ptr(x: &Self) -> NonNull<u8> {
-        T::get_ptr(&x.inner)
-    }
     pub unsafe fn from_raw(vtable: NonNull<T::VTable>, ptr: NonNull<u8>) -> Self {
-        Self { inner: T::from_raw(vtable, ptr) }
+        Self {
-    }
+            inner: Inner { vtable: vtable.cast().as_ptr(), ptr: ptr.cast().as_ptr() },
-    pub fn get_vtable(&self) -> &T::VTable {
+            phantom: PhantomData,
-        unsafe { T::get_vtable(&self.inner) }
+        }
     }
     pub fn borrow<'b>(&'b self) -> VRef<'b, T> {
         unsafe { VRef::from_inner(self.inner) }
@@ -94,8 +80,8 @@ impl<T: ?Sized + VTableMetaDrop> VBox<T> {
 }
 
 pub struct VRef<'a, T: ?Sized + VTableMeta> {
-    inner: T::TraitObject,
+    inner: Inner,
-    _phantom: PhantomData<&'a T::Trait>,
+    phantom: PhantomData<&'a T::Target>,
 }
 
 // Need to implement manually otheriwse it is not implemented if T do not implement Copy / Clone
@@ -103,80 +89,65 @@ impl<'a, T: ?Sized + VTableMeta> Copy for VRef<'a, T> {}
 
 impl<'a, T: ?Sized + VTableMeta> Clone for VRef<'a, T> {
     fn clone(&self) -> Self {
-        Self { inner: self.inner, _phantom: self._phantom }
+        Self { inner: self.inner, phantom: PhantomData }
     }
 }
 
 impl<'a, T: ?Sized + VTableMeta> Deref for VRef<'a, T> {
-    type Target = T::Trait;
+    type Target = T::Target;
     fn deref(&self) -> &Self::Target {
-        sanity_checks::<T>();
+        unsafe { &*self.inner.deref::<T>() }
-        unsafe { T::map_to(&self.inner) }
     }
 }
 
 impl<'a, T: ?Sized + VTableMeta> VRef<'a, T> {
-    pub unsafe fn from_inner(inner: T::TraitObject) -> Self {
+    unsafe fn from_inner(inner: Inner) -> Self {
-        Self { inner, _phantom: PhantomData }
+        Self { inner, phantom: PhantomData }
-    }
-    pub unsafe fn inner(x: &Self) -> T::TraitObject {
-        x.inner
-    }
-    pub unsafe fn get_ptr(x: &Self) -> NonNull<u8> {
-        T::get_ptr(&x.inner)
     }
     pub unsafe fn from_raw(vtable: NonNull<T::VTable>, ptr: NonNull<u8>) -> Self {
-        Self { inner: T::from_raw(vtable, ptr), _phantom: PhantomData }
+        Self {
-    }
+            inner: Inner { vtable: vtable.cast().as_ptr(), ptr: ptr.cast().as_ptr() },
-    pub fn get_vtable(&self) -> &T::VTable {
+            phantom: PhantomData,
-        unsafe { T::get_vtable(&self.inner) }
+        }
     }
 }
 
 pub struct VRefMut<'a, T: ?Sized + VTableMeta> {
-    inner: T::TraitObject,
+    inner: Inner,
-    _phantom: PhantomData<&'a mut T::Trait>,
+    phantom: PhantomData<&'a mut T::Target>,
 }
 
 impl<'a, T: ?Sized + VTableMeta> Deref for VRefMut<'a, T> {
-    type Target = T::Trait;
+    type Target = T::Target;
     fn deref(&self) -> &Self::Target {
-        sanity_checks::<T>();
+        unsafe { &*self.inner.deref::<T>() }
-        unsafe { T::map_to(&self.inner) }
     }
 }
 
 impl<'a, T: ?Sized + VTableMeta> DerefMut for VRefMut<'a, T> {
     fn deref_mut(&mut self) -> &mut Self::Target {
-        sanity_checks::<T>();
+        unsafe { &mut *(self.inner.deref::<T>() as *mut _) }
-        unsafe { T::map_to_mut(&mut self.inner) }
     }
 }
 
 impl<'a, T: ?Sized + VTableMeta> VRefMut<'a, T> {
-    pub unsafe fn from_inner(inner: T::TraitObject) -> Self {
+    unsafe fn from_inner(inner: Inner) -> Self {
-        Self { inner, _phantom: PhantomData }
+        Self { inner, phantom: PhantomData }
-    }
-    pub unsafe fn inner(x: &Self) -> T::TraitObject {
-        x.inner
-    }
-    pub unsafe fn get_ptr(x: &Self) -> NonNull<u8> {
-        T::get_ptr(&x.inner)
-    }
-    pub fn borrow<'b>(&'b self) -> VRef<'b, T> {
-        unsafe { VRef::from_inner(VRefMut::inner(self)) }
-    }
-    pub fn borrow_mut<'b>(&'b mut self) -> VRefMut<'b, T> {
-        unsafe { VRefMut::from_inner(VRefMut::inner(self)) }
-    }
-    pub fn into_ref(self) -> VRef<'a, T> {
-        unsafe { VRef::from_inner(VRefMut::inner(&self)) }
     }
     pub unsafe fn from_raw(vtable: NonNull<T::VTable>, ptr: NonNull<u8>) -> Self {
-        Self { inner: T::from_raw(vtable, ptr), _phantom: PhantomData }
+        Self {
+            inner: Inner { vtable: vtable.cast().as_ptr(), ptr: ptr.cast().as_ptr() },
+            phantom: PhantomData,
+        }
     }
-    pub fn get_vtable(&self) -> &T::VTable {
+    pub fn borrow<'b>(&'b self) -> VRef<'b, T> {
-        unsafe { T::get_vtable(&self.inner) }
+        unsafe { VRef::from_inner(self.inner) }
+    }
+    pub fn borrow_mut<'b>(&'b mut self) -> VRefMut<'b, T> {
+        unsafe { VRefMut::from_inner(self.inner) }
+    }
+    pub fn into_ref(self) -> VRef<'a, T> {
+        unsafe { VRef::from_inner(self.inner) }
     }
 }
 

helper_crates/vtable/tests/test_vtable.rs

@@ -26,7 +26,6 @@ impl Hello for SomeStruct {
         self.0
     }
 
-
     fn construct(init: u32) -> Self {
         println!("calling Construct {}", init);
         Self(init)
@@ -40,7 +39,7 @@ impl HelloConsts for SomeStruct {
     const CONSTANT: usize = 88;
 }
 
-static SOME_STRUCT_TYPE : HelloVTable = HelloVTable_static!(SomeStruct);
+static SOME_STRUCT_TYPE: HelloVTable = HelloVTable_static!(SomeStruct);
 
 #[test]
 fn test() {
@@ -53,5 +52,3 @@ fn test() {
     assert_eq!(bx.foo(2), 97);
     assert_eq!(bx.get_vtable().CONSTANT, 88);
 }
-
-

sixtyfps_runtime/corelib/abi/datastructures.rs

@@ -121,14 +121,14 @@ pub type MouseEvent = ();
 
 pub fn cached_rendering_data(item: VRef<'_, ItemVTable>) -> &CachedRenderingData {
     unsafe {
-        &*(VRef::get_ptr(&item).as_ptr().offset(item.get_vtable().cached_rendering_data_offset)
+        &*(item.as_ptr().offset(item.get_vtable().cached_rendering_data_offset)
             as *const CachedRenderingData)
     }
 }
 
 pub fn cached_rendering_data_mut(item: VRefMut<'_, ItemVTable>) -> &mut CachedRenderingData {
     unsafe {
-        &mut *(VRefMut::get_ptr(&item).as_ptr().offset(item.get_vtable().cached_rendering_data_offset)
+        &mut *(item.as_ptr().offset(item.get_vtable().cached_rendering_data_offset)
             as *mut CachedRenderingData)
     }
 }
@@ -156,9 +156,7 @@ fn visit_internal<State>(
             let item = unsafe {
                 ItemRef::from_raw(
                     NonNull::new_unchecked(*vtable as *mut _),
-                    NonNull::new_unchecked(
+                    NonNull::new_unchecked(component.as_ptr().offset(*offset) as *mut _),
-                        (VRef::get_ptr(&component).as_ptr()).offset(*offset) as *mut _
-                    ),
                 )
             };
             let state = visitor(item, state);
@@ -191,8 +189,7 @@ fn visit_internal_mut<State>(
                 ItemRefMut::from_raw(
                     NonNull::new_unchecked(*vtable as *mut _),
                     NonNull::new_unchecked(
-                        (VRefMut::get_ptr(&component).as_ptr() as *mut u8).offset(*offset)
+                        (component.as_ptr() as *mut u8).offset(*offset) as *mut _
-                            as *mut _,
                     ),
                 )
             };
@@ -296,8 +293,6 @@ pub static QT_BUTTON_VTABLE: ItemVTable = ItemVTable {
 };
 */
 
-
-
 #[allow(non_upper_case_globals)]
 #[no_mangle]
 pub static RectangleVTable: ItemVTable = ItemVTable_static!(crate::abi::primitives::Rectangle);

sixtyfps_runtime/corelib/abi/primitives.rs

@@ -80,5 +80,4 @@ impl ItemConsts for Image {
         Image::field_offsets().cached_rendering_data as isize;
 }
 
-
 pub use super::datastructures::{ImageVTable, RectangleVTable};

tools/viewer/main.rs

@@ -69,14 +69,10 @@ struct MyComponentType {
     it: Vec<corelib::abi::datastructures::ItemTreeNode>,
 }
 
-extern "C" fn item_tree(
+extern "C" fn item_tree(r: ComponentRef<'_>) -> *const corelib::abi::datastructures::ItemTreeNode {
-    r: ComponentRef<'_>,
-) -> *const corelib::abi::datastructures::ItemTreeNode {
     // FIXME! unsafe is not correct here, as the ComponentVTable might not be a MyComponentType
     // (one can safely take a copy of the vtable and call the create function to get a box)
-    unsafe {
+    unsafe { (*(r.get_vtable() as *const ComponentVTable as *const MyComponentType)).it.as_ptr() }
-        (*(ComponentRef::get_vtable(&r) as *const ComponentVTable as *const MyComponentType)).it.as_ptr()
-    }
 }
 
 struct RuntimeTypeInfo {