diff --git a/components/salsa-macros/src/xform.rs b/components/salsa-macros/src/xform.rs index 43ffa046..4c719cbe 100644 --- a/components/salsa-macros/src/xform.rs +++ b/components/salsa-macros/src/xform.rs @@ -5,7 +5,7 @@ pub(crate) struct ChangeLt<'a> { to: String, } -impl<'a> ChangeLt<'a> { +impl ChangeLt<'_> { pub fn elided_to(db_lt: &syn::Lifetime) -> Self { ChangeLt { from: Some("_"), diff --git a/src/input/setter.rs b/src/input/setter.rs index 6a636c5a..4f157565 100644 --- a/src/input/setter.rs +++ b/src/input/setter.rs @@ -45,7 +45,7 @@ where } } -impl<'setter, C, S, F> Setter for SetterImpl<'setter, C, S, F> +impl Setter for SetterImpl<'_, C, S, F> where C: Configuration, S: FnOnce(&mut C::Fields, F) -> F, diff --git a/src/table/sync.rs b/src/table/sync.rs index 6a02bc8c..dfe78a23 100644 --- a/src/table/sync.rs +++ b/src/table/sync.rs @@ -85,7 +85,7 @@ pub(crate) struct ClaimGuard<'me> { sync_table: &'me SyncTable, } -impl<'me> ClaimGuard<'me> { +impl ClaimGuard<'_> { fn remove_from_map_and_unblock_queries(&self, wait_result: WaitResult) { let mut syncs = self.sync_table.syncs.write(); @@ -101,7 +101,7 @@ impl<'me> ClaimGuard<'me> { } } -impl<'me> Drop for ClaimGuard<'me> { +impl Drop for ClaimGuard<'_> { fn drop(&mut self) { let wait_result = if std::thread::panicking() { WaitResult::Panicked diff --git a/src/views.rs b/src/views.rs index 87d66061..d9f01de0 100644 --- a/src/views.rs +++ b/src/views.rs @@ -24,65 +24,54 @@ use std::{ #[derive(Clone)] pub struct Views { source_type_id: TypeId, - view_casters: Arc>, + view_casters: Arc>, } -/// A ViewCaster contains a trait object that can cast from the +/// A DynViewCaster contains a manual trait object that can cast from the /// (ghost) `Db` type of `Views` to some (ghost) `DbView` type. /// /// You can think of the struct as looking like: /// /// ```rust,ignore -/// struct ViewCaster { +/// struct DynViewCaster { /// target_type_id: TypeId, // TypeId of DbView /// type_name: &'static str, // type name of DbView -/// cast_to: OpaqueBoxDyn, // a `Box>` that expects a `Db` -/// free_box: Box, // the same box as above, but upcast to `dyn Free` +/// view_caster: *mut (), // a `Box>` +/// cast: *const (), // a `unsafe fn (&ViewCaster, &dyn Database) -> &DbView` +/// drop: *const (), // the destructor for the box above /// } /// ``` /// -/// As you can see, we have to work very hard to manage things -/// in a way that miri is happy with. What is going on here? -/// -/// * The `cast_to` is the cast object, but we can't actually name its type, so -/// we transmute it into some opaque bytes. We can transmute it back once we -/// are in a function monormophized over some function `T` that has the same type-id -/// as `target_type_id`. -/// * The problem is that dropping `cast_to` has no effect and we need -/// to free the box! To do that, we *also* upcast the box to a `Box`. -/// This trait has no purpose but to carry a destructor. -struct ViewCaster { +/// The manual trait object and vtable allows for type erasure without +/// transmuting between fat pointers, whose layout is undefined. +struct DynViewCaster { /// The id of the target type `DbView` that we can cast to. target_type_id: TypeId, /// The name of the target type `DbView` that we can cast to. type_name: &'static str, - /// A "type-obscured" `Box>`, where `DbView` - /// is the type whose id is encoded in `target_type_id`. - cast_to: OpaqueBoxDyn, - - /// An upcasted version of `cast_to`; the only purpose of this field is - /// to be dropped in the destructor, see `ViewCaster` comment. - #[allow(dead_code)] - free_box: Box, -} + /// A pointer to a `ViewCaster`. + view_caster: *mut (), -type OpaqueBoxDyn = [u8; std::mem::size_of::>>()]; - -trait CastTo: Free { - /// # Safety requirement - /// - /// `db` must have a data pointer whose type is the `Db` type for `Self` - unsafe fn cast<'db>(&self, db: &'db dyn Database) -> &'db DbView; + /// Type-erased `ViewCaster::::vtable_cast`. + cast: *const (), - fn into_box_free(self: Box) -> Box; + /// Type-erased `ViewCaster::::drop`. + drop: unsafe fn(*mut ()), } -trait Free: Send + Sync {} +impl Drop for DynViewCaster { + fn drop(&mut self) { + // SAFETY: We own `self.caster` and are in the destructor. + unsafe { (self.drop)(self.view_caster) }; + } +} -#[allow(dead_code)] -enum Dummy {} +// SAFETY: These traits can be implemented normally as the raw pointers +// in `DynViewCaster` are only used for type-erasure. +unsafe impl Send for DynViewCaster {} +unsafe impl Sync for DynViewCaster {} impl Views { pub(crate) fn new() -> Self { @@ -107,21 +96,14 @@ impl Views { return; } - let cast_to: Box> = Box::new(func); - let cast_to: OpaqueBoxDyn = - unsafe { std::mem::transmute::>, OpaqueBoxDyn>(cast_to) }; + let view_caster = Box::into_raw(Box::new(ViewCaster(func))); - // Create a second copy of `cast_to` (which is now `Copy`) and upcast it to a `Box`. - // We will drop this box to run the destructor. - let free_box: Box = unsafe { - std::mem::transmute::>>(cast_to).into_box_free() - }; - - self.view_casters.push(ViewCaster { + self.view_casters.push(DynViewCaster { target_type_id, type_name: std::any::type_name::(), - cast_to, - free_box, + view_caster: view_caster.cast(), + cast: ViewCaster::::erased_cast as _, + drop: ViewCaster::::erased_drop, }); } @@ -138,20 +120,17 @@ impl Views { assert_eq!(self.source_type_id, db_type_id, "database type mismatch"); let view_type_id = TypeId::of::(); - for caster in self.view_casters.iter() { - if caster.target_type_id == view_type_id { - // SAFETY: We have some function that takes a thin reference to the underlying - // database type `X` and returns a (potentially wide) reference to `View`. - // - // While the compiler doesn't know what `X` is at this point, we know it's the - // same as the true type of `db_data_ptr`, and the memory representation for `()` - // and `&X` are the same (since `X` is `Sized`). - let cast_to: &OpaqueBoxDyn = &caster.cast_to; - unsafe { - let cast_to = - std::mem::transmute::<&OpaqueBoxDyn, &Box>>(cast_to); - return Some(cast_to.cast(db)); + for view in self.view_casters.iter() { + if view.target_type_id == view_type_id { + // SAFETY: We verified that this is the view caster for the + // `DbView` type by checking type IDs above. + let view = unsafe { + let caster: unsafe fn(*const (), &dyn Database) -> &DbView = + std::mem::transmute(view.cast); + caster(view.view_caster, db) }; + + return Some(view); } } @@ -159,25 +138,19 @@ impl Views { } } -impl std::fmt::Debug for Views { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("DynDowncasts") - .field("vec", &self.view_casters) - .finish() - } -} - -impl std::fmt::Debug for ViewCaster { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("DynDowncast").field(&self.type_name).finish() - } -} +/// A generic downcaster for specific `Db` and `DbView` types. +struct ViewCaster(fn(&Db) -> &DbView); -impl CastTo for fn(&Db) -> &DbView +impl ViewCaster where Db: Database, DbView: ?Sized + Any, { + /// Obtain a reference of type `DbView` from a database. + /// + /// # Safety + /// + /// The input database must be of type `Db`. unsafe fn cast<'db>(&self, db: &'db dyn Database) -> &'db DbView { // This tests the safety requirement: debug_assert_eq!(db.type_id(), TypeId::of::()); @@ -187,12 +160,41 @@ where // Caller guarantees that the input is of type `Db` // (we test it in the debug-assertion above). let db = unsafe { transmute_data_ptr::(db) }; - (*self)(db) + (self.0)(db) + } + + /// A type-erased version of `ViewCaster::::cast`. + /// + /// # Safety + /// + /// The underlying type of `caster` must be `ViewCaster::`. + unsafe fn erased_cast(caster: *mut (), db: &dyn Database) -> &DbView { + let caster = unsafe { &*caster.cast::>() }; + caster.cast(db) + } + + /// The destructor for `Box>`. + /// + /// # Safety + /// + /// All the safety requirements of `Box::>::from_raw` apply. + unsafe fn erased_drop(caster: *mut ()) { + let _: Box> = unsafe { Box::from_raw(caster.cast()) }; } +} - fn into_box_free(self: Box) -> Box { - self +impl std::fmt::Debug for Views { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Views") + .field("view_casters", &self.view_casters) + .finish() } } -impl Free for T {} +impl std::fmt::Debug for DynViewCaster { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_tuple("DynViewCaster") + .field(&self.type_name) + .finish() + } +}