Remove bounds and type checks from IngredientCache

components/salsa-macro-rules/src/setup_accumulator_impl.rs

@@ -34,9 +34,13 @@ macro_rules! setup_accumulator_impl {
                 static $CACHE: $zalsa::IngredientCache<$zalsa_struct::IngredientImpl<$Struct>> =
                     $zalsa::IngredientCache::new();
 
-                $CACHE.get_or_create(zalsa, || {
-                    zalsa.lookup_jar_by_type::<$zalsa_struct::JarImpl<$Struct>>()
-                })
+                // SAFETY: `lookup_jar_by_type` returns a valid ingredient index, and the only
+                // ingredient created by our jar is the struct ingredient.
+                unsafe {
+                    $CACHE.get_or_create(zalsa, || {
+                        zalsa.lookup_jar_by_type::<$zalsa_struct::JarImpl<$Struct>>()
+                    })
+                }
             }
 
             impl $zalsa::Accumulator for $Struct {

components/salsa-macro-rules/src/setup_input_struct.rs

@@ -109,9 +109,13 @@ macro_rules! setup_input_struct {
                     static CACHE: $zalsa::IngredientCache<$zalsa_struct::IngredientImpl<$Configuration>> =
                         $zalsa::IngredientCache::new();
 
-                    CACHE.get_or_create(zalsa, || {
-                        zalsa.lookup_jar_by_type::<$zalsa_struct::JarImpl<$Configuration>>()
-                    })
+                    // SAFETY: `lookup_jar_by_type` returns a valid ingredient index, and the only
+                    // ingredient created by our jar is the struct ingredient.
+                    unsafe {
+                        CACHE.get_or_create(zalsa, || {
+                            zalsa.lookup_jar_by_type::<$zalsa_struct::JarImpl<$Configuration>>()
+                        })
+                    }
                 }
 
                 pub fn ingredient_mut(db: &mut dyn $zalsa::Database) -> (&mut $zalsa_struct::IngredientImpl<Self>, &mut $zalsa::Runtime) {

components/salsa-macro-rules/src/setup_interned_struct.rs

@@ -157,9 +157,14 @@ macro_rules! setup_interned_struct {
                         $zalsa::IngredientCache::new();
 
                     let zalsa = db.zalsa();
-                    CACHE.get_or_create(zalsa, || {
-                        zalsa.lookup_jar_by_type::<$zalsa_struct::JarImpl<$Configuration>>()
-                    })
+
+                    // SAFETY: `lookup_jar_by_type` returns a valid ingredient index, and the only
+                    // ingredient created by our jar is the struct ingredient.
+                    unsafe {
+                        CACHE.get_or_create(zalsa, || {
+                            zalsa.lookup_jar_by_type::<$zalsa_struct::JarImpl<$Configuration>>()
+                        })
+                    }
                 }
             }
 

components/salsa-macro-rules/src/setup_tracked_fn.rs

@@ -165,9 +165,13 @@ macro_rules! setup_tracked_fn {
             impl $Configuration {
                 fn fn_ingredient(db: &dyn $Db) -> &$zalsa::function::IngredientImpl<$Configuration> {
                     let zalsa = db.zalsa();
-                    $FN_CACHE
-                        .get_or_create(zalsa, || zalsa.lookup_jar_by_type::<$fn_name>())
-                        .get_or_init(|| <dyn $Db as $Db>::zalsa_register_downcaster(db))
+
+                    // SAFETY: `lookup_jar_by_type` returns a valid ingredient index, and the first
+                    // ingredient created by our jar is the function ingredient.
+                    unsafe {
+                        $FN_CACHE.get_or_create(zalsa, || zalsa.lookup_jar_by_type::<$fn_name>())
+                    }
+                    .get_or_init(|| <dyn $Db as $Db>::zalsa_register_downcaster(db))
                 }
 
                 pub fn fn_ingredient_mut(db: &mut dyn $Db) -> &mut $zalsa::function::IngredientImpl<Self> {
@@ -185,9 +189,14 @@ macro_rules! setup_tracked_fn {
                         db: &dyn $Db,
                     ) -> &$zalsa::interned::IngredientImpl<$Configuration> {
                         let zalsa = db.zalsa();
-                        $INTERN_CACHE.get_or_create(zalsa, || {
-                            zalsa.lookup_jar_by_type::<$fn_name>().successor(0)
-                        })
+
+                        // SAFETY: `lookup_jar_by_type` returns a valid ingredient index, and the second
+                        // ingredient created by our jar is the interned ingredient (given `needs_interner`).
+                        unsafe {
+                            $INTERN_CACHE.get_or_create(zalsa, || {
+                                zalsa.lookup_jar_by_type::<$fn_name>().successor(0)
+                            })
+                        }
                     }
                 }
             }

components/salsa-macro-rules/src/setup_tracked_struct.rs

@@ -196,9 +196,13 @@ macro_rules! setup_tracked_struct {
                     static CACHE: $zalsa::IngredientCache<$zalsa_struct::IngredientImpl<$Configuration>> =
                         $zalsa::IngredientCache::new();
 
-                    CACHE.get_or_create(zalsa, || {
-                        zalsa.lookup_jar_by_type::<$zalsa_struct::JarImpl<$Configuration>>()
-                    })
+                    // SAFETY: `lookup_jar_by_type` returns a valid ingredient index, and the only
+                    // ingredient created by our jar is the struct ingredient.
+                    unsafe {
+                        CACHE.get_or_create(zalsa, || {
+                            zalsa.lookup_jar_by_type::<$zalsa_struct::JarImpl<$Configuration>>()
+                        })
+                    }
                 }
             }
 

src/ingredient.rs

@@ -177,7 +177,8 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync {
 }
 
 impl dyn Ingredient {
-    /// Equivalent to the `downcast` methods on `any`.
+    /// Equivalent to the `downcast` method on `Any`.
+    ///
     /// Because we do not have dyn-upcasting support, we need this workaround.
     pub fn assert_type<T: Any>(&self) -> &T {
         assert_eq!(
@@ -192,7 +193,27 @@ impl dyn Ingredient {
         unsafe { transmute_data_ptr(self) }
     }
 
-    /// Equivalent to the `downcast` methods on `any`.
+    /// Equivalent to the `downcast` methods on `Any`.
+    ///
+    /// Because we do not have dyn-upcasting support, we need this workaround.
+    ///
+    /// # Safety
+    ///
+    /// The contained value must be of type `T`.
+    pub unsafe fn assert_type_unchecked<T: Any>(&self) -> &T {
+        debug_assert_eq!(
+            self.type_id(),
+            TypeId::of::<T>(),
+            "ingredient `{self:?}` is not of type `{}`",
+            std::any::type_name::<T>()
+        );
+
+        // SAFETY: Guaranteed by caller.
+        unsafe { transmute_data_ptr(self) }
+    }
+
+    /// Equivalent to the `downcast` method on `Any`.
+    ///
     /// Because we do not have dyn-upcasting support, we need this workaround.
     pub fn assert_type_mut<T: Any>(&mut self) -> &mut T {
         assert_eq!(

src/ingredient_cache.rs

@@ -47,7 +47,12 @@ mod imp {
         /// Get a reference to the ingredient in the database.
         ///
         /// If the ingredient index is not already in the cache, it will be loaded and cached.
-        pub fn get_or_create<'db>(
+        ///
+        /// # Safety
+        ///
+        /// The `IngredientIndex` returned by the closure must reference a valid ingredient of
+        /// type `I` in the provided zalsa database.
+        pub unsafe fn get_or_create<'db>(
             &self,
             zalsa: &'db Zalsa,
             load_index: impl Fn() -> IngredientIndex,
@@ -57,9 +62,21 @@ mod imp {
                 ingredient_index = self.get_or_create_index_slow(load_index).as_u32();
             };
 
-            zalsa
-                .lookup_ingredient(IngredientIndex::from_unchecked(ingredient_index))
-                .assert_type()
+            // SAFETY: `ingredient_index` is initialized from a valid `IngredientIndex`.
+            let ingredient_index = unsafe { IngredientIndex::new_unchecked(ingredient_index) };
+
+            // SAFETY: There are a two cases here:
+            // - The `create_index` closure was called due to the data being uncached. In this
+            //   case, the caller guarantees the index is in-bounds and has the correct type.
+            // - The index was cached. While the current database might not be the same database
+            //   the ingredient was initially loaded from, the `inventory` feature is enabled, so
+            //   ingredient indices are stable across databases. Thus the index is still in-bounds
+            //   and has the correct type.
+            unsafe {
+                zalsa
+                    .lookup_ingredient_unchecked(ingredient_index)
+                    .assert_type_unchecked()
+            }
         }
 
         #[cold]
@@ -134,14 +151,30 @@ mod imp {
         /// Get a reference to the ingredient in the database.
         ///
         /// If the ingredient is not already in the cache, it will be created.
+        ///
+        /// # Safety
+        ///
+        /// The `IngredientIndex` returned by the closure must reference a valid ingredient of
+        /// type `I` in the provided zalsa database.
         #[inline(always)]
-        pub fn get_or_create<'db>(
+        pub unsafe fn get_or_create<'db>(
             &self,
             zalsa: &'db Zalsa,
             create_index: impl Fn() -> IngredientIndex,
         ) -> &'db I {
             let index = self.get_or_create_index(zalsa, create_index);
-            zalsa.lookup_ingredient(index).assert_type::<I>()
+
+            // SAFETY: There are a two cases here:
+            // - The `create_index` closure was called due to the data being uncached for the
+            //   provided database. In this case, the caller guarantees the index is in-bounds
+            //   and has the correct type.
+            // - We verified the index was cached for the same database, by the nonce check.
+            //   Thus the initial safety argument still applies.
+            unsafe {
+                zalsa
+                    .lookup_ingredient_unchecked(index)
+                    .assert_type_unchecked::<I>()
+            }
         }
 
         pub fn get_or_create_index(
@@ -159,7 +192,9 @@ mod imp {
             };
 
             // Unpack our `u64` into the nonce and index.
-            let index = IngredientIndex::from_unchecked(cached_data as u32);
+            //
+            // SAFETY: The lower bits of `cached_data` are initialized from a valid `IngredientIndex`.
+            let index = unsafe { IngredientIndex::new_unchecked(cached_data as u32) };
 
             // SAFETY: We've checked against `UNINITIALIZED` (0) above and so the upper bits must be non-zero.
             let nonce = crate::nonce::Nonce::<StorageNonce>::from_u32(unsafe {

src/tracked_struct.rs

@@ -976,19 +976,19 @@ mod tests {
         let mut d = DisambiguatorMap::default();
         // set up all 4 permutations of differing field values
         let h1 = IdentityHash {
-            ingredient_index: IngredientIndex::from(0),
+            ingredient_index: IngredientIndex::new(0),
             hash: 0,
         };
         let h2 = IdentityHash {
-            ingredient_index: IngredientIndex::from(1),
+            ingredient_index: IngredientIndex::new(1),
             hash: 0,
         };
         let h3 = IdentityHash {
-            ingredient_index: IngredientIndex::from(0),
+            ingredient_index: IngredientIndex::new(0),
             hash: 1,
         };
         let h4 = IdentityHash {
-            ingredient_index: IngredientIndex::from(1),
+            ingredient_index: IngredientIndex::new(1),
             hash: 1,
         };
         assert_eq!(d.disambiguate(h1), Disambiguator(0));
@@ -1006,42 +1006,42 @@ mod tests {
         let mut d = IdentityMap::default();
         // set up all 8 permutations of differing field values
         let i1 = Identity {
-            ingredient_index: IngredientIndex::from(0),
+            ingredient_index: IngredientIndex::new(0),
             hash: 0,
             disambiguator: Disambiguator(0),
         };
         let i2 = Identity {
-            ingredient_index: IngredientIndex::from(1),
+            ingredient_index: IngredientIndex::new(1),
             hash: 0,
             disambiguator: Disambiguator(0),
         };
         let i3 = Identity {
-            ingredient_index: IngredientIndex::from(0),
+            ingredient_index: IngredientIndex::new(0),
             hash: 1,
             disambiguator: Disambiguator(0),
         };
         let i4 = Identity {
-            ingredient_index: IngredientIndex::from(1),
+            ingredient_index: IngredientIndex::new(1),
             hash: 1,
             disambiguator: Disambiguator(0),
         };
         let i5 = Identity {
-            ingredient_index: IngredientIndex::from(0),
+            ingredient_index: IngredientIndex::new(0),
             hash: 0,
             disambiguator: Disambiguator(1),
         };
         let i6 = Identity {
-            ingredient_index: IngredientIndex::from(1),
+            ingredient_index: IngredientIndex::new(1),
             hash: 0,
             disambiguator: Disambiguator(1),
         };
         let i7 = Identity {
-            ingredient_index: IngredientIndex::from(0),
+            ingredient_index: IngredientIndex::new(0),
             hash: 1,
             disambiguator: Disambiguator(1),
         };
         let i8 = Identity {
-            ingredient_index: IngredientIndex::from(1),
+            ingredient_index: IngredientIndex::new(1),
             hash: 1,
             disambiguator: Disambiguator(1),
         };

src/zalsa.rs

@@ -80,14 +80,18 @@ impl IngredientIndex {
     const MAX_INDEX: u32 = 0x7FFF_FFFF;
 
     /// Create an ingredient index from a `u32`.
-    pub(crate) fn from(v: u32) -> Self {
+    pub(crate) fn new(v: u32) -> Self {
         assert!(v <= Self::MAX_INDEX);
         Self(v)
     }
 
     /// Create an ingredient index from a `u32`, without performing validating
     /// that the index is valid.
-    pub(crate) fn from_unchecked(v: u32) -> Self {
+    ///
+    /// # Safety
+    ///
+    /// The index must be less than or equal to `IngredientIndex::MAX_INDEX`.
+    pub(crate) unsafe fn new_unchecked(v: u32) -> Self {
         Self(v)
     }
 
@@ -236,6 +240,7 @@ impl Zalsa {
         unsafe { table.memos(id, self.current_revision()) }
     }
 
+    /// Returns the ingredient at the given index, or panics if it is out-of-bounds.
     #[inline]
     pub fn lookup_ingredient(&self, index: IngredientIndex) -> &dyn Ingredient {
         let index = index.as_u32() as usize;
@@ -245,6 +250,19 @@ impl Zalsa {
             .as_ref()
     }
 
+    /// Returns the ingredient at the given index.
+    ///
+    /// # Safety
+    ///
+    /// The index must be in-bounds.
+    #[inline]
+    pub unsafe fn lookup_ingredient_unchecked(&self, index: IngredientIndex) -> &dyn Ingredient {
+        let index = index.as_u32() as usize;
+
+        // SAFETY: Guaranteed by caller.
+        unsafe { self.ingredients_vec.get_unchecked(index).as_ref() }
+    }
+
     pub(crate) fn ingredient_index_for_memo(
         &self,
         struct_ingredient_index: IngredientIndex,
@@ -331,7 +349,7 @@ impl Zalsa {
     fn insert_jar(&mut self, jar: ErasedJar) {
         let jar_type_id = (jar.type_id)();
 
-        let index = IngredientIndex::from(self.ingredients_vec.len() as u32);
+        let index = IngredientIndex::new(self.ingredients_vec.len() as u32);
 
         if self.jar_map.contains_key(&jar_type_id) {
             return;

src/zalsa_local.rs

@@ -754,7 +754,11 @@ impl QueryOrigin {
             QueryOriginKind::Assigned => {
                 // SAFETY: `data.index` is initialized when the tag is `QueryOriginKind::Assigned`.
                 let index = unsafe { self.data.index };
-                let ingredient_index = IngredientIndex::from(self.metadata);
+
+                // SAFETY: `metadata` is initialized from a valid `IngredientIndex` when the tag
+                // is `QueryOriginKind::Assigned`.
+                let ingredient_index = unsafe { IngredientIndex::new_unchecked(self.metadata) };
+
                 QueryOriginRef::Assigned(DatabaseKeyIndex::new(ingredient_index, index))
             }
 

tests/interned-structs_self_ref.rs

@@ -99,9 +99,14 @@ const _: () = {
                 zalsa_::IngredientCache::new();
 
             let zalsa = db.zalsa();
-            CACHE.get_or_create(zalsa, || {
-                zalsa.lookup_jar_by_type::<zalsa_struct_::JarImpl<Configuration_>>()
-            })
+
+            // SAFETY: `lookup_jar_by_type` returns a valid ingredient index, and the only
+            // ingredient created by our jar is the struct ingredient.
+            unsafe {
+                CACHE.get_or_create(zalsa, || {
+                    zalsa.lookup_jar_by_type::<zalsa_struct_::JarImpl<Configuration_>>()
+                })
+            }
         }
     }
     impl zalsa_::AsId for InternedString<'_> {