diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 666ae9ab..220f59f9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,7 @@ jobs: - nightly - beta - stable - - 1.59.0 # MSRV + - 1.65.0 # MSRV timeout-minutes: 10 @@ -62,7 +62,6 @@ jobs: miri: name: "Miri" runs-on: ubuntu-latest - continue-on-error: true # Needed until all Miri errors are fixed steps: - uses: actions/checkout@v3 - name: Install Miri @@ -71,4 +70,7 @@ jobs: rustup override set nightly cargo miri setup - name: Test with Miri - run: cargo miri test + # Miri currently reports leaks in some tests so we disable that check + # here (might be due to ptr-int-ptr in crossbeam-epoch so might be + # resolved in future versions of that crate). + run: MIRIFLAGS="-Zmiri-ignore-leaks" cargo miri test diff --git a/Cargo.toml b/Cargo.toml index 5e7a3925..ee77eaf5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ categories = ["concurrency"] license = "MIT OR Apache-2.0" exclude = ["bors.toml", ".travis.yml"] edition = "2021" -rust-version = "1.59.0" +rust-version = "1.65.0" [dependencies] ahash = "0.7.6" @@ -35,6 +35,7 @@ shred-derive = { path = "shred-derive", version = "0.6.3" } [features] default = ["parallel", "shred-derive"] parallel = ["rayon"] +nightly = [] [[example]] name = "async" diff --git a/benches/bench.rs b/benches/bench.rs index 4d3a20ec..ccb4508b 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -1,9 +1,5 @@ #![feature(test)] -extern crate cgmath; -extern crate shred; -#[macro_use] -extern crate shred_derive; extern crate test; use std::ops::{Index, IndexMut}; diff --git a/clippy.toml b/clippy.toml index 8622df3f..a0f7d24a 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1,2 +1,2 @@ -msrv = "1.56.1" +msrv = "1.65.0" disallowed-types = ["std::collections::HashMap"] diff --git a/examples/basic_dispatch.rs b/examples/basic_dispatch.rs index 9dccf243..d6e04b64 100644 --- a/examples/basic_dispatch.rs +++ b/examples/basic_dispatch.rs @@ -22,7 +22,7 @@ impl<'a> System<'a> for PrintSystem { println!("{:?}", &*b); *b = ResB; // We can mutate ResB here - // because it's `Write`. + // because it's `Write`. } } diff --git a/examples/dyn_sys_data.rs b/examples/dyn_sys_data.rs index 93e5fc84..a5bacea0 100644 --- a/examples/dyn_sys_data.rs +++ b/examples/dyn_sys_data.rs @@ -99,11 +99,7 @@ unsafe impl CastFrom for dyn Reflection where T: Reflection + 'static, { - fn cast(t: &T) -> &Self { - t - } - - fn cast_mut(t: &mut T) -> &mut Self { + fn cast(t: *mut T) -> *mut Self { t } } @@ -253,8 +249,8 @@ fn main() { { let mut table = res.entry().or_insert_with(ReflectionTable::new); - table.register(&Foo); - table.register(&Bar); + table.register::(); + table.register::(); } { diff --git a/src/lib.rs b/src/lib.rs index 8bb91848..7857e6dc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,4 @@ +#![cfg_attr(feature = "nightly", feature(ptr_metadata, strict_provenance))] //! **Sh**ared **re**source **d**ispatcher //! //! This library allows to dispatch @@ -82,6 +83,7 @@ //! virtual function calls. #![deny(unused_must_use, clippy::disallowed_types)] +#![deny(unsafe_op_in_unsafe_fn)] #![warn(missing_docs)] /// Re-exports from [`atomic_refcell`] diff --git a/src/meta.rs b/src/meta.rs index 71898003..5982eccb 100644 --- a/src/meta.rs +++ b/src/meta.rs @@ -2,8 +2,12 @@ use std::{any::TypeId, collections::hash_map::Entry, marker::PhantomData}; use ahash::AHashMap as HashMap; +use crate::cell::{AtomicRef, AtomicRefMut}; use crate::{Resource, ResourceId, World}; +#[cfg(feature = "nightly")] +use core::ptr::{DynMetadata, Pointee}; + /// This implements `Send` and `Sync` unconditionally. /// (the trait itself doesn't need to have these bounds and the /// resources are already guaranteed to fulfill it). @@ -14,13 +18,14 @@ unsafe impl Send for Invariant where T: ?Sized {} unsafe impl Sync for Invariant where T: ?Sized {} /// Helper trait for the `MetaTable`. +/// /// This trait is required to be implemented for a trait to be compatible with /// the meta table. /// /// # Safety /// -/// Not casting `self` but e.g. a field to the trait object will result in -/// Undefined Bahavior. +/// The produced pointer must have the same provenance and address as the +/// provided pointer and a vtable that is valid for the type `T`. /// /// # Examples /// @@ -36,26 +41,22 @@ unsafe impl Sync for Invariant where T: ?Sized {} /// where /// T: Foo + 'static, /// { -/// fn cast(t: &T) -> &(dyn Foo + 'static) { -/// t -/// } -/// -/// fn cast_mut(t: &mut T) -> &mut (dyn Foo + 'static) { +/// fn cast(t: *mut T) -> *mut (dyn Foo + 'static) { /// t /// } /// } /// ``` pub unsafe trait CastFrom { - /// Casts an immutable `T` reference to a trait object. - fn cast(t: &T) -> &Self; - - /// Casts a mutable `T` reference to a trait object. - fn cast_mut(t: &mut T) -> &mut Self; + /// Casts a concrete pointer to `T` to a trait object pointer. + fn cast(t: *mut T) -> *mut Self; } /// An iterator for the `MetaTable`. pub struct MetaIter<'a, T: ?Sized + 'a> { - fat: &'a [Fat], + #[cfg(not(feature = "nightly"))] + vtable_fns: &'a [fn(*mut ()) -> *mut T], + #[cfg(feature = "nightly")] + vtables: &'a [DynMetadata], index: usize, tys: &'a [TypeId], // `MetaIter` is invariant over `T` @@ -63,65 +64,93 @@ pub struct MetaIter<'a, T: ?Sized + 'a> { world: &'a World, } +#[cfg(not(feature = "nightly"))] impl<'a, T> Iterator for MetaIter<'a, T> where T: ?Sized + 'a, { - type Item = &'a T; + type Item = AtomicRef<'a, T>; + #[allow(clippy::borrowed_box)] // variant of https://github.com/rust-lang/rust-clippy/issues/5770 fn next(&mut self) -> Option<::Item> { - let index = self.index; - self.index += 1; - - // Ugly hack that works due to `UnsafeCell` and distinct resources. - unsafe { - self.world - // SAFETY: We just read the value and don't replace it. - .try_fetch_internal(match self.tys.get(index) { - Some(&x) => ResourceId::from_type_id(x), - None => return None, - }) - .map(|res| { - self.fat[index] - .create_ptr::(Box::as_ref(&res.borrow()) as *const dyn Resource as *const - ()) - }) - // we lengthen the lifetime from `'_` to `'a` here, see above - .map(|ptr| &*ptr) - .or_else(|| self.next()) + loop { + let resource_id = match self.tys.get(self.index) { + Some(&x) => ResourceId::from_type_id(x), + None => return None, + }; + + let index = self.index; + self.index += 1; + + // SAFETY: We just read the value and don't replace it. + if let Some(res) = unsafe { self.world.try_fetch_internal(resource_id) } { + let vtable_fn = self.vtable_fns[index]; + let trait_object = AtomicRef::map(res.borrow(), |res: &Box| { + let ptr: *const dyn Resource = Box::as_ref(res); + let trait_ptr = (vtable_fn)(ptr.cast::<()>().cast_mut()); + // SAFETY: For a particular index we store a corresponding + // TypeId and vtable_fn in tys and vtable_fns respectively. + // We rely on `try_fetch_interal` returning a trait object + // with a concrete type that has the provided TypeId. The + // signature of the closure parameter of `AtomicRef::map` + // should ensure we aren't accidentally extending the + // lifetime here. Also see safety note in `MetaTable::get`. + unsafe { &*trait_ptr } + }); + + return Some(trait_object); + } } } } -struct Fat(usize); - -impl Fat { - pub unsafe fn from_ptr(t: &T) -> Self { - use std::ptr::read; - - assert_unsized::(); - - let fat_ptr = &t as *const &T as *const usize; - // Memory layout: - // [object pointer, vtable pointer] - // ^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^ - // 8 bytes | 8 bytes - // (on 32-bit both have 4 bytes) - let vtable = read::(fat_ptr.offset(1)); - - Fat(vtable) - } - - pub unsafe fn create_ptr(&self, ptr: *const ()) -> *const T { - let fat_ptr: (*const (), usize) = (ptr, self.0); +#[cfg(feature = "nightly")] +impl<'a, T> Iterator for MetaIter<'a, T> +where + T: ?Sized + 'a, + T: Pointee>, +{ + type Item = AtomicRef<'a, T>; - *(&fat_ptr as *const (*const (), usize) as *const *const T) + #[allow(clippy::borrowed_box)] // variant of https://github.com/rust-lang/rust-clippy/issues/5770 + fn next(&mut self) -> Option<::Item> { + loop { + let resource_id = match self.tys.get(self.index) { + Some(&x) => ResourceId::from_type_id(x), + None => return None, + }; + + let index = self.index; + self.index += 1; + + // SAFETY: We just read the value and don't replace it. + if let Some(res) = unsafe { self.world.try_fetch_internal(resource_id) } { + let vtable = self.vtables[index]; + let trait_object = AtomicRef::map(res.borrow(), |res: &Box| { + let ptr: *const dyn Resource = Box::as_ref(res); + let trait_ptr = core::ptr::from_raw_parts(ptr.cast::<()>(), vtable); + // SAFETY: For a particular index we store a corresponding + // TypeId and vtable in tys and vtables respectively. + // We rely on `try_fetch_interal` returning a trait object + // with a concrete type that has the provided TypeId. The + // signature of the closure parameter of `AtomicRef::map` + // should ensure we aren't accidentally extending the + // lifetime here. Also see safety note in `MetaTable::get`. + unsafe { &*trait_ptr } + }); + + return Some(trait_object); + } + } } } /// A mutable iterator for the `MetaTable`. pub struct MetaIterMut<'a, T: ?Sized + 'a> { - fat: &'a [Fat], + #[cfg(not(feature = "nightly"))] + vtable_fns: &'a [fn(*mut ()) -> *mut T], + #[cfg(feature = "nightly")] + vtables: &'a [DynMetadata], index: usize, tys: &'a [TypeId], // `MetaIterMut` is invariant over `T` @@ -129,36 +158,122 @@ pub struct MetaIterMut<'a, T: ?Sized + 'a> { world: &'a World, } +#[cfg(not(feature = "nightly"))] impl<'a, T> Iterator for MetaIterMut<'a, T> where T: ?Sized + 'a, { - type Item = &'a mut T; + type Item = AtomicRefMut<'a, T>; fn next(&mut self) -> Option<::Item> { - let index = self.index; - self.index += 1; - - // Ugly hack that works due to `UnsafeCell` and distinct resources. - unsafe { - self.world - // SAFETY: We don't swap out the Box or expose a mutable reference to it. - .try_fetch_internal(match self.tys.get(index) { - Some(&x) => ResourceId::from_type_id(x), - None => return None, - }) - .map(|res| { - self.fat[index].create_ptr::(Box::as_mut(&mut res.borrow_mut()) - as *mut dyn Resource - as *const ()) as *mut T - }) - // we lengthen the lifetime from `'_` to `'a` here, see above - .map(|ptr| &mut *ptr) - .or_else(|| self.next()) + loop { + let resource_id = match self.tys.get(self.index) { + Some(&x) => ResourceId::from_type_id(x), + None => return None, + }; + + let index = self.index; + self.index += 1; + + // Note: this relies on implementation details of + // try_fetch_internal! + // SAFETY: We don't swap out the Box or expose a mutable reference to it. + if let Some(res) = unsafe { self.world.try_fetch_internal(resource_id) } { + let vtable_fn = self.vtable_fns[index]; + let trait_object = + AtomicRefMut::map(res.borrow_mut(), |res: &mut Box| { + let ptr: *mut dyn Resource = Box::as_mut(res); + let trait_ptr = (vtable_fn)(ptr.cast::<()>()); + // SAFETY: For a particular index we store a corresponding + // TypeId and vtable_fn in tys and vtable_fns respectively. + // We rely on `try_fetch_interal` returning a trait object + // with a concrete type that has the provided TypeId. The + // signature of the closure parameter of `AtomicRefMut::map` + // should ensure we aren't accidentally extending the + // lifetime here. Also see safety note in + // `MetaTable::get_mut`. + unsafe { &mut *trait_ptr } + }); + + return Some(trait_object); + } } } } +#[cfg(feature = "nightly")] +impl<'a, T> Iterator for MetaIterMut<'a, T> +where + T: ?Sized + 'a, + T: Pointee>, +{ + type Item = AtomicRefMut<'a, T>; + + fn next(&mut self) -> Option<::Item> { + loop { + let resource_id = match self.tys.get(self.index) { + Some(&x) => ResourceId::from_type_id(x), + None => return None, + }; + + let index = self.index; + self.index += 1; + + // Note: this relies on implementation details of + // try_fetch_internal! + // SAFETY: We don't swap out the Box or expose a mutable reference to it. + if let Some(res) = unsafe { self.world.try_fetch_internal(resource_id) } { + let vtable = self.vtables[index]; + let trait_object = + AtomicRefMut::map(res.borrow_mut(), |res: &mut Box| { + let ptr: *mut dyn Resource = Box::as_mut(res); + let trait_ptr = core::ptr::from_raw_parts_mut(ptr.cast::<()>(), vtable); + // SAFETY: For a particular index we store a corresponding + // TypeId and vtable in tys and vtables respectively. + // We rely on `try_fetch_interal` returning a trait object + // with a concrete type that has the provided TypeId. The + // signature of the closure parameter of `AtomicRefMut::map` + // should ensure we aren't accidentally extending the + // lifetime here. Also see safety note in + // `MetaTable::get_mut`. + unsafe { &mut *trait_ptr } + }); + + return Some(trait_object); + } + } + } +} + +/// Given an address and provenance, produces a pointer to a trait object for +/// which `CastFrom` is implemented. +/// +/// Returned pointer has: +/// * the provenance of the provided pointer +/// * the address of the provided pointer +/// * a vtable that is valid for the concrete type `T` +/// +/// We exclusively operate on pointers here so we only need a single function +/// pointer in the meta-table for both `&T` and `&mut T` cases. +#[cfg(not(feature = "nightly"))] +fn attach_vtable(value: *mut ()) -> *mut TraitObject +where + TraitObject: CastFrom + 'static, + T: core::any::Any, +{ + // NOTE: This should be equivalent to `Any::downcast_ref_unchecked` except + // with pointers and we don't require `Any` trait but still require that the + // types are 'static. + let trait_ptr = >::cast(value.cast::()); + // TODO: use `.addr()` when stabilized + // assert that address not changed (to catch some mistakes in CastFrom impl) + assert!( + core::ptr::eq(value, trait_ptr.cast::<()>()), + "Bug: `CastFrom` did not cast `self`" + ); + trait_ptr +} + /// The `MetaTable` which allows to store object-safe trait implementations for /// resources. /// @@ -182,11 +297,7 @@ where /// where /// T: Object + 'static, /// { -/// fn cast(t: &T) -> &Self { -/// t -/// } -/// -/// fn cast_mut(t: &mut T) -> &mut Self { +/// fn cast(t: *mut T) -> *mut Self { /// t /// } /// } @@ -221,8 +332,8 @@ where /// world.insert(ImplementorB(1)); /// /// let mut table = MetaTable::::new(); -/// table.register(&ImplementorA(31415)); // Can just be some instance of type `&ImplementorA`. -/// table.register(&ImplementorB(27182)); +/// table.register::(); +/// table.register::(); /// /// { /// let mut iter = table.iter(&mut world); @@ -231,7 +342,10 @@ where /// } /// ``` pub struct MetaTable { - fat: Vec, + #[cfg(not(feature = "nightly"))] + vtable_fns: Vec *mut T>, + #[cfg(feature = "nightly")] + vtables: Vec>, indices: HashMap, tys: Vec, // `MetaTable` is invariant over `T` @@ -241,31 +355,59 @@ pub struct MetaTable { impl MetaTable { /// Creates a new `MetaTable`. pub fn new() -> Self { + // TODO: when ptr_metadata is stablilized this can just be a trait bound: Pointee> assert_unsized::(); Default::default() } /// Registers a resource `R` that implements the trait `T`. - /// This just needs some instance of type `R` to retrieve the vtable. - /// It doesn't have to be the same object you're calling `get` with later. - pub fn register(&mut self, r: &R) + #[cfg(not(feature = "nightly"))] + pub fn register(&mut self) where R: Resource, T: CastFrom + 'static, { - let thin_ptr = r as *const R as usize; - let casted_ptr = >::cast(r); - let thin_casted_ptr = casted_ptr as *const T as *const () as usize; + let ty_id = TypeId::of::(); + let vtable_fn = attach_vtable::; - assert_eq!( - thin_ptr, thin_casted_ptr, - "Bug: `CastFrom` did not cast `self`" - ); + // Important: ensure no entry exists twice! + let len = self.indices.len(); + match self.indices.entry(ty_id) { + Entry::Occupied(occ) => { + let ind = *occ.get(); + + self.vtable_fns[ind] = vtable_fn; + } + Entry::Vacant(vac) => { + vac.insert(len); - let fat = unsafe { Fat::from_ptr(casted_ptr) }; + self.vtable_fns.push(vtable_fn); + self.tys.push(ty_id); + } + } + } + /// Registers a resource `R` that implements the trait `T`. + #[cfg(feature = "nightly")] + pub fn register(&mut self) + where + R: Resource, + T: CastFrom + 'static, + T: Pointee>, + { let ty_id = TypeId::of::(); + // use self.addr() for unpredictable address to use for checking consistency below + let invalid_ptr = core::ptr::invalid_mut::((self as *mut Self).addr()); + let trait_ptr = >::cast(invalid_ptr); + // assert that address not changed (to catch some mistakes in CastFrom impl) + assert_eq!( + invalid_ptr.addr(), + trait_ptr.addr(), + "Bug: `CastFrom` did not cast `self`" + ); + let vtable = core::ptr::metadata(trait_ptr); // Important: ensure no entry exists twice! let len = self.indices.len(); @@ -273,12 +415,12 @@ impl MetaTable { Entry::Occupied(occ) => { let ind = *occ.get(); - self.fat[ind] = fat; + self.vtables[ind] = vtable; } Entry::Vacant(vac) => { vac.insert(len); - self.fat.push(fat); + self.vtables.push(vtable); self.tys.push(ty_id); } } @@ -287,29 +429,89 @@ impl MetaTable { /// Tries to convert `world` to a trait object of type `&T`. /// If `world` doesn't have an implementation for `T` (or it wasn't /// registered), this will return `None`. + #[cfg(not(feature = "nightly"))] pub fn get<'a>(&self, res: &'a dyn Resource) -> Option<&'a T> { - unsafe { - self.indices - .get(&res.type_id()) - .map(move |&ind| &*self.fat[ind].create_ptr(res as *const _ as *const ())) - } + self.indices.get(&res.type_id()).map(|&ind| { + let vtable_fn = self.vtable_fns[ind]; + + let ptr = <*const dyn Resource>::cast::<()>(res).cast_mut(); + let trait_ptr = (vtable_fn)(ptr); + // SAFETY: We retrieved the `vtable_fn` via TypeId so it will attach + // a vtable that corresponds with the erased type that the TypeId + // refers to. `vtable_fn` will also preserve the provenance and + // address (so we can safely produce a shared reference since we + // started with one). + unsafe { &*trait_ptr } + }) + } + + /// Tries to convert `world` to a trait object of type `&T`. + /// If `world` doesn't have an implementation for `T` (or it wasn't + /// registered), this will return `None`. + #[cfg(feature = "nightly")] + pub fn get<'a>(&self, res: &'a dyn Resource) -> Option<&'a T> + where + T: Pointee>, + { + self.indices.get(&res.type_id()).map(|&ind| { + let vtable = self.vtables[ind]; + let ptr = <*const dyn Resource>::cast::<()>(res); + let trait_ptr = core::ptr::from_raw_parts(ptr, vtable); + // SAFETY: We retrieved the `vtable` via TypeId so it will be a + // vtable that corresponds with the erased type that the TypeId + // refers to. `from_raw_parts` will also preserve the provenance and + // address (so we can safely produce a shared reference since we + // started with one). + unsafe { &*trait_ptr } + }) } /// Tries to convert `world` to a trait object of type `&mut T`. /// If `world` doesn't have an implementation for `T` (or it wasn't /// registered), this will return `None`. - pub fn get_mut<'a>(&self, res: &'a dyn Resource) -> Option<&'a mut T> { - unsafe { - self.indices.get(&res.type_id()).map(move |&ind| { - &mut *(self.fat[ind].create_ptr::(res as *const _ as *const ()) as *mut T) - }) - } + #[cfg(not(feature = "nightly"))] + pub fn get_mut<'a>(&self, res: &'a mut dyn Resource) -> Option<&'a mut T> { + self.indices.get(&res.type_id()).map(|&ind| { + let vtable_fn = self.vtable_fns[ind]; + let ptr = <*mut dyn Resource>::cast::<()>(res); + let trait_ptr = (vtable_fn)(ptr); + // SAFETY: We retrieved the `vtable_fn` via TypeId so it will attach + // a vtable that corresponds with the erased type that the TypeId + // refers to. `vtable_fn` will also preserve the provenance and + // address (so we can safely produce a mutable reference since we + // started with one). + unsafe { &mut *trait_ptr } + }) + } + + /// Tries to convert `world` to a trait object of type `&mut T`. + /// If `world` doesn't have an implementation for `T` (or it wasn't + /// registered), this will return `None`. + #[cfg(feature = "nightly")] + pub fn get_mut<'a>(&self, res: &'a mut dyn Resource) -> Option<&'a mut T> + where + T: Pointee>, + { + self.indices.get(&res.type_id()).map(|&ind| { + let vtable = self.vtables[ind]; + let ptr = <*mut dyn Resource>::cast::<()>(res); + let trait_ptr = core::ptr::from_raw_parts_mut(ptr, vtable); + // SAFETY: We retrieved the `vtable` via TypeId so it will be a + // vtable that corresponds with the erased type that the TypeId + // refers to. `from_raw_parts_mut` will also preserve the provenance + // and address (so we can safely produce a mutable reference since + // we started with one). + unsafe { &mut *trait_ptr } + }) } /// Iterates all resources that implement `T` and were registered. pub fn iter<'a>(&'a self, res: &'a World) -> MetaIter<'a, T> { MetaIter { - fat: &self.fat, + #[cfg(not(feature = "nightly"))] + vtable_fns: &self.vtable_fns, + #[cfg(feature = "nightly")] + vtables: &self.vtables, index: 0, world: res, tys: &self.tys, @@ -320,7 +522,10 @@ impl MetaTable { /// Iterates all resources that implement `T` and were registered mutably. pub fn iter_mut<'a>(&'a self, res: &'a World) -> MetaIterMut<'a, T> { MetaIterMut { - fat: &self.fat, + #[cfg(not(feature = "nightly"))] + vtable_fns: &self.vtable_fns, + #[cfg(feature = "nightly")] + vtables: &self.vtables, index: 0, world: res, tys: &self.tys, @@ -335,7 +540,10 @@ where { fn default() -> Self { MetaTable { - fat: Default::default(), + #[cfg(not(feature = "nightly"))] + vtable_fns: Default::default(), + #[cfg(feature = "nightly")] + vtables: Default::default(), indices: Default::default(), tys: Default::default(), marker: Default::default(), @@ -344,7 +552,7 @@ where } fn assert_unsized() { - use std::mem::size_of; + use core::mem::size_of; assert_eq!(size_of::<&T>(), 2 * size_of::()); } @@ -363,11 +571,7 @@ mod tests { where T: Object + 'static, { - fn cast(t: &T) -> &Self { - t - } - - fn cast_mut(t: &mut T) -> &mut Self { + fn cast(t: *mut T) -> *mut Self { t } } @@ -404,8 +608,8 @@ mod tests { world.insert(ImplementorB(1)); let mut table = MetaTable::::new(); - table.register(&ImplementorA(125)); - table.register(&ImplementorB(111_111)); + table.register::(); + table.register::(); { let mut iter = table.iter(&world); @@ -415,10 +619,10 @@ mod tests { { let mut iter_mut = table.iter_mut(&world); - let obj = iter_mut.next().unwrap(); + let mut obj = iter_mut.next().unwrap(); obj.method2(3); assert_eq!(obj.method1(), 6); - let obj = iter_mut.next().unwrap(); + let mut obj = iter_mut.next().unwrap(); obj.method2(4); assert_eq!(obj.method1(), 4); } @@ -432,8 +636,8 @@ mod tests { world.insert(ImplementorB(1)); let mut table = MetaTable::::new(); - table.register(&ImplementorA(125)); - table.register(&ImplementorB(111_111)); + table.register::(); + table.register::(); { let mut iter = table.iter(&world); @@ -483,8 +687,8 @@ mod tests { world.insert(ImplementorD); let mut table = MetaTable::::new(); - table.register(&ImplementorC); - table.register(&ImplementorD); + table.register::(); + table.register::(); assert_eq!( table diff --git a/src/world/res_downcast/mod.rs b/src/world/res_downcast/mod.rs index 08f780de..c0dbb7e0 100644 --- a/src/world/res_downcast/mod.rs +++ b/src/world/res_downcast/mod.rs @@ -16,6 +16,7 @@ impl dyn Resource { #[inline] pub fn downcast(self: Box) -> Result, Box> { if self.is::() { + // SAFETY: We just checked that the type is `T`. unsafe { Ok(self.downcast_unchecked()) } } else { Err(self) @@ -31,7 +32,8 @@ impl dyn Resource { /// will result in UB. #[inline] pub unsafe fn downcast_unchecked(self: Box) -> Box { - Box::from_raw(Box::into_raw(self) as *mut T) + // SAFETY: Caller promises the concrete type is `T`. + unsafe { Box::from_raw(Box::into_raw(self) as *mut T) } } /// Returns true if the boxed type is the same as `T` @@ -45,6 +47,7 @@ impl dyn Resource { #[inline] pub fn downcast_ref(&self) -> Option<&T> { if self.is::() { + // SAFETY: We just checked that the type is `T`. unsafe { Some(self.downcast_ref_unchecked()) } } else { Option::None @@ -61,7 +64,8 @@ impl dyn Resource { /// will result in UB. #[inline] pub unsafe fn downcast_ref_unchecked(&self) -> &T { - &*(self as *const Self as *const T) + // SAFETY: Caller promises the concrete type is `T`. + unsafe { &*(self as *const Self as *const T) } } /// Returns some mutable reference to the boxed value if it is of type `T`, @@ -69,6 +73,7 @@ impl dyn Resource { #[inline] pub fn downcast_mut(&mut self) -> Option<&mut T> { if self.is::() { + // SAFETY: We just checked that the type is `T`. unsafe { Some(self.downcast_mut_unchecked()) } } else { Option::None @@ -85,6 +90,7 @@ impl dyn Resource { /// will result in UB. #[inline] pub unsafe fn downcast_mut_unchecked(&mut self) -> &mut T { - &mut *(self as *mut Self as *mut T) + // SAFETY: Caller promises the concrete type is `T`. + unsafe { &mut *(self as *mut Self as *mut T) } } } diff --git a/tests/meta_table_safety_113.rs b/tests/meta_table_safety_113.rs index 990ab8d4..7be0b97f 100644 --- a/tests/meta_table_safety_113.rs +++ b/tests/meta_table_safety_113.rs @@ -14,17 +14,17 @@ impl PointsToU64 for Box { struct MultipleData { _number: u64, - pointer: Box, + _pointer: Box, } unsafe impl CastFrom for dyn PointsToU64 { - fn cast(t: &MultipleData) -> &Self { - // this is wrong and will cause a panic - &t.pointer - } - - fn cast_mut(t: &mut MultipleData) -> &mut Self { - &mut t.pointer + fn cast(_t: *mut MultipleData) -> *mut Self { + // This is wrong and will cause a panic + // + // NOTE: we use this instead of constructing a pointer to the field since + // there is no way to easily and safely do that currently! (this can be + // changed if offset_of macro is added to std). + core::ptr::NonNull::>::dangling().as_ptr() } } @@ -34,9 +34,9 @@ fn test_panics() { let mut table: MetaTable = MetaTable::new(); let md = MultipleData { _number: 0x0, // this will be casted to a pointer, then dereferenced - pointer: Box::new(42), + _pointer: Box::new(42), }; - table.register(&md); + table.register::(); if let Some(t) = table.get(&md) { println!("{}", t.get_u64()); }