chore: Document most safety blocks

components/salsa-macros/src/db.rs

@@ -143,7 +143,7 @@ impl DbMacro {
             #[inline(always)]
             unsafe fn downcast(db: &dyn salsa::plumbing::Database) -> &dyn #TraitPath where Self: Sized {
                 debug_assert_eq!(db.type_id(), ::core::any::TypeId::of::<Self>());
-                // SAFETY: Same as the safety of the `downcast` method.
+                // SAFETY: The input database must be of type `Self`.
                 unsafe { &*salsa::plumbing::transmute_data_ptr::<dyn salsa::plumbing::Database, Self>(db) }
             }
         });

src/database_impl.rs

@@ -25,10 +25,7 @@ impl Database for DatabaseImpl {
     }
 }
 
-// # Safety
-//
-// The `storage` and `storage_mut` fields return a reference to the same
-// storage field owned by `self`.
+// SAFETY: The `storage` and `storage_mut` fields return a reference to the same storage field owned by `self`.
 unsafe impl HasStorage for DatabaseImpl {
     fn storage(&self) -> &Storage<Self> {
         &self.storage

src/function.rs

@@ -179,6 +179,7 @@ where
         &'this self,
         memo: &memo::Memo<C::Output<'this>>,
     ) -> &'this memo::Memo<C::Output<'this>> {
+        // SAFETY: the caller must guarantee that the memo will not be released before `&self`
         unsafe { std::mem::transmute(memo) }
     }
 
@@ -191,16 +192,19 @@ where
     ) -> &'db memo::Memo<C::Output<'db>> {
         // We convert to a `NonNull` here as soon as possible because we are going to alias
         // into the `Box`, which is a `noalias` type.
+        // SAFETY: memo is not null
         let memo = unsafe { NonNull::new_unchecked(Box::into_raw(Box::new(memo))) };
 
-        // Unsafety conditions: memo must be in the map (it's not yet, but it will be by the time this
+        // SAFETY: memo must be in the map (it's not yet, but it will be by the time this
         // value is returned) and anything removed from map is added to deleted entries (ensured elsewhere).
         let db_memo = unsafe { self.extend_memo_lifetime(memo.as_ref()) };
 
-        // Safety: We delay the drop of `old_value` until a new revision starts which ensures no
-        // references will exist for the memo contents.
         if let Some(old_value) =
-            unsafe { self.insert_memo_into_table_for(zalsa, id, memo, memo_ingredient_index) }
+            // SAFETY: We delay the drop of `old_value` until a new revision starts which ensures no
+            // references will exist for the memo contents.
+            unsafe {
+                self.insert_memo_into_table_for(zalsa, id, memo, memo_ingredient_index)
+            }
         {
             // In case there is a reference to the old memo out there, we have to store it
             // in the deleted entries. This will get cleared when a new revision starts.

src/function/delete.rs

@@ -10,7 +10,9 @@ pub(super) struct DeletedEntries<C: Configuration> {
     memos: boxcar::Vec<SharedBox<Memo<C::Output<'static>>>>,
 }
 
+#[allow(clippy::undocumented_unsafe_blocks)] // TODO(#697) document safety
 unsafe impl<T: Send> Send for SharedBox<T> {}
+#[allow(clippy::undocumented_unsafe_blocks)] // TODO(#697) document safety
 unsafe impl<T: Sync> Sync for SharedBox<T> {}
 
 impl<C: Configuration> Default for DeletedEntries<C> {
@@ -26,6 +28,7 @@ impl<C: Configuration> DeletedEntries<C> {
     ///
     /// The memo must be valid and safe to free when the `DeletedEntries` list is cleared or dropped.
     pub(super) unsafe fn push(&self, memo: NonNull<Memo<C::Output<'_>>>) {
+        // Safety: The memo must be valid and safe to free when the `DeletedEntries` list is cleared or dropped.
         let memo = unsafe {
             std::mem::transmute::<NonNull<Memo<C::Output<'_>>>, NonNull<Memo<C::Output<'static>>>>(
                 memo,

src/function/fetch.rs

@@ -19,13 +19,12 @@ where
 
         let memo = self.refresh_memo(db, id);
         // SAFETY: We just refreshed the memo so it is guaranteed to contain a value now.
+        let memo_value = unsafe { memo.value.as_ref().unwrap_unchecked() };
         let StampedValue {
             value,
             durability,
             changed_at,
-        } = memo
-            .revisions
-            .stamped_value(unsafe { memo.value.as_ref().unwrap_unchecked() });
+        } = memo.revisions.stamped_value(memo_value);
 
         self.lru.record_use(id);
 
@@ -89,7 +88,7 @@ where
                 && self.validate_may_be_provisional(db, zalsa, database_key_index, memo)
                 && self.shallow_verify_memo(db, zalsa, database_key_index, memo)
             {
-                // Unsafety invariant: memo is present in memo_map and we have verified that it is
+                // SAFETY: memo is present in memo_map and we have verified that it is
                 // still valid for the current revision.
                 return unsafe { Some(self.extend_memo_lifetime(memo)) };
             }
@@ -124,7 +123,7 @@ where
                         && memo.revisions.cycle_heads.contains(&database_key_index)
                         && self.shallow_verify_memo(db, zalsa, database_key_index, memo)
                     {
-                        // Unsafety invariant: memo is present in memo_map.
+                        // SAFETY: memo is present in memo_map.
                         return unsafe { Some(self.extend_memo_lifetime(memo)) };
                     }
                 }
@@ -171,7 +170,7 @@ where
                     self.deep_verify_memo(db, zalsa, old_memo, &active_query)
                 {
                     if cycle_heads.is_empty() {
-                        // Unsafety invariant: memo is present in memo_map and we have verified that it is
+                        // SAFETY: memo is present in memo_map and we have verified that it is
                         // still valid for the current revision.
                         return unsafe { Some(self.extend_memo_lifetime(old_memo)) };
                     }

src/function/memo.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::undocumented_unsafe_blocks)] // TODO(#697) document safety
+
 use std::any::Any;
 use std::fmt::Debug;
 use std::fmt::Formatter;

src/interned.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::undocumented_unsafe_blocks)] // TODO(#697) document safety
+
 use dashmap::SharedValue;
 
 use crate::durability::Durability;

src/lib.rs

@@ -1,3 +1,4 @@
+#![deny(clippy::undocumented_unsafe_blocks)]
 #![forbid(unsafe_op_in_unsafe_fn)]
 
 mod accumulator;

src/revision.rs

@@ -61,9 +61,9 @@ impl From<Revision> for AtomicRevision {
 
 impl AtomicRevision {
     pub(crate) fn load(&self) -> Revision {
-        // Safety: We know that the value is non-zero because we only ever store `START` which 1, or a
-        // Revision which is guaranteed to be non-zero.
         Revision {
+            // SAFETY: We know that the value is non-zero because we only ever store `START` which 1, or a
+            // Revision which is guaranteed to be non-zero.
             generation: unsafe { NonZeroUsize::new_unchecked(self.data.load(Ordering::Acquire)) },
         }
     }

src/storage.rs

@@ -146,6 +146,7 @@ impl<Db: Database> Storage<Db> {
     // ANCHOR_END: cancel_other_workers
 }
 
+#[allow(clippy::undocumented_unsafe_blocks)] // TODO(#697) document safety
 unsafe impl<T: HasStorage> ZalsaDatabase for T {
     fn zalsa(&self) -> &Zalsa {
         &self.storage().handle.zalsa_impl

src/table.rs

@@ -81,8 +81,9 @@ impl SlotVTable {
     const fn of<T: Slot>() -> &'static Self {
         const {
             &Self {
+                drop_impl: |data, initialized|
                 // SAFETY: The caller is required to supply a correct data pointer and initialized length
-                drop_impl: |data, initialized| unsafe {
+                unsafe {
                     let data = Box::from_raw(data.cast::<PageData<T>>());
                     for i in 0..initialized {
                         ptr::drop_in_place(data[i].get().cast::<T>());

src/table/memo.rs

@@ -73,6 +73,7 @@ impl MemoTable {
     fn to_dyn_fn<M: Memo>() -> fn(NonNull<DummyMemo>) -> NonNull<dyn Memo> {
         let f: fn(NonNull<M>) -> NonNull<dyn Memo> = |x| x;
 
+        #[allow(clippy::undocumented_unsafe_blocks)] // TODO(#697) document safety
         unsafe {
             std::mem::transmute::<
                 fn(NonNull<M>) -> NonNull<dyn Memo>,
@@ -145,10 +146,9 @@ impl MemoTable {
                  type_id: _,
                  to_dyn_fn: _,
                  atomic_memo,
-             }| unsafe {
+                }|
                 // SAFETY: The `atomic_memo` field is never null.
-                Self::from_dummy(NonNull::new_unchecked(atomic_memo.into_inner()))
-            },
+                unsafe { Self::from_dummy(NonNull::new_unchecked(atomic_memo.into_inner())) },
         )
     }
 

src/tracked_struct.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::undocumented_unsafe_blocks)] // TODO(#697) document safety
+
 use std::{any::TypeId, fmt, hash::Hash, marker::PhantomData, ops::DerefMut};
 
 use crossbeam_queue::SegQueue;

src/update.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::undocumented_unsafe_blocks)] // TODO(#697) document safety
+
 use std::{
     collections::{BTreeMap, BTreeSet, HashMap, HashSet},
     hash::{BuildHasher, Hash},
@@ -52,6 +54,7 @@ pub mod helper {
         ///
         /// See the `maybe_update` method in the [`Update`][] trait.
         pub unsafe fn maybe_update(old_pointer: *mut D, new_value: D) -> bool {
+            // SAFETY: Same safety conditions as `Update::maybe_update`
             unsafe { D::maybe_update(old_pointer, new_value) }
         }
     }
@@ -66,8 +69,10 @@ pub mod helper {
         unsafe fn maybe_update(old_pointer: *mut T, new_value: T) -> bool;
     }
 
+    // SAFETY: Same safety conditions as `Update::maybe_update`
     unsafe impl<T: 'static + PartialEq> Fallback<T> for Dispatch<T> {
         unsafe fn maybe_update(old_pointer: *mut T, new_value: T) -> bool {
+            // SAFETY: Same safety conditions as `Update::maybe_update`
             unsafe { update_fallback(old_pointer, new_value) }
         }
     }
@@ -87,7 +92,7 @@ pub unsafe fn update_fallback<T>(old_pointer: *mut T, new_value: T) -> bool
 where
     T: 'static + PartialEq,
 {
-    // Because everything is owned, this ref is simply a valid `&mut`
+    // SAFETY: Because everything is owned, this ref is simply a valid `&mut`
     let old_ref: &mut T = unsafe { &mut *old_pointer };
 
     if *old_ref != new_value {

src/views.rs

@@ -62,6 +62,7 @@ impl<DbView: ?Sized + Any> DatabaseDownCaster<DbView> {
     ///
     /// The caller must ensure that `db` is of the correct type.
     pub unsafe fn downcast_unchecked<'db>(&self, db: &'db dyn Database) -> &'db DbView {
+        // SAFETY: The caller must ensure that `db` is of the correct type.
         unsafe { (self.1)(db) }
     }
 }

src/zalsa.rs

@@ -393,8 +393,6 @@ where
     phantom: PhantomData<fn() -> I>,
 }
 
-unsafe impl<I> Sync for IngredientCache<I> where I: Ingredient + Sync {}
-
 impl<I> Default for IngredientCache<I>
 where
     I: Ingredient,
@@ -469,6 +467,7 @@ where
 pub unsafe fn transmute_data_ptr<T: ?Sized, U>(t: &T) -> &U {
     let t: *const T = t;
     let u: *const U = t as *const U;
+    // SAFETY: the caller must guarantee that `T` is a wide pointer for `U`
     unsafe { &*u }
 }
 
@@ -480,5 +479,6 @@ pub unsafe fn transmute_data_ptr<T: ?Sized, U>(t: &T) -> &U {
 pub(crate) unsafe fn transmute_data_mut_ptr<T: ?Sized, U>(t: &mut T) -> &mut U {
     let t: *mut T = t;
     let u: *mut U = t as *mut U;
+    // SAFETY: the caller must guarantee that `T` is a wide pointer for `U`
     unsafe { &mut *u }
 }