From de911b02872b4cdb6ec5df6ce24c51e04aff469a Mon Sep 17 00:00:00 2001
From: David Barsky <me@davidbarsky.com>
Date: Mon, 25 Nov 2024 13:51:04 -0500
Subject: [PATCH 1/4] feature: support interned structs without lifetimes
---
components/salsa-macro-rules/src/lib.rs | 1 +
.../setup_interned_struct_sans_lifetime.rs | 201 ++++++++++++++++++
.../src/interned_sans_lifetime.rs | 131 ++++++++++++
components/salsa-macros/src/lib.rs | 6 +
src/lib.rs | 2 +
tests/interned-sans-lifetime.rs | 114 ++++++++++
6 files changed, 455 insertions(+)
create mode 100644 components/salsa-macro-rules/src/setup_interned_struct_sans_lifetime.rs
create mode 100644 components/salsa-macros/src/interned_sans_lifetime.rs
create mode 100644 tests/interned-sans-lifetime.rs
diff --git a/components/salsa-macro-rules/src/lib.rs b/components/salsa-macro-rules/src/lib.rs
index 4834b0f2d..7834b731d 100644
--- a/components/salsa-macro-rules/src/lib.rs
+++ b/components/salsa-macro-rules/src/lib.rs
@@ -19,6 +19,7 @@ mod maybe_default;
mod setup_accumulator_impl;
mod setup_input_struct;
mod setup_interned_struct;
+mod setup_interned_struct_sans_lifetime;
mod setup_method_body;
mod setup_tracked_fn;
mod setup_tracked_struct;
diff --git a/components/salsa-macro-rules/src/setup_interned_struct_sans_lifetime.rs b/components/salsa-macro-rules/src/setup_interned_struct_sans_lifetime.rs
new file mode 100644
index 000000000..aa9e4ef54
--- /dev/null
+++ b/components/salsa-macro-rules/src/setup_interned_struct_sans_lifetime.rs
@@ -0,0 +1,201 @@
+/// Macro for setting up a function that must intern its arguments, without any lifetimes.
+#[macro_export]
+macro_rules! setup_interned_struct_sans_lifetime {
+ (
+ // Attributes on the struct
+ attrs: [$(#[$attr:meta]),*],
+
+ // Visibility of the struct
+ vis: $vis:vis,
+
+ // Name of the struct
+ Struct: $Struct:ident,
+
+ // Name of the `'db` lifetime that the user gave
+ db_lt: $db_lt:lifetime,
+
+ // Name user gave for `new`
+ new_fn: $new_fn:ident,
+
+ // A series of option tuples; see `setup_tracked_struct` macro
+ field_options: [$($field_option:tt),*],
+
+ // Field names
+ field_ids: [$($field_id:ident),*],
+
+ // Names for field setter methods (typically `set_foo`)
+ field_getters: [$($field_getter_vis:vis $field_getter_id:ident),*],
+
+ // Field types
+ field_tys: [$($field_ty:ty),*],
+
+ // Indices for each field from 0..N -- must be unsuffixed (e.g., `0`, `1`).
+ field_indices: [$($field_index:tt),*],
+
+ // Indexed types for each field (T0, T1, ...)
+ field_indexed_tys: [$($indexed_ty:ident),*],
+
+ // Number of fields
+ num_fields: $N:literal,
+
+ // If true, generate a debug impl.
+ generate_debug_impl: $generate_debug_impl:tt,
+
+ // Annoyingly macro-rules hygiene does not extend to items defined in the macro.
+ // We have the procedural macro generate names for those items that are
+ // not used elsewhere in the user's code.
+ unused_names: [
+ $zalsa:ident,
+ $zalsa_struct:ident,
+ $Configuration:ident,
+ $CACHE:ident,
+ $Db:ident,
+ ]
+ ) => {
+ $(#[$attr])*
+ #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+ $vis struct $Struct(
+ salsa::Id,
+ std::marker::PhantomData < &'static salsa::plumbing::interned::Value < $Struct > >
+ );
+
+ 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>>
+ 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 {
+ ($($zalsa::interned::Lookup::eq(&self.$field_index, &data.$field_index) && )* true)
+ }
+
+ #[allow(unused_unit)]
+ fn into_owned(self) -> StructData<$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> {
+ $Struct(id, std::marker::PhantomData)
+ }
+ fn deref_struct(s: Self::Struct<'_>) -> salsa::Id {
+ s.0
+ }
+ }
+
+ impl $Configuration {
+ pub fn ingredient<Db>(db: &Db) -> &$zalsa_struct::IngredientImpl<Self>
+ where
+ Db: ?Sized + $zalsa::Database,
+ {
+ static CACHE: $zalsa::IngredientCache<$zalsa_struct::IngredientImpl<$Configuration>> =
+ $zalsa::IngredientCache::new();
+ CACHE.get_or_create(db.as_dyn_database(), || {
+ db.zalsa().add_or_lookup_jar_by_type(&<$zalsa_struct::JarImpl<$Configuration>>::default())
+ })
+ }
+ }
+
+ impl $zalsa::AsId for $Struct {
+ fn as_id(&self) -> salsa::Id {
+ self.0
+ }
+ }
+
+ impl $zalsa::FromId for $Struct {
+ fn from_id(id: salsa::Id) -> Self {
+ Self(id, std::marker::PhantomData)
+ }
+ }
+
+ unsafe impl Send for $Struct {}
+
+ unsafe impl Sync for $Struct {}
+
+ $zalsa::macro_if! { $generate_debug_impl =>
+ impl std::fmt::Debug for $Struct {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ Self::default_debug_fmt(*self, f)
+ }
+ }
+ }
+
+ impl $zalsa::SalsaStructInDb for $Struct {
+ }
+
+ unsafe impl $zalsa::Update for $Struct {
+ unsafe fn maybe_update(old_pointer: *mut Self, new_value: Self) -> bool {
+ if unsafe { *old_pointer } != new_value {
+ unsafe { *old_pointer = new_value };
+ true
+ } else {
+ false
+ }
+ }
+ }
+
+ impl<$db_lt> $Struct {
+ pub fn $new_fn<$Db>(db: &$db_lt $Db, $($field_id: impl $zalsa::interned::Lookup<$field_ty>),*) -> Self
+ where
+ // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
+ $Db: ?Sized + salsa::Database,
+ {
+ let current_revision = $zalsa::current_revision(db);
+ $Configuration::ingredient(db).intern(db.as_dyn_database(),
+ StructKey::<$db_lt>($($field_id,)* std::marker::PhantomData::default()))
+ }
+
+ $(
+ $field_getter_vis fn $field_getter_id<$Db>(self, db: &'db $Db) -> $zalsa::maybe_cloned_ty!($field_option, 'db, $field_ty)
+ where
+ // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
+ $Db: ?Sized + $zalsa::Database,
+ {
+ let fields = $Configuration::ingredient(db).fields(db.as_dyn_database(), self);
+ $zalsa::maybe_clone!(
+ $field_option,
+ $field_ty,
+ &fields.$field_index,
+ )
+ }
+ )*
+
+ /// Default debug formatting for this struct (may be useful if you define your own `Debug` impl)
+ pub fn default_debug_fmt(this: Self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ $zalsa::with_attached_database(|db| {
+ let fields = $Configuration::ingredient(db).fields(db.as_dyn_database(), this);
+ let mut f = f.debug_struct(stringify!($Struct));
+ $(
+ let f = f.field(stringify!($field_id), &fields.$field_index);
+ )*
+ f.finish()
+ }).unwrap_or_else(|| {
+ f.debug_tuple(stringify!($Struct))
+ .field(&$zalsa::AsId::as_id(&this))
+ .finish()
+ })
+ }
+ }
+ };
+ };
+}
diff --git a/components/salsa-macros/src/interned_sans_lifetime.rs b/components/salsa-macros/src/interned_sans_lifetime.rs
new file mode 100644
index 000000000..215e05fd3
--- /dev/null
+++ b/components/salsa-macros/src/interned_sans_lifetime.rs
@@ -0,0 +1,131 @@
+use crate::{
+ db_lifetime,
+ hygiene::Hygiene,
+ options::Options,
+ salsa_struct::{SalsaStruct, SalsaStructAllowedOptions},
+ token_stream_with_error,
+};
+use proc_macro2::TokenStream;
+
+/// For an entity struct `Foo` with fields `f1: T1, ..., fN: TN`, we generate...
+///
+/// * the "id struct" `struct Foo(salsa::Id)`
+/// * the entity ingredient, which maps the id fields to the `Id`
+/// * for each value field, a function ingredient
+pub(crate) fn interned_sans_lifetime(
+ args: proc_macro::TokenStream,
+ input: proc_macro::TokenStream,
+) -> proc_macro::TokenStream {
+ let args = syn::parse_macro_input!(args as InternedArgs);
+ let hygiene = Hygiene::from1(&input);
+ let struct_item = parse_macro_input!(input as syn::ItemStruct);
+ let m = Macro {
+ hygiene,
+ args,
+ struct_item,
+ };
+ match m.try_macro() {
+ Ok(v) => v.into(),
+ Err(e) => token_stream_with_error(input, e),
+ }
+}
+
+type InternedArgs = Options<InternedStruct>;
+
+struct InternedStruct;
+
+impl crate::options::AllowedOptions for InternedStruct {
+ const RETURN_REF: bool = false;
+
+ const SPECIFY: bool = false;
+
+ const NO_EQ: bool = false;
+
+ const NO_DEBUG: bool = true;
+
+ const NO_CLONE: bool = false;
+
+ const SINGLETON: bool = true;
+
+ const DATA: bool = true;
+
+ const DB: bool = false;
+
+ const RECOVERY_FN: bool = false;
+
+ const LRU: bool = false;
+
+ const CONSTRUCTOR_NAME: bool = true;
+}
+
+impl SalsaStructAllowedOptions for InternedStruct {
+ const KIND: &'static str = "interned";
+
+ const ALLOW_ID: bool = false;
+
+ const HAS_LIFETIME: bool = false;
+
+ const ALLOW_DEFAULT: bool = false;
+}
+
+struct Macro {
+ hygiene: Hygiene,
+ args: InternedArgs,
+ struct_item: syn::ItemStruct,
+}
+
+impl Macro {
+ #[allow(non_snake_case)]
+ fn try_macro(&self) -> syn::Result<TokenStream> {
+ let salsa_struct = SalsaStruct::new(&self.struct_item, &self.args)?;
+
+ let attrs = &self.struct_item.attrs;
+ let vis = &self.struct_item.vis;
+ let struct_ident = &self.struct_item.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();
+ let field_indices = salsa_struct.field_indices();
+ let num_fields = salsa_struct.num_fields();
+ let field_vis = salsa_struct.field_vis();
+ let field_getter_ids = salsa_struct.field_getter_ids();
+ let field_options = salsa_struct.field_options();
+ let field_tys = salsa_struct.field_tys();
+ let field_indexed_tys = salsa_struct.field_indexed_tys();
+ let generate_debug_impl = salsa_struct.generate_debug_impl();
+
+ let zalsa = self.hygiene.ident("zalsa");
+ let zalsa_struct = self.hygiene.ident("zalsa_struct");
+ let Configuration = self.hygiene.ident("Configuration");
+ let CACHE = self.hygiene.ident("CACHE");
+ let Db = self.hygiene.ident("Db");
+
+ Ok(crate::debug::dump_tokens(
+ struct_ident,
+ quote! {
+ salsa::plumbing::setup_interned_struct_sans_lifetime!(
+ attrs: [#(#attrs),*],
+ vis: #vis,
+ Struct: #struct_ident,
+ db_lt: #db_lt,
+ new_fn: #new_fn,
+ field_options: [#(#field_options),*],
+ field_ids: [#(#field_ids),*],
+ field_getters: [#(#field_vis #field_getter_ids),*],
+ field_tys: [#(#field_tys),*],
+ field_indices: [#(#field_indices),*],
+ field_indexed_tys: [#(#field_indexed_tys),*],
+ num_fields: #num_fields,
+ generate_debug_impl: #generate_debug_impl,
+ unused_names: [
+ #zalsa,
+ #zalsa_struct,
+ #Configuration,
+ #CACHE,
+ #Db,
+ ]
+ );
+ },
+ ))
+ }
+}
diff --git a/components/salsa-macros/src/lib.rs b/components/salsa-macros/src/lib.rs
index 2b2de5228..d643a956c 100644
--- a/components/salsa-macros/src/lib.rs
+++ b/components/salsa-macros/src/lib.rs
@@ -42,6 +42,7 @@ mod fn_util;
mod hygiene;
mod input;
mod interned;
+mod interned_sans_lifetime;
mod options;
mod salsa_struct;
mod tracked;
@@ -66,6 +67,11 @@ pub fn interned(args: TokenStream, input: TokenStream) -> TokenStream {
interned::interned(args, input)
}
+#[proc_macro_attribute]
+pub fn interned_sans_lifetime(args: TokenStream, input: TokenStream) -> TokenStream {
+ interned_sans_lifetime::interned_sans_lifetime(args, input)
+}
+
#[proc_macro_attribute]
pub fn input(args: TokenStream, input: TokenStream) -> TokenStream {
input::input(args, input)
diff --git a/src/lib.rs b/src/lib.rs
index 53dddcfd4..64e6cb301 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -51,6 +51,7 @@ pub use salsa_macros::accumulator;
pub use salsa_macros::db;
pub use salsa_macros::input;
pub use salsa_macros::interned;
+pub use salsa_macros::interned_sans_lifetime;
pub use salsa_macros::tracked;
pub use salsa_macros::Update;
@@ -111,6 +112,7 @@ pub mod plumbing {
pub use salsa_macro_rules::setup_accumulator_impl;
pub use salsa_macro_rules::setup_input_struct;
pub use salsa_macro_rules::setup_interned_struct;
+ pub use salsa_macro_rules::setup_interned_struct_sans_lifetime;
pub use salsa_macro_rules::setup_method_body;
pub use salsa_macro_rules::setup_tracked_fn;
pub use salsa_macro_rules::setup_tracked_struct;
diff --git a/tests/interned-sans-lifetime.rs b/tests/interned-sans-lifetime.rs
new file mode 100644
index 000000000..082247125
--- /dev/null
+++ b/tests/interned-sans-lifetime.rs
@@ -0,0 +1,114 @@
+use expect_test::expect;
+use std::path::{Path, PathBuf};
+use test_log::test;
+
+#[salsa::interned_sans_lifetime]
+struct InternedString {
+ data: String,
+}
+
+#[salsa::interned_sans_lifetime]
+struct InternedPair {
+ data: (InternedString, InternedString),
+}
+
+#[salsa::interned_sans_lifetime]
+struct InternedTwoFields {
+ data1: String,
+ data2: String,
+}
+
+#[salsa::interned_sans_lifetime]
+struct InternedVec {
+ data1: Vec<String>,
+}
+
+#[salsa::interned_sans_lifetime]
+struct InternedPathBuf {
+ data1: PathBuf,
+}
+
+#[salsa::tracked]
+fn intern_stuff(db: &dyn salsa::Database) -> String {
+ let s1 = InternedString::new(db, "Hello, ".to_string());
+ let s2 = InternedString::new(db, "World, ");
+ let s3 = InternedPair::new(db, (s1, s2));
+
+ format!("{s3:?}")
+}
+
+#[test]
+fn execute() {
+ let db = salsa::DatabaseImpl::new();
+ expect![[r#"
+ "InternedPair { data: (InternedString { data: \"Hello, \" }, InternedString { data: \"World, \" }) }"
+ "#]].assert_debug_eq(&intern_stuff(&db));
+}
+
+#[test]
+fn interning_returns_equal_keys_for_equal_data() {
+ let db = salsa::DatabaseImpl::new();
+ let s1 = InternedString::new(&db, "Hello, ".to_string());
+ let s2 = InternedString::new(&db, "World, ".to_string());
+ let s1_2 = InternedString::new(&db, "Hello, ");
+ let s2_2 = InternedString::new(&db, "World, ");
+ 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();
+ let s1 = InternedTwoFields::new(&db, "Hello, ".to_string(), "World");
+ let s2 = InternedTwoFields::new(&db, "World, ", "Hello".to_string());
+ let s1_2 = InternedTwoFields::new(&db, "Hello, ", "World");
+ let s2_2 = InternedTwoFields::new(&db, "World, ", "Hello");
+ let new = InternedTwoFields::new(&db, "Hello, World", "");
+
+ assert_eq!(s1, s1_2);
+ assert_eq!(s2, s2_2);
+ assert_ne!(s1, s2_2);
+ assert_ne!(s1, new);
+}
+
+#[test]
+fn interning_vec() {
+ let db = salsa::DatabaseImpl::new();
+ let s1 = InternedVec::new(&db, ["Hello, ".to_string(), "World".to_string()].as_slice());
+ let s2 = InternedVec::new(&db, ["Hello, ", "World"].as_slice());
+ let s3 = InternedVec::new(&db, vec!["Hello, ".to_string(), "World".to_string()]);
+ let s4 = InternedVec::new(&db, ["Hello, ", "World"].as_slice());
+ let s5 = InternedVec::new(&db, ["Hello, ", "World", "Test"].as_slice());
+ let s6 = InternedVec::new(&db, ["Hello, ", "World", ""].as_slice());
+ let s7 = InternedVec::new(&db, ["Hello, "].as_slice());
+ assert_eq!(s1, s2);
+ assert_eq!(s1, s3);
+ assert_eq!(s1, s4);
+ assert_ne!(s1, s5);
+ assert_ne!(s1, s6);
+ assert_ne!(s5, s6);
+ assert_ne!(s6, s7);
+}
+
+#[test]
+fn interning_path_buf() {
+ let db = salsa::DatabaseImpl::new();
+ let s1 = InternedPathBuf::new(&db, PathBuf::from("test_path".to_string()));
+ let s2 = InternedPathBuf::new(&db, Path::new("test_path"));
+ let s3 = InternedPathBuf::new(&db, Path::new("test_path/"));
+ let s4 = InternedPathBuf::new(&db, Path::new("test_path/a"));
+ assert_eq!(s1, s2);
+ assert_eq!(s1, s3);
+ assert_ne!(s1, s4);
+}
+
+#[salsa::tracked]
+fn length(db: &dyn salsa::Database, s: InternedString) -> usize {
+ s.data(db).len()
+}
+
+#[test]
+fn tracked_static_query_works() {
+ let db = salsa::DatabaseImpl::new();
+ let s1 = InternedString::new(&db, "Hello, World!".to_string());
+ assert_eq!(length(&db, s1), 13);
+}
From 661c8d9566022c00b139c073e99f5c5919fd1b7c Mon Sep 17 00:00:00 2001
From: David Barsky <me@davidbarsky.com>
Date: Mon, 2 Dec 2024 14:14:32 +0000
Subject: [PATCH 2/4] feature: add id paramater to interned structs
---
.../setup_interned_struct_sans_lifetime.rs | 15 ++++++++----
components/salsa-macros/src/accumulator.rs | 1 +
components/salsa-macros/src/input.rs | 2 ++
components/salsa-macros/src/interned.rs | 2 ++
.../src/interned_sans_lifetime.rs | 5 ++++
components/salsa-macros/src/options.rs | 22 +++++++++++++++++-
components/salsa-macros/src/salsa_struct.rs | 7 ++++++
components/salsa-macros/src/tracked_fn.rs | 2 ++
components/salsa-macros/src/tracked_struct.rs | 2 ++
tests/interned-sans-lifetime.rs | 23 +++++++++++++++----
10 files changed, 71 insertions(+), 10 deletions(-)
diff --git a/components/salsa-macro-rules/src/setup_interned_struct_sans_lifetime.rs b/components/salsa-macro-rules/src/setup_interned_struct_sans_lifetime.rs
index aa9e4ef54..bcb254d03 100644
--- a/components/salsa-macro-rules/src/setup_interned_struct_sans_lifetime.rs
+++ b/components/salsa-macro-rules/src/setup_interned_struct_sans_lifetime.rs
@@ -14,6 +14,9 @@ macro_rules! setup_interned_struct_sans_lifetime {
// Name of the `'db` lifetime that the user gave
db_lt: $db_lt:lifetime,
+ // the salsa ID
+ id: $Id:path,
+
// Name user gave for `new`
new_fn: $new_fn:ident,
@@ -55,7 +58,7 @@ macro_rules! setup_interned_struct_sans_lifetime {
$(#[$attr])*
#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
$vis struct $Struct(
- salsa::Id,
+ $Id,
std::marker::PhantomData < &'static salsa::plumbing::interned::Value < $Struct > >
);
@@ -96,10 +99,12 @@ macro_rules! setup_interned_struct_sans_lifetime {
type Data<'a> = StructData<'a>;
type Struct<'a> = $Struct;
fn struct_from_id<'db>(id: salsa::Id) -> Self::Struct<'db> {
- $Struct(id, std::marker::PhantomData)
+ use $zalsa::FromId;
+ $Struct(<$Id>::from_id(id), std::marker::PhantomData)
}
fn deref_struct(s: Self::Struct<'_>) -> salsa::Id {
- s.0
+ use $zalsa::AsId;
+ s.0.as_id()
}
}
@@ -118,13 +123,13 @@ macro_rules! setup_interned_struct_sans_lifetime {
impl $zalsa::AsId for $Struct {
fn as_id(&self) -> salsa::Id {
- self.0
+ self.0.as_id()
}
}
impl $zalsa::FromId for $Struct {
fn from_id(id: salsa::Id) -> Self {
- Self(id, std::marker::PhantomData)
+ Self(<$Id>::from_id(id), std::marker::PhantomData)
}
}
diff --git a/components/salsa-macros/src/accumulator.rs b/components/salsa-macros/src/accumulator.rs
index 1e09cb08f..f48a1c25e 100644
--- a/components/salsa-macros/src/accumulator.rs
+++ b/components/salsa-macros/src/accumulator.rs
@@ -42,6 +42,7 @@ impl AllowedOptions for Accumulator {
const RECOVERY_FN: bool = false;
const LRU: bool = false;
const CONSTRUCTOR_NAME: bool = false;
+ const ID: bool = true;
}
struct StructMacro {
diff --git a/components/salsa-macros/src/input.rs b/components/salsa-macros/src/input.rs
index 9ad444913..3c71b7f1c 100644
--- a/components/salsa-macros/src/input.rs
+++ b/components/salsa-macros/src/input.rs
@@ -55,6 +55,8 @@ impl crate::options::AllowedOptions for InputStruct {
const LRU: bool = false;
const CONSTRUCTOR_NAME: bool = true;
+
+ const ID: bool = true;
}
impl SalsaStructAllowedOptions for InputStruct {
diff --git a/components/salsa-macros/src/interned.rs b/components/salsa-macros/src/interned.rs
index 8caba77e4..92c278a0e 100644
--- a/components/salsa-macros/src/interned.rs
+++ b/components/salsa-macros/src/interned.rs
@@ -56,6 +56,8 @@ impl crate::options::AllowedOptions for InternedStruct {
const LRU: bool = false;
const CONSTRUCTOR_NAME: bool = true;
+
+ const ID: bool = true;
}
impl SalsaStructAllowedOptions for InternedStruct {
diff --git a/components/salsa-macros/src/interned_sans_lifetime.rs b/components/salsa-macros/src/interned_sans_lifetime.rs
index 215e05fd3..384c02c5f 100644
--- a/components/salsa-macros/src/interned_sans_lifetime.rs
+++ b/components/salsa-macros/src/interned_sans_lifetime.rs
@@ -32,6 +32,7 @@ pub(crate) fn interned_sans_lifetime(
type InternedArgs = Options<InternedStruct>;
+#[derive(Debug)]
struct InternedStruct;
impl crate::options::AllowedOptions for InternedStruct {
@@ -56,6 +57,8 @@ impl crate::options::AllowedOptions for InternedStruct {
const LRU: bool = false;
const CONSTRUCTOR_NAME: bool = true;
+
+ const ID: bool = true;
}
impl SalsaStructAllowedOptions for InternedStruct {
@@ -93,6 +96,7 @@ impl Macro {
let field_tys = salsa_struct.field_tys();
let field_indexed_tys = salsa_struct.field_indexed_tys();
let generate_debug_impl = salsa_struct.generate_debug_impl();
+ let id = salsa_struct.id();
let zalsa = self.hygiene.ident("zalsa");
let zalsa_struct = self.hygiene.ident("zalsa_struct");
@@ -108,6 +112,7 @@ impl Macro {
vis: #vis,
Struct: #struct_ident,
db_lt: #db_lt,
+ id: #id,
new_fn: #new_fn,
field_options: [#(#field_options),*],
field_ids: [#(#field_ids),*],
diff --git a/components/salsa-macros/src/options.rs b/components/salsa-macros/src/options.rs
index 6f30bb3e6..f16466300 100644
--- a/components/salsa-macros/src/options.rs
+++ b/components/salsa-macros/src/options.rs
@@ -7,6 +7,7 @@ use syn::{ext::IdentExt, spanned::Spanned};
/// are required and trailing commas are permitted. The options accepted
/// for any particular location are configured via the `AllowedOptions`
/// trait.
+#[derive(Debug)]
pub(crate) struct Options<A: AllowedOptions> {
/// The `return_ref` option is used to signal that field/return type is "by ref"
///
@@ -66,6 +67,12 @@ pub(crate) struct Options<A: AllowedOptions> {
/// If this is `Some`, the value is the `<ident>`.
pub constructor_name: Option<syn::Ident>,
+ /// The `id = <path>` option is used to set a custom ID for interrned structs.
+ ///
+ /// The custom ID needs to handle
+ /// If this is `Some`, the value is the `<ident>`.
+ pub id: Option<syn::Path>,
+
/// Remember the `A` parameter, which plays no role after parsing.
phantom: PhantomData<A>,
}
@@ -85,6 +92,7 @@ impl<A: AllowedOptions> Default for Options<A> {
phantom: Default::default(),
lru: Default::default(),
singleton: Default::default(),
+ id: Default::default(),
}
}
}
@@ -102,6 +110,7 @@ pub(crate) trait AllowedOptions {
const RECOVERY_FN: bool;
const LRU: bool;
const CONSTRUCTOR_NAME: bool;
+ const ID: bool;
}
type Equals = syn::Token![=];
@@ -252,7 +261,7 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
}
} else if ident == "constructor" {
if A::CONSTRUCTOR_NAME {
- let _eq = Equals::parse(input)?;
+ let _eq: syn::token::Eq = Equals::parse(input)?;
let ident = syn::Ident::parse(input)?;
if let Some(old) = std::mem::replace(&mut options.constructor_name, Some(ident))
{
@@ -267,6 +276,17 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
"`constructor` option not allowed here",
));
}
+ } else if ident == "id" {
+ if A::ID {
+ let _eq = Equals::parse(input)?;
+ let path = syn::Path::parse(input)?;
+ options.id = Some(path);
+ } else {
+ return Err(syn::Error::new(
+ ident.span(),
+ "`id` option not allowed here",
+ ));
+ }
} else {
return Err(syn::Error::new(
ident.span(),
diff --git a/components/salsa-macros/src/salsa_struct.rs b/components/salsa-macros/src/salsa_struct.rs
index 1a24fdc42..6e0759e13 100644
--- a/components/salsa-macros/src/salsa_struct.rs
+++ b/components/salsa-macros/src/salsa_struct.rs
@@ -118,6 +118,13 @@ where
}
}
+ pub(crate) fn id(&self) -> syn::Path {
+ match &self.args.id {
+ Some(id) => id.clone(),
+ None => parse_quote!(salsa::Id),
+ }
+ }
+
/// Disallow `#[id]` attributes on the fields of this struct.
///
/// If an `#[id]` field is found, return an error.
diff --git a/components/salsa-macros/src/tracked_fn.rs b/components/salsa-macros/src/tracked_fn.rs
index 57023ef24..3e9d2f35b 100644
--- a/components/salsa-macros/src/tracked_fn.rs
+++ b/components/salsa-macros/src/tracked_fn.rs
@@ -44,6 +44,8 @@ impl crate::options::AllowedOptions for TrackedFn {
const LRU: bool = true;
const CONSTRUCTOR_NAME: bool = false;
+
+ const ID: bool = true;
}
struct Macro {
diff --git a/components/salsa-macros/src/tracked_struct.rs b/components/salsa-macros/src/tracked_struct.rs
index 1730b3404..4904d3655 100644
--- a/components/salsa-macros/src/tracked_struct.rs
+++ b/components/salsa-macros/src/tracked_struct.rs
@@ -50,6 +50,8 @@ impl crate::options::AllowedOptions for TrackedStruct {
const LRU: bool = false;
const CONSTRUCTOR_NAME: bool = true;
+
+ const ID: bool = true;
}
impl SalsaStructAllowedOptions for TrackedStruct {
diff --git a/tests/interned-sans-lifetime.rs b/tests/interned-sans-lifetime.rs
index 082247125..6ab50026f 100644
--- a/tests/interned-sans-lifetime.rs
+++ b/tests/interned-sans-lifetime.rs
@@ -2,23 +2,38 @@ use expect_test::expect;
use std::path::{Path, PathBuf};
use test_log::test;
-#[salsa::interned_sans_lifetime]
+#[derive(Clone, Copy, Hash, Debug, PartialEq, Eq, PartialOrd, Ord)]
+struct CustomSalsaIdWrapper(salsa::Id);
+
+impl salsa::plumbing::AsId for CustomSalsaIdWrapper {
+ fn as_id(&self) -> salsa::Id {
+ self.0
+ }
+}
+
+impl salsa::plumbing::FromId for CustomSalsaIdWrapper {
+ fn from_id(id: salsa::Id) -> Self {
+ CustomSalsaIdWrapper(id)
+ }
+}
+
+#[salsa::interned_sans_lifetime(id = CustomSalsaIdWrapper)]
struct InternedString {
data: String,
}
-#[salsa::interned_sans_lifetime]
+#[salsa::interned_sans_lifetime(id = CustomSalsaIdWrapper)]
struct InternedPair {
data: (InternedString, InternedString),
}
-#[salsa::interned_sans_lifetime]
+#[salsa::interned_sans_lifetime(id = CustomSalsaIdWrapper)]
struct InternedTwoFields {
data1: String,
data2: String,
}
-#[salsa::interned_sans_lifetime]
+#[salsa::interned_sans_lifetime(id = CustomSalsaIdWrapper)]
struct InternedVec {
data1: Vec<String>,
}
From b0256907c2d5df419cb2389991e730bdfc6ba748 Mon Sep 17 00:00:00 2001
From: David Barsky <me@davidbarsky.com>
Date: Tue, 3 Dec 2024 16:16:03 +0000
Subject: [PATCH 3/4] internal: make `salsa::Id::new` pub, but `#[doc(hidden)]`
---
src/id.rs | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/id.rs b/src/id.rs
index 859039ac3..06b54b88b 100644
--- a/src/id.rs
+++ b/src/id.rs
@@ -29,8 +29,9 @@ impl Id {
/// In general, you should not need to create salsa ids yourself,
/// but it can be useful if you are using the type as a general
/// purpose "identifier" internally.
+ #[doc(hidden)]
#[track_caller]
- pub(crate) const fn from_u32(x: u32) -> Self {
+ pub const fn from_u32(x: u32) -> Self {
Id {
value: match NonZeroU32::new(x + 1) {
Some(v) => v,
From 2cc51938169ca4e9d56631041e8532ba52023326 Mon Sep 17 00:00:00 2001
From: David Barsky <me@davidbarsky.com>
Date: Thu, 5 Dec 2024 12:36:33 +0000
Subject: [PATCH 4/4] feature: allow calling ingredient on some interned
structs
---
.../setup_interned_struct_sans_lifetime.rs | 74 ++++++++++++-------
.../src/interned_sans_lifetime.rs | 2 +
components/salsa-macros/src/lib.rs | 44 +++++++++++
src/table/memo.rs | 2 +-
src/table/sync.rs | 2 +-
tests/interned-sans-lifetime.rs | 37 +++++++++-
6 files changed, 129 insertions(+), 32 deletions(-)
diff --git a/components/salsa-macro-rules/src/setup_interned_struct_sans_lifetime.rs b/components/salsa-macro-rules/src/setup_interned_struct_sans_lifetime.rs
index bcb254d03..b9fe0288a 100644
--- a/components/salsa-macro-rules/src/setup_interned_struct_sans_lifetime.rs
+++ b/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,59 @@ 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>),*>(
+ #[derive(Hash)]
+ struct StructKey<$db_lt, $($indexed_ty),*>(
$($indexed_ty,)*
std::marker::PhantomData<&$db_lt ()>,
);
- impl<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*> $zalsa::interned::Lookup<StructData<$db_lt>>
- for StructKey<$db_lt, $($indexed_ty),*> {
-
+ impl<$db_lt, $($indexed_ty,)*> $zalsa::interned::HashEqLike<StructKey<$db_lt, $($indexed_ty),*>>
+ for $StructDataIdent<$db_lt>
+ where
+ $($field_ty: $zalsa::interned::HashEqLike<$indexed_ty>),*
+ {
fn hash<H: std::hash::Hasher>(&self, h: &mut H) {
- $($zalsa::interned::Lookup::hash(&self.$field_index, &mut *h);)*
+ $($zalsa::interned::HashEqLike::<$indexed_ty>::hash(&self.$field_index, &mut *h);)*
}
- fn eq(&self, data: &StructData<$db_lt>) -> bool {
- ($($zalsa::interned::Lookup::eq(&self.$field_index, &data.$field_index) && )* true)
+ fn eq(&self, data: &StructKey<$db_lt, $($indexed_ty),*>) -> bool {
+ ($($zalsa::interned::HashEqLike::<$indexed_ty>::eq(&self.$field_index, &data.$field_index) && )* true)
}
+ }
+
+ impl<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*> $zalsa::interned::Lookup<$StructDataIdent<$db_lt>>
+ for StructKey<$db_lt, $($indexed_ty),*> {
#[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
@@ -146,6 +157,9 @@ macro_rules! setup_interned_struct_sans_lifetime {
}
impl $zalsa::SalsaStructInDb for $Struct {
+ fn lookup_ingredient_index(aux: &dyn $zalsa::JarAux) -> core::option::Option<$zalsa::IngredientIndex> {
+ aux.lookup_jar_by_type(&<$zalsa_struct::JarImpl<$Configuration>>::default())
+ }
}
unsafe impl $zalsa::Update for $Struct {
@@ -160,14 +174,20 @@ macro_rules! setup_interned_struct_sans_lifetime {
}
impl<$db_lt> $Struct {
- pub fn $new_fn<$Db>(db: &$db_lt $Db, $($field_id: impl $zalsa::interned::Lookup<$field_ty>),*) -> Self
+ pub fn $new_fn<$Db, $($indexed_ty: $zalsa::interned::Lookup<$field_ty> + std::hash::Hash,)*>(db: &$db_lt $Db, $($field_id: $indexed_ty),*) -> Self
where
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + salsa::Database,
+ $(
+ $field_ty: $zalsa::interned::HashEqLike<$indexed_ty>,
+ )*
{
let current_revision = $zalsa::current_revision(db);
- $Configuration::ingredient(db).intern(db.as_dyn_database(),
- StructKey::<$db_lt>($($field_id,)* std::marker::PhantomData::default()))
+ $Configuration::ingredient(db).intern(
+ db.as_dyn_database(),
+ StructKey::<$db_lt>($($field_id,)* std::marker::PhantomData::default()),
+ |_, data| ($($zalsa::interned::Lookup::into_owned(data.$field_index),)*)
+ )
}
$(
diff --git a/components/salsa-macros/src/interned_sans_lifetime.rs b/components/salsa-macros/src/interned_sans_lifetime.rs
index 384c02c5f..b2342695d 100644
--- a/components/salsa-macros/src/interned_sans_lifetime.rs
+++ b/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,
diff --git a/components/salsa-macros/src/lib.rs b/components/salsa-macros/src/lib.rs
index d643a956c..688d87355 100644
--- a/components/salsa-macros/src/lib.rs
+++ b/components/salsa-macros/src/lib.rs
@@ -67,6 +67,50 @@ 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:
+///
+/// ```no_compile
+/// use salsa::plumbing::{AsId, FromId};
+///
+/// #[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)
diff --git a/src/table/memo.rs b/src/table/memo.rs
index fe2fc9583..e26293b26 100644
--- a/src/table/memo.rs
+++ b/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>>,
}
diff --git a/src/table/sync.rs b/src/table/sync.rs
index dfe78a23a..58159cfe5 100644
--- a/src/table/sync.rs
+++ b/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>>>,
}
diff --git a/tests/interned-sans-lifetime.rs b/tests/interned-sans-lifetime.rs
index 6ab50026f..9f2cbd6f8 100644
--- a/tests/interned-sans-lifetime.rs
+++ b/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());
+}