diff --git a/components/salsa-macro-rules/src/setup_interned_struct.rs b/components/salsa-macro-rules/src/setup_interned_struct.rs index f3eeb83c3..d4bcf19b3 100644 --- a/components/salsa-macro-rules/src/setup_interned_struct.rs +++ b/components/salsa-macro-rules/src/setup_interned_struct.rs @@ -11,9 +11,28 @@ macro_rules! setup_interned_struct { // 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 additional dependency is unnecessary. + StructData: $StructDataIdent:ident, + + // Name of the struct type with a `'static` argument (unless this type has no db lifetime, + // in which case this is the same as `$Struct`) + StructWithStatic: $StructWithStatic:ty, + // Name of the `'db` lifetime that the user gave db_lt: $db_lt:lifetime, + // optional db lifetime argument. + db_lt_arg: $($db_lt_arg:lifetime)?, + + // the salsa ID + id: $Id:path, + + // the lifetime used in the desugared interned struct. + // if the `db_lt_arg`, is present, this is `db_lt_arg`, but otherwise, + // it is `'static`. + interior_lt: $interior_lt:lifetime, + // Name user gave for `new` new_fn: $new_fn:ident, @@ -54,18 +73,18 @@ macro_rules! setup_interned_struct { ) => { $(#[$attr])* #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] - $vis struct $Struct<$db_lt>( - salsa::Id, - std::marker::PhantomData < & $db_lt salsa::plumbing::interned::Value < $Struct<'static> > > + $vis struct $Struct< $($db_lt_arg)? >( + $Id, + std::marker::PhantomData < & $interior_lt salsa::plumbing::interned::Value <$StructWithStatic> > ); const _: () = { use salsa::plumbing as $zalsa; use $zalsa::interned as $zalsa_struct; - type $Configuration = $Struct<'static>; + type $Configuration = $StructWithStatic; - type StructData<$db_lt> = ($($field_ty,)*); + type $StructDataIdent<$db_lt> = ($($field_ty,)*); /// Key to use during hash lookups. Each field is some type that implements `Lookup` /// for the owned type. This permits interning with an `&str` when a `String` is required and so forth. @@ -76,7 +95,7 @@ macro_rules! setup_interned_struct { ); impl<$db_lt, $($indexed_ty,)*> $zalsa::interned::HashEqLike> - for StructData<$db_lt> + for $StructDataIdent<$db_lt> where $($field_ty: $zalsa::interned::HashEqLike<$indexed_ty>),* { @@ -90,24 +109,27 @@ macro_rules! setup_interned_struct { } } - impl<$db_lt, $($indexed_ty: $zalsa::interned::Lookup<$field_ty>),*> $zalsa::interned::Lookup> + 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 { + impl salsa::plumbing::interned::Configuration for $StructWithStatic { const DEBUG_NAME: &'static str = stringify!($Struct); - type Data<'a> = StructData<'a>; - type Struct<'a> = $Struct<'a>; + type Data<'a> = $StructDataIdent<'a>; + type Struct<'db> = $Struct< $($db_lt_arg)? >; fn struct_from_id<'db>(id: salsa::Id) -> Self::Struct<'db> { - $Struct(id, std::marker::PhantomData) + use salsa::plumbing::FromId; + $Struct(<$Id>::from_id(id), std::marker::PhantomData) } + fn deref_struct(s: Self::Struct<'_>) -> salsa::Id { - s.0 + use salsa::plumbing::AsId; + s.0.as_id() } } @@ -124,37 +146,37 @@ macro_rules! setup_interned_struct { } } - impl $zalsa::AsId for $Struct<'_> { + impl< $($db_lt_arg)? > $zalsa::AsId for $Struct< $($db_lt_arg)? > { fn as_id(&self) -> salsa::Id { - self.0 + self.0.as_id() } } - impl $zalsa::FromId for $Struct<'_> { + impl< $($db_lt_arg)? > $zalsa::FromId for $Struct< $($db_lt_arg)? > { fn from_id(id: salsa::Id) -> Self { - Self(id, std::marker::PhantomData) + Self(<$Id>::from_id(id), std::marker::PhantomData) } } - unsafe impl Send for $Struct<'_> {} + unsafe impl< $($db_lt_arg)? > Send for $Struct< $($db_lt_arg)? > {} - unsafe impl Sync for $Struct<'_> {} + unsafe impl< $($db_lt_arg)? > Sync for $Struct< $($db_lt_arg)? > {} $zalsa::macro_if! { $generate_debug_impl => - impl std::fmt::Debug for $Struct<'_> { + impl< $($db_lt_arg)? > std::fmt::Debug for $Struct< $($db_lt_arg)? > { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { Self::default_debug_fmt(*self, f) } } } - impl $zalsa::SalsaStructInDb for $Struct<'_> { + impl< $($db_lt_arg)? > $zalsa::SalsaStructInDb for $Struct< $($db_lt_arg)? > { 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<'_> { + unsafe impl< $($db_lt_arg)? > $zalsa::Update for $Struct< $($db_lt_arg)? > { unsafe fn maybe_update(old_pointer: *mut Self, new_value: Self) -> bool { if unsafe { *old_pointer } != new_value { unsafe { *old_pointer = new_value }; @@ -165,7 +187,7 @@ macro_rules! setup_interned_struct { } } - impl<$db_lt> $Struct<$db_lt> { + impl<$db_lt> $Struct< $($db_lt_arg)? > { 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` diff --git a/components/salsa-macros/src/accumulator.rs b/components/salsa-macros/src/accumulator.rs index 1e09cb08f..e84bae121 100644 --- a/components/salsa-macros/src/accumulator.rs +++ b/components/salsa-macros/src/accumulator.rs @@ -36,12 +36,14 @@ impl AllowedOptions for Accumulator { const NO_EQ: bool = false; const NO_DEBUG: bool = true; const NO_CLONE: bool = true; + const NO_LIFETIME: bool = false; const SINGLETON: bool = false; const DATA: bool = false; const DB: bool = false; const RECOVERY_FN: bool = false; const LRU: bool = false; const CONSTRUCTOR_NAME: bool = false; + const ID: bool = false; } struct StructMacro { diff --git a/components/salsa-macros/src/input.rs b/components/salsa-macros/src/input.rs index 9ad444913..356d6b944 100644 --- a/components/salsa-macros/src/input.rs +++ b/components/salsa-macros/src/input.rs @@ -42,6 +42,8 @@ impl crate::options::AllowedOptions for InputStruct { const NO_DEBUG: bool = true; + const NO_LIFETIME: bool = false; + const NO_CLONE: bool = false; const SINGLETON: bool = true; @@ -55,6 +57,8 @@ impl crate::options::AllowedOptions for InputStruct { const LRU: bool = false; const CONSTRUCTOR_NAME: bool = true; + + const ID: bool = false; } impl SalsaStructAllowedOptions for InputStruct { @@ -64,6 +68,8 @@ impl SalsaStructAllowedOptions for InputStruct { const HAS_LIFETIME: bool = false; + const ELIDABLE_LIFETIME: bool = false; + const ALLOW_DEFAULT: bool = true; } diff --git a/components/salsa-macros/src/interned.rs b/components/salsa-macros/src/interned.rs index 8caba77e4..0d6f37a3b 100644 --- a/components/salsa-macros/src/interned.rs +++ b/components/salsa-macros/src/interned.rs @@ -43,6 +43,8 @@ impl crate::options::AllowedOptions for InternedStruct { const NO_DEBUG: bool = true; + const NO_LIFETIME: bool = true; + const NO_CLONE: bool = false; const SINGLETON: bool = true; @@ -56,6 +58,8 @@ impl crate::options::AllowedOptions for InternedStruct { const LRU: bool = false; const CONSTRUCTOR_NAME: bool = true; + + const ID: bool = true; } impl SalsaStructAllowedOptions for InternedStruct { @@ -65,6 +69,8 @@ impl SalsaStructAllowedOptions for InternedStruct { const HAS_LIFETIME: bool = true; + const ELIDABLE_LIFETIME: bool = true; + const ALLOW_DEFAULT: bool = false; } @@ -82,6 +88,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(); @@ -93,6 +100,24 @@ 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 has_lifetime = salsa_struct.generate_lifetime(); + let id = salsa_struct.id(); + + let (db_lt_arg, cfg, interior_lt) = if has_lifetime { + ( + Some(db_lt.clone()), + quote!(#struct_ident<'static>), + db_lt.clone(), + ) + } else { + let span = syn::spanned::Spanned::span(&self.struct_item.generics); + let static_lifetime = syn::Lifetime { + apostrophe: span, + ident: syn::Ident::new("static", span), + }; + + (None, quote!(#struct_ident), static_lifetime) + }; let zalsa = self.hygiene.ident("zalsa"); let zalsa_struct = self.hygiene.ident("zalsa_struct"); @@ -107,7 +132,12 @@ impl Macro { attrs: [#(#attrs),*], vis: #vis, Struct: #struct_ident, + StructData: #struct_data_ident, + StructWithStatic: #cfg, db_lt: #db_lt, + db_lt_arg: #db_lt_arg, + id: #id, + interior_lt: #interior_lt, 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..a114f0c86 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 { /// The `return_ref` option is used to signal that field/return type is "by ref" /// @@ -24,6 +25,11 @@ pub(crate) struct Options { /// If this is `Some`, the value is the `no_debug` identifier. pub no_debug: Option, + /// Signal we should not include the `'db` lifetime. + /// + /// If this is `Some`, the value is the `no_lifetime` identifier. + pub no_lifetime: Option, + /// Signal we should not generate a `Clone` impl. /// /// If this is `Some`, the value is the `no_clone` identifier. @@ -66,6 +72,12 @@ pub(crate) struct Options { /// If this is `Some`, the value is the ``. pub constructor_name: Option, + /// The `id = ` option is used to set a custom ID for interrned structs. + /// + /// The ID must implement `salsa::plumbing::AsId` and `salsa::plumbing::FromId`. + /// If this is `Some`, the value is the ``. + pub id: Option, + /// Remember the `A` parameter, which plays no role after parsing. phantom: PhantomData, } @@ -77,6 +89,7 @@ impl Default for Options { specify: Default::default(), no_eq: Default::default(), no_debug: Default::default(), + no_lifetime: Default::default(), no_clone: Default::default(), db_path: Default::default(), recovery_fn: Default::default(), @@ -85,6 +98,7 @@ impl Default for Options { phantom: Default::default(), lru: Default::default(), singleton: Default::default(), + id: Default::default(), } } } @@ -95,6 +109,7 @@ pub(crate) trait AllowedOptions { const SPECIFY: bool; const NO_EQ: bool; const NO_DEBUG: bool; + const NO_LIFETIME: bool; const NO_CLONE: bool; const SINGLETON: bool; const DATA: bool; @@ -102,6 +117,7 @@ pub(crate) trait AllowedOptions { const RECOVERY_FN: bool; const LRU: bool; const CONSTRUCTOR_NAME: bool; + const ID: bool; } type Equals = syn::Token![=]; @@ -152,6 +168,20 @@ impl syn::parse::Parse for Options { "`no_debug` option not allowed here", )); } + } else if ident == "no_lifetime" { + if A::NO_LIFETIME { + if let Some(old) = std::mem::replace(&mut options.no_lifetime, Some(ident)) { + return Err(syn::Error::new( + old.span(), + "option `no_lifetime` provided twice", + )); + } + } else { + return Err(syn::Error::new( + ident.span(), + "`no_lifetime` option not allowed here", + )); + } } else if ident == "no_clone" { if A::NO_CLONE { if let Some(old) = std::mem::replace(&mut options.no_clone, Some(ident)) { @@ -267,6 +297,17 @@ impl syn::parse::Parse for Options { "`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..2027dcbad 100644 --- a/components/salsa-macros/src/salsa_struct.rs +++ b/components/salsa-macros/src/salsa_struct.rs @@ -47,6 +47,9 @@ pub(crate) trait SalsaStructAllowedOptions: AllowedOptions { /// Does this kind of struct have a `'db` lifetime? const HAS_LIFETIME: bool; + /// Can this struct elide the `'db` lifetime? + const ELIDABLE_LIFETIME: bool; + /// Are `#[default]` fields allowed? const ALLOW_DEFAULT: bool; } @@ -118,6 +121,14 @@ where } } + /// Returns the `id` in `Options` if it is `Some`, else `salsa::Id`. + 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. @@ -171,7 +182,11 @@ where /// Check that the generic parameters look as expected for this kind of struct. fn check_generics(&self) -> syn::Result<()> { if A::HAS_LIFETIME { - db_lifetime::require_db_lifetime(&self.struct_item.generics) + if !A::ELIDABLE_LIFETIME { + db_lifetime::require_db_lifetime(&self.struct_item.generics) + } else { + Ok(()) + } } else { db_lifetime::require_no_generics(&self.struct_item.generics) } @@ -279,6 +294,10 @@ where pub fn generate_debug_impl(&self) -> bool { self.args.no_debug.is_none() } + + pub fn generate_lifetime(&self) -> bool { + self.args.no_lifetime.is_none() + } } impl<'s> SalsaField<'s> { diff --git a/components/salsa-macros/src/tracked_fn.rs b/components/salsa-macros/src/tracked_fn.rs index a59072ef4..98db9a958 100644 --- a/components/salsa-macros/src/tracked_fn.rs +++ b/components/salsa-macros/src/tracked_fn.rs @@ -31,6 +31,8 @@ impl crate::options::AllowedOptions for TrackedFn { const NO_DEBUG: bool = false; + const NO_LIFETIME: bool = false; + const NO_CLONE: bool = false; const SINGLETON: bool = false; @@ -44,6 +46,8 @@ impl crate::options::AllowedOptions for TrackedFn { const LRU: bool = true; const CONSTRUCTOR_NAME: bool = false; + + const ID: bool = false; } struct Macro { @@ -160,6 +164,7 @@ impl Macro { Ok(ValidFn { db_ident, db_path }) } + fn cycle_recovery(&self) -> (TokenStream, TokenStream) { if let Some(recovery_fn) = &self.args.recovery_fn { (quote!((#recovery_fn)), quote!(Fallback)) diff --git a/components/salsa-macros/src/tracked_struct.rs b/components/salsa-macros/src/tracked_struct.rs index 1730b3404..5fc6fc68c 100644 --- a/components/salsa-macros/src/tracked_struct.rs +++ b/components/salsa-macros/src/tracked_struct.rs @@ -37,6 +37,8 @@ impl crate::options::AllowedOptions for TrackedStruct { const NO_DEBUG: bool = true; + const NO_LIFETIME: bool = false; + const NO_CLONE: bool = false; const SINGLETON: bool = true; @@ -50,6 +52,8 @@ impl crate::options::AllowedOptions for TrackedStruct { const LRU: bool = false; const CONSTRUCTOR_NAME: bool = true; + + const ID: bool = false; } impl SalsaStructAllowedOptions for TrackedStruct { @@ -59,6 +63,8 @@ impl SalsaStructAllowedOptions for TrackedStruct { const HAS_LIFETIME: bool = true; + const ELIDABLE_LIFETIME: bool = false; + const ALLOW_DEFAULT: bool = false; } 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, diff --git a/tests/interned-structs.rs b/tests/interned-structs.rs index a8263ce3a..2b3e19e2a 100644 --- a/tests/interned-structs.rs +++ b/tests/interned-structs.rs @@ -2,6 +2,7 @@ //! compiles and executes successfully. use expect_test::expect; +use salsa::plumbing::{AsId, FromId}; use std::path::{Path, PathBuf}; use test_log::test; @@ -36,6 +37,36 @@ struct InternedPathBuf<'db> { data1: PathBuf, } +#[salsa::interned(no_lifetime)] +struct InternedStringNoLifetime { + data: String, +} + +#[derive(Clone, Copy, Hash, Debug, PartialEq, Eq, PartialOrd, Ord)] +struct SalsaIdWrapper(salsa::Id); + +impl AsId for SalsaIdWrapper { + fn as_id(&self) -> salsa::Id { + self.0 + } +} + +impl FromId for SalsaIdWrapper { + fn from_id(id: salsa::Id) -> Self { + SalsaIdWrapper(id) + } +} + +#[salsa::interned(id = SalsaIdWrapper)] +struct InternedStringWithCustomId<'db> { + data: String, +} + +#[salsa::interned(id = SalsaIdWrapper, no_lifetime)] +struct InternedStringWithCustomIdAndNoLiftime<'db> { + data: String, +} + #[salsa::tracked] fn intern_stuff(db: &dyn salsa::Database) -> String { let s1 = InternedString::new(db, "Hello, ".to_string()); @@ -88,6 +119,18 @@ fn interning_boxed() { ); } +#[test] +fn interned_structs_have_public_ingredients() { + use salsa::plumbing::AsId; + + 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!"),)); +} + #[test] fn interning_vec() { let db = salsa::DatabaseImpl::new(); @@ -118,3 +161,39 @@ fn interning_path_buf() { assert_eq!(s1, s3); assert_ne!(s1, s4); } + +#[test] +fn interning_without_lifetimes() { + let db = salsa::DatabaseImpl::new(); + + let s1 = InternedStringNoLifetime::new(&db, "Hello, ".to_string()); + let s2 = InternedStringNoLifetime::new(&db, "World, ".to_string()); + let s1_2 = InternedStringNoLifetime::new(&db, "Hello, "); + let s2_2 = InternedStringNoLifetime::new(&db, "World, "); + assert_eq!(s1, s1_2); + assert_eq!(s2, s2_2); +} + +#[test] +fn interning_with_custom_ids() { + let db = salsa::DatabaseImpl::new(); + + let s1 = InternedStringWithCustomId::new(&db, "Hello, ".to_string()); + let s2 = InternedStringWithCustomId::new(&db, "World, ".to_string()); + let s1_2 = InternedStringWithCustomId::new(&db, "Hello, "); + let s2_2 = InternedStringWithCustomId::new(&db, "World, "); + assert_eq!(s1, s1_2); + assert_eq!(s2, s2_2); +} + +#[test] +fn interning_with_custom_ids_and_no_lifetime() { + let db = salsa::DatabaseImpl::new(); + + let s1 = InternedStringWithCustomIdAndNoLiftime::new(&db, "Hello, ".to_string()); + let s2 = InternedStringWithCustomIdAndNoLiftime::new(&db, "World, ".to_string()); + let s1_2 = InternedStringWithCustomIdAndNoLiftime::new(&db, "Hello, "); + let s2_2 = InternedStringWithCustomIdAndNoLiftime::new(&db, "World, "); + assert_eq!(s1, s1_2); + assert_eq!(s2, s2_2); +}