feature: allow calling ingredient on some interned structs

Author: davidbarsky

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

@@ -11,6 +11,10 @@ macro_rules! setup_interned_struct_sans_lifetime {
         // Name of the struct
         Struct: $Struct:ident,
 
+        // Name of the struct data. This is a parameter because `std::concat_idents`
+        // is unstable and taking an addition dependency is unnecessary.
+        StructData: $StructDataIdent:ident,
+
         // Name of the `'db` lifetime that the user gave
         db_lt: $db_lt:lifetime,
 
@@ -62,52 +66,52 @@ macro_rules! setup_interned_struct_sans_lifetime {
             std::marker::PhantomData < &'static  salsa::plumbing::interned::Value < $Struct > >
         );
 
+        type $StructDataIdent<$db_lt> = ($($field_ty,)*);
+
+        impl salsa::plumbing::interned::Configuration for $Struct {
+            const DEBUG_NAME: &'static str = stringify!($Struct);
+            type Data<'a> = $StructDataIdent<'a>;
+            type Struct<'a> = $Struct;
+            fn struct_from_id<'db>(id: salsa::Id) -> Self::Struct<'db> {
+                use salsa::plumbing::FromId;
+                $Struct(<$Id>::from_id(id), std::marker::PhantomData)
+            }
+            fn deref_struct(s: Self::Struct<'_>) -> salsa::Id {
+                use salsa::plumbing::AsId;
+                s.0.as_id()
+            }
+        }
+
         const _: () = {
             use salsa::plumbing as $zalsa;
             use $zalsa::interned as $zalsa_struct;
 
             type $Configuration = $Struct;
 
-            type StructData<$db_lt> = ($($field_ty,)*);
-
             /// Key to use during hash lookups. Each field is some type that implements `Lookup<T>`
             /// for the owned type. This permits interning with an `&str` when a `String` is required and so forth.
             struct StructKey<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*>(
                 $($indexed_ty,)*
                 std::marker::PhantomData<&$db_lt ()>,
             );
 
-            impl<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*> $zalsa::interned::Lookup<StructData<$db_lt>>
+            impl<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*> $zalsa::interned::Lookup<$StructDataIdent<$db_lt>>
                 for StructKey<$db_lt, $($indexed_ty),*> {
 
                 fn hash<H: std::hash::Hasher>(&self, h: &mut H) {
                     $($zalsa::interned::Lookup::hash(&self.$field_index, &mut *h);)*
                 }
 
-                fn eq(&self, data: &StructData<$db_lt>) -> bool {
+                fn eq(&self, data: &$StructDataIdent<$db_lt>) -> bool {
                     ($($zalsa::interned::Lookup::eq(&self.$field_index, &data.$field_index) && )* true)
                 }
 
                 #[allow(unused_unit)]
-                fn into_owned(self) -> StructData<$db_lt> {
+                fn into_owned(self) -> $StructDataIdent<$db_lt> {
                     ($($zalsa::interned::Lookup::into_owned(self.$field_index),)*)
                 }
             }
 
-            impl $zalsa_struct::Configuration for $Configuration {
-                const DEBUG_NAME: &'static str = stringify!($Struct);
-                type Data<'a> = StructData<'a>;
-                type Struct<'a> = $Struct;
-                fn struct_from_id<'db>(id: salsa::Id) -> Self::Struct<'db> {
-                    use $zalsa::FromId;
-                    $Struct(<$Id>::from_id(id), std::marker::PhantomData)
-                }
-                fn deref_struct(s: Self::Struct<'_>) -> salsa::Id {
-                    use $zalsa::AsId;
-                    s.0.as_id()
-                }
-            }
-
             impl $Configuration {
                 pub fn ingredient<Db>(db: &Db) -> &$zalsa_struct::IngredientImpl<Self>
                 where

components/salsa-macros/src/interned_sans_lifetime.rs

@@ -85,6 +85,7 @@ impl Macro {
         let attrs = &self.struct_item.attrs;
         let vis = &self.struct_item.vis;
         let struct_ident = &self.struct_item.ident;
+        let struct_data_ident = format_ident!("{}Data", struct_ident);
         let db_lt = db_lifetime::db_lifetime(&self.struct_item.generics);
         let new_fn = salsa_struct.constructor_name();
         let field_ids = salsa_struct.field_ids();
@@ -111,6 +112,7 @@ impl Macro {
                     attrs: [#(#attrs),*],
                     vis: #vis,
                     Struct: #struct_ident,
+                    StructData: #struct_data_ident,
                     db_lt: #db_lt,
                     id: #id,
                     new_fn: #new_fn,

components/salsa-macros/src/lib.rs

@@ -67,6 +67,48 @@ pub fn interned(args: TokenStream, input: TokenStream) -> TokenStream {
     interned::interned(args, input)
 }
 
+/// A discouraged variant of `#[salsa::interned]`.
+///
+/// `#[salsa::interned_sans_lifetime]` is intended to be used in codebases that are migrating from
+/// the original Salsa to the current version of Salsa. New codebases that are just starting to use
+/// Salsa should avoid using this macro and prefer `#[salsa::interned]` instead.
+///
+/// `#[salsa::interned_sans_lifetime]` differs from `#[salsa::interned]` in a two key ways:
+/// 1. As the name suggests, it removes the `'db` lifetime from the interned struct. This lifetime is
+///    designed to meant to certain values as "salsa structs", but it also adds the desirable property
+///    of misuse resistance: it is difficult to embed an `#[salsa::interned]` struct into an auxiliary
+///    structures or collections collection, which can lead to subtle invalidation bugs. However, old
+///    Salsa encouraged storing keys to interned values in auxiliary structures and collections, so
+///    so converting all usage to Salsa's current API guidelines might not be desirable or feasible.
+/// 2. `#[salsa::interned_sans_lifetime]` requires specifiying the ID. In most cases, `salsa::Id`
+///    is sufficent, but in rare, performance-sensitive circumstances, it might be desireable to
+///    set the Id to a type that implements `salsa::plumbing::AsId` and `salsa::plumbing::FromId`.
+///
+/// ## Example
+///
+/// Below is an example of a struct using `#[salsa::interned_sans_lifetime]` with a custom Id:
+///
+/// ```rust
+/// #[derive(Clone, Copy, Hash, Debug, PartialEq, Eq, PartialOrd, Ord)]
+/// struct CustomSalsaIdWrapper(salsa::Id);
+///
+/// impl AsId for CustomSalsaIdWrapper {
+///     fn as_id(&self) -> salsa::Id {
+///         self.0
+///     }
+/// }
+///
+/// impl FromId for CustomSalsaIdWrapper {
+///     fn from_id(id: salsa::Id) -> Self {
+///         CustomSalsaIdWrapper(id)
+///     }
+/// }
+///
+/// #[salsa::interned_sans_lifetime(id = CustomSalsaIdWrapper)]
+/// struct InternedString {
+///     data: String,
+/// }
+/// ```
 #[proc_macro_attribute]
 pub fn interned_sans_lifetime(args: TokenStream, input: TokenStream) -> TokenStream {
     interned_sans_lifetime::interned_sans_lifetime(args, input)

src/table/memo.rs

@@ -13,7 +13,7 @@ use crate::{zalsa::MemoIngredientIndex, zalsa_local::QueryOrigin};
 /// Every tracked function must take a salsa struct as its first argument
 /// and memo tables are attached to those salsa structs as auxiliary data.
 #[derive(Default)]
-pub(crate) struct MemoTable {
+pub struct MemoTable {
     memos: RwLock<Vec<MemoEntry>>,
 }
 

src/table/sync.rs

@@ -18,7 +18,7 @@ use super::util;
 /// Tracks the keys that are currently being processed; used to coordinate between
 /// worker threads.
 #[derive(Default)]
-pub(crate) struct SyncTable {
+pub struct SyncTable {
     syncs: RwLock<Vec<Option<SyncState>>>,
 }
 

tests/interned-sans-lifetime.rs

@@ -1,17 +1,18 @@
 use expect_test::expect;
+use salsa::plumbing::{AsId, FromId};
 use std::path::{Path, PathBuf};
 use test_log::test;
 
 #[derive(Clone, Copy, Hash, Debug, PartialEq, Eq, PartialOrd, Ord)]
 struct CustomSalsaIdWrapper(salsa::Id);
 
-impl salsa::plumbing::AsId for CustomSalsaIdWrapper {
+impl AsId for CustomSalsaIdWrapper {
     fn as_id(&self) -> salsa::Id {
         self.0
     }
 }
 
-impl salsa::plumbing::FromId for CustomSalsaIdWrapper {
+impl FromId for CustomSalsaIdWrapper {
     fn from_id(id: salsa::Id) -> Self {
         CustomSalsaIdWrapper(id)
     }
@@ -45,7 +46,7 @@ struct InternedPathBuf {
 
 #[salsa::tracked]
 fn intern_stuff(db: &dyn salsa::Database) -> String {
-    let s1 = InternedString::new(db, "Hello, ".to_string());
+    let s1 = InternedString::new(db, "Hello, ");
     let s2 = InternedString::new(db, "World, ");
     let s3 = InternedPair::new(db, (s1, s2));
 
@@ -70,6 +71,7 @@ fn interning_returns_equal_keys_for_equal_data() {
     assert_eq!(s1, s1_2);
     assert_eq!(s2, s2_2);
 }
+
 #[test]
 fn interning_returns_equal_keys_for_equal_data_multi_field() {
     let db = salsa::DatabaseImpl::new();
@@ -127,3 +129,32 @@ fn tracked_static_query_works() {
     let s1 = InternedString::new(&db, "Hello, World!".to_string());
     assert_eq!(length(&db, s1), 13);
 }
+
+#[test]
+fn public_ingredient() {
+    let db = salsa::DatabaseImpl::new();
+    let s = InternedString::new(&db, String::from("Hello, world!"));
+    let underlying_id = s.0;
+
+    let data = InternedString::ingredient(&db).data(&db, underlying_id.as_id());
+    assert_eq!(data, &(String::from("Hello, world!"),));
+}
+
+#[salsa::tracked]
+fn intern_more_stuff(db: &dyn salsa::Database) -> (InternedString, InternedString, InternedPair) {
+    let s1 = InternedString::new(db, "Hello, ");
+    let s2 = InternedString::new(db, "World, ");
+    let pair = InternedPair::new(db, (s1, s2));
+    (s1, s2, pair)
+}
+
+#[test]
+fn public_ingredients() {
+    let db = salsa::DatabaseImpl::new();
+
+    let (_, _, pair) = intern_more_stuff(&db);
+    let (interned_s1, interned_s2) = InternedPair::ingredient(&db).fields(&db, pair).0;
+
+    assert_eq!(interned_s1.data(&db), "Hello, ".to_owned());
+    assert_eq!(interned_s2.data(&db), "World, ".to_owned());
+}