From 4822013523e4a7e82819ff667bfd486456728a20 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 13 May 2024 05:35:23 -0400 Subject: [PATCH] permit interned data to take 'db lifetime --- components/salsa-2022-macros/src/interned.rs | 70 ++++++++++++------- .../salsa-2022-macros/src/tracked_fn.rs | 13 +++- components/salsa-2022/src/interned.rs | 46 +++++++----- components/salsa-2022/src/tracked_struct.rs | 2 +- 4 files changed, 84 insertions(+), 47 deletions(-) diff --git a/components/salsa-2022-macros/src/interned.rs b/components/salsa-2022-macros/src/interned.rs index 2ce7594a..3667f6c1 100644 --- a/components/salsa-2022-macros/src/interned.rs +++ b/components/salsa-2022-macros/src/interned.rs @@ -54,8 +54,8 @@ impl crate::options::AllowedOptions for InternedStruct { impl InternedStruct { fn generate_interned(&self) -> syn::Result { self.validate_interned()?; - let id_struct = self.the_struct_id(); let config_struct = self.config_struct(); + let the_struct = self.the_struct(&config_struct.ident)?; let data_struct = self.data_struct(); let configuration_impl = self.configuration_impl(&data_struct.ident, &config_struct.ident); let ingredients_for_impl = self.ingredients_for_impl(&config_struct.ident); @@ -65,7 +65,7 @@ impl InternedStruct { let as_debug_with_db_impl = self.as_debug_with_db_impl(); Ok(quote! { - #id_struct + #the_struct #config_struct #configuration_impl #data_struct @@ -79,7 +79,7 @@ impl InternedStruct { fn validate_interned(&self) -> syn::Result<()> { self.disallow_id_fields("interned")?; - self.require_no_generics()?; + self.require_db_lifetime()?; Ok(()) } @@ -102,14 +102,19 @@ impl InternedStruct { /// /// When no named fields are available, copy the existing type. fn data_struct(&self) -> syn::ItemStruct { - let ident = self.data_ident(); + let data_ident = self.data_ident(); + let (_, _, impl_generics, _, where_clause) = self.the_ident_and_generics(); + let visibility = self.visibility(); let all_field_names = self.all_field_names(); let all_field_tys = self.all_field_tys(); - parse_quote_spanned! { ident.span() => + parse_quote_spanned! { data_ident.span() => /// Internal struct used for interned item #[derive(Eq, PartialEq, Hash, Clone)] - #visibility struct #ident { + #visibility struct #data_ident #impl_generics + where + #where_clause + { #( #all_field_names: #all_field_tys, )* @@ -119,14 +124,16 @@ impl InternedStruct { fn configuration_impl( &self, - data_struct: &syn::Ident, - config_struct: &syn::Ident, + data_ident: &syn::Ident, + config_ident: &syn::Ident, ) -> syn::ItemImpl { + let lt_db = &self.named_db_lifetime(); + let (_, _, _, type_generics, _) = self.the_ident_and_generics(); parse_quote_spanned!( - config_struct.span() => + config_ident.span() => - impl salsa::interned::Configuration for #config_struct { - type Data = #data_struct; + impl salsa::interned::Configuration for #config_ident { + type Data<#lt_db> = #data_ident #type_generics; } ) } @@ -134,8 +141,9 @@ impl InternedStruct { /// If this is an interned struct, then generate methods to access each field, /// as well as a `new` method. fn inherent_impl_for_named_fields(&self) -> syn::ItemImpl { - let vis = self.visibility(); - let id_ident = self.the_ident(); + let vis: &syn::Visibility = self.visibility(); + let (the_ident, _, impl_generics, type_generics, where_clause) = + self.the_ident_and_generics(); let db_dyn_ty = self.db_dyn_ty(); let jar_ty = self.jar_ty(); @@ -150,7 +158,7 @@ impl InternedStruct { parse_quote_spanned! { field_get_name.span() => #field_vis fn #field_get_name(self, db: &#db_dyn_ty) -> #field_ty { let (jar, runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(db); - let ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #id_ident >>::ingredient(jar); + let ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #the_ident #type_generics >>::ingredient(jar); std::clone::Clone::clone(&ingredients.data(runtime, self.0).#field_name) } } @@ -158,7 +166,7 @@ impl InternedStruct { parse_quote_spanned! { field_get_name.span() => #field_vis fn #field_get_name<'db>(self, db: &'db #db_dyn_ty) -> &'db #field_ty { let (jar, runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(db); - let ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #id_ident >>::ingredient(jar); + let ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #the_ident #type_generics >>::ingredient(jar); &ingredients.data(runtime, self.0).#field_name } } @@ -176,7 +184,7 @@ impl InternedStruct { #(#field_names: #field_tys,)* ) -> Self { let (jar, runtime) = <_ as salsa::storage::HasJar<#jar_ty>>::jar(db); - let ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #id_ident >>::ingredient(jar); + let ingredients = <#jar_ty as salsa::storage::HasIngredientsFor< #the_ident #type_generics >>::ingredient(jar); Self(ingredients.intern(runtime, #data_ident { #(#field_names,)* })) @@ -191,7 +199,10 @@ impl InternedStruct { parse_quote! { #[allow(dead_code, clippy::pedantic, clippy::complexity, clippy::style)] - impl #id_ident { + impl #impl_generics #the_ident #type_generics + where + #where_clause + { #(#field_getters)* #new_method @@ -204,14 +215,18 @@ impl InternedStruct { /// Generates an impl of `salsa::storage::IngredientsFor`. /// /// For a memoized type, the only ingredient is an `InternedIngredient`. - fn ingredients_for_impl(&self, config_struct: &syn::Ident) -> syn::ItemImpl { - let id_ident = self.the_ident(); - let debug_name = crate::literal(id_ident); + fn ingredients_for_impl(&self, config_ident: &syn::Ident) -> syn::ItemImpl { + let (the_ident, _, impl_generics, type_generics, where_clause) = + self.the_ident_and_generics(); + let debug_name = crate::literal(the_ident); let jar_ty = self.jar_ty(); parse_quote! { - impl salsa::storage::IngredientsFor for #id_ident { + impl #impl_generics salsa::storage::IngredientsFor for #the_ident #type_generics + where + #where_clause + { type Jar = #jar_ty; - type Ingredients = salsa::interned::InternedIngredient<#config_struct>; + type Ingredients = salsa::interned::InternedIngredient<#config_ident>; fn create_ingredients( routes: &mut salsa::routes::Routes, @@ -237,14 +252,17 @@ impl InternedStruct { /// Implementation of `SalsaStructInDb`. fn salsa_struct_in_db_impl(&self) -> syn::ItemImpl { - let ident = self.the_ident(); + let (the_ident, parameters, _, type_generics, where_clause) = self.the_ident_and_generics(); + #[allow(non_snake_case)] + let DB = syn::Ident::new("DB", the_ident.span()); let jar_ty = self.jar_ty(); parse_quote! { - impl salsa::salsa_struct::SalsaStructInDb for #ident + impl<#DB, #parameters> salsa::salsa_struct::SalsaStructInDb for #the_ident #type_generics where - DB: ?Sized + salsa::DbWithJar<#jar_ty>, + #DB: ?Sized + salsa::DbWithJar<#jar_ty>, + #where_clause { - fn register_dependent_fn(_db: &DB, _index: salsa::routes::IngredientIndex) { + fn register_dependent_fn(_db: &#DB, _index: salsa::routes::IngredientIndex) { // Do nothing here, at least for now. // If/when we add ability to delete inputs, this would become relevant. } diff --git a/components/salsa-2022-macros/src/tracked_fn.rs b/components/salsa-2022-macros/src/tracked_fn.rs index 8cee8aab..0cdafecb 100644 --- a/components/salsa-2022-macros/src/tracked_fn.rs +++ b/components/salsa-2022-macros/src/tracked_fn.rs @@ -4,6 +4,7 @@ use syn::visit_mut::VisitMut; use syn::{ReturnType, Token}; use crate::configuration::{self, Configuration, CycleRecoveryStrategy}; +use crate::db_lifetime::{db_lifetime, require_db_lifetime}; use crate::options::Options; pub(crate) fn tracked_fn( @@ -288,12 +289,14 @@ fn rename_self_in_block(mut block: syn::Block) -> syn::Result { /// /// This returns the name of the constructed type and the code defining everything. fn fn_struct(args: &FnArgs, item_fn: &syn::ItemFn) -> syn::Result<(syn::Type, TokenStream)> { + require_db_lifetime(&item_fn.sig.generics)?; + let db_lt = &db_lifetime(&item_fn.sig.generics); let struct_item = configuration_struct(item_fn); let configuration = fn_configuration(args, item_fn); let struct_item_ident = &struct_item.ident; let config_ty: syn::Type = parse_quote!(#struct_item_ident); let configuration_impl = configuration.to_impl(&config_ty); - let interned_configuration_impl = interned_configuration_impl(item_fn, &config_ty); + let interned_configuration_impl = interned_configuration_impl(db_lt, item_fn, &config_ty); let ingredients_for_impl = ingredients_for_impl(args, item_fn, &config_ty); let item_impl = setter_impl(args, item_fn, &config_ty)?; @@ -309,7 +312,11 @@ fn fn_struct(args: &FnArgs, item_fn: &syn::ItemFn) -> syn::Result<(syn::Type, To )) } -fn interned_configuration_impl(item_fn: &syn::ItemFn, config_ty: &syn::Type) -> syn::ItemImpl { +fn interned_configuration_impl( + db_lt: &syn::Lifetime, + item_fn: &syn::ItemFn, + config_ty: &syn::Type, +) -> syn::ItemImpl { let arg_tys = item_fn.sig.inputs.iter().skip(1).map(|arg| match arg { syn::FnArg::Receiver(_) => unreachable!(), syn::FnArg::Typed(pat_ty) => pat_ty.ty.clone(), @@ -321,7 +328,7 @@ fn interned_configuration_impl(item_fn: &syn::ItemFn, config_ty: &syn::Type) -> parse_quote!( impl salsa::interned::Configuration for #config_ty { - type Data = #intern_data_ty; + type Data<#db_lt> = #intern_data_ty; } ) } diff --git a/components/salsa-2022/src/interned.rs b/components/salsa-2022/src/interned.rs index c7639aeb..5301812f 100644 --- a/components/salsa-2022/src/interned.rs +++ b/components/salsa-2022/src/interned.rs @@ -8,7 +8,6 @@ use crate::durability::Durability; use crate::id::AsId; use crate::ingredient::{fmt_index, IngredientRequiresReset}; use crate::key::DependencyIndex; -use crate::plumbing::transmute_lifetime; use crate::runtime::local_state::QueryOrigin; use crate::runtime::Runtime; use crate::{DatabaseKeyIndex, Id}; @@ -19,7 +18,7 @@ use super::routes::IngredientIndex; use super::Revision; pub trait Configuration { - type Data: InternedData; + type Data<'db>: InternedData; } pub trait InternedData: Sized + Eq + Hash + Clone {} @@ -35,12 +34,12 @@ pub struct InternedIngredient { /// Maps from data to the existing interned id for that data. /// /// Deadlock requirement: We access `value_map` while holding lock on `key_map`, but not vice versa. - key_map: FxDashMap, + key_map: FxDashMap, Id>, /// Maps from an interned id to its data. /// /// Deadlock requirement: We access `value_map` while holding lock on `key_map`, but not vice versa. - value_map: FxDashMap>, + value_map: FxDashMap>>, /// counter for the next id. counter: AtomicCell, @@ -56,7 +55,7 @@ pub struct InternedIngredient { /// references to that data floating about that are tied to the lifetime of some /// `&db` reference. This queue itself is not freed until we have an `&mut db` reference, /// guaranteeing that there are no more references to it. - deleted_entries: SegQueue>, + deleted_entries: SegQueue>>, debug_name: &'static str, } @@ -77,12 +76,24 @@ where } } + unsafe fn to_internal_data<'db>(&'db self, data: C::Data<'db>) -> C::Data<'static> { + unsafe { std::mem::transmute(data) } + } + + unsafe fn from_internal_data<'db>(&'db self, data: &C::Data<'static>) -> &'db C::Data<'db> { + unsafe { std::mem::transmute(data) } + } + /// Intern `data` and return `(id, b`) where /// /// * `id` is the interned id /// * `b` is a boolean, `true` indicates this fn call added `data` to the interning table; /// `false` indicates it was already present - pub(crate) fn intern_full(&self, runtime: &Runtime, data: C::Data) -> (Id, bool) { + pub(crate) fn intern_full<'db>( + &'db self, + runtime: &'db Runtime, + data: C::Data<'db>, + ) -> (Id, bool) { runtime.report_tracked_read( DependencyIndex::for_table(self.ingredient_index), Durability::MAX, @@ -91,18 +102,19 @@ where // Optimisation to only get read lock on the map if the data has already // been interned. - if let Some(id) = self.key_map.get(&data) { + let internal_data = unsafe { self.to_internal_data(data) }; + if let Some(id) = self.key_map.get(&internal_data) { return (*id, false); } - match self.key_map.entry(data.clone()) { + match self.key_map.entry(internal_data.clone()) { // Data has been interned by a racing call, use that ID instead dashmap::mapref::entry::Entry::Occupied(entry) => (*entry.get(), false), // We won any races so should intern the data dashmap::mapref::entry::Entry::Vacant(entry) => { let next_id = self.counter.fetch_add(1); let next_id = Id::from_id(crate::id::Id::from_u32(next_id)); - let old_value = self.value_map.insert(next_id, Box::new(data)); + let old_value = self.value_map.insert(next_id, Box::new(internal_data)); assert!( old_value.is_none(), "next_id is guaranteed to be unique, bar overflow" @@ -113,7 +125,7 @@ where } } - pub fn intern(&self, runtime: &Runtime, data: C::Data) -> Id { + pub fn intern<'db>(&'db self, runtime: &'db Runtime, data: C::Data<'db>) -> Id { self.intern_full(runtime, data).0 } @@ -129,7 +141,7 @@ where } #[track_caller] - pub fn data<'db>(&'db self, runtime: &'db Runtime, id: Id) -> &'db C::Data { + pub fn data<'db>(&'db self, runtime: &'db Runtime, id: Id) -> &'db C::Data<'db> { runtime.report_tracked_read( DependencyIndex::for_table(self.ingredient_index), Durability::MAX, @@ -146,7 +158,7 @@ where // Unsafety clause: // // * Values are only removed or altered when we have `&mut self` - unsafe { transmute_lifetime(self, &**data) } + unsafe { self.from_internal_data(&data) } } /// Get the ingredient index for this table. @@ -261,7 +273,7 @@ where pub struct IdentityInterner where C: Configuration, - C::Data: AsId, + for<'db> C::Data<'db>: AsId, { data: PhantomData, } @@ -269,18 +281,18 @@ where impl IdentityInterner where C: Configuration, - C::Data: AsId, + for<'db> C::Data<'db>: AsId, { #[allow(clippy::new_without_default)] pub fn new() -> Self { IdentityInterner { data: PhantomData } } - pub fn intern(&self, _runtime: &Runtime, id: C::Data) -> crate::Id { + pub fn intern<'db>(&'db self, _runtime: &'db Runtime, id: C::Data<'db>) -> crate::Id { id.as_id() } - pub fn data(&self, _runtime: &Runtime, id: crate::Id) -> C::Data { - ::from_id(id) + pub fn data<'db>(&'db self, _runtime: &'db Runtime, id: crate::Id) -> C::Data<'db> { + >::from_id(id) } } diff --git a/components/salsa-2022/src/tracked_struct.rs b/components/salsa-2022/src/tracked_struct.rs index 4240a428..c9fdb134 100644 --- a/components/salsa-2022/src/tracked_struct.rs +++ b/components/salsa-2022/src/tracked_struct.rs @@ -111,7 +111,7 @@ struct TrackedStructKey { } impl crate::interned::Configuration for TrackedStructKey { - type Data = TrackedStructKey; + type Data<'db> = TrackedStructKey; } // ANCHOR: TrackedStructValue