From ce6428fbbd20f63c754e327f0f9d35ab8a1a788c Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Tue, 21 Jul 2020 23:59:28 +0200 Subject: [PATCH 1/3] Lift the static restriction on the traits --- book/src/rfcs/RFC0006-Dynamic-Databases.md | 6 +- components/salsa-macros/src/query_group.rs | 12 ++- src/debug.rs | 6 +- src/derived.rs | 14 ++-- src/derived/slot.rs | 23 +++--- src/input.rs | 24 +++--- src/interned.rs | 86 +++++++++------------- src/lib.rs | 28 +++---- src/plumbing.rs | 36 +++++---- 9 files changed, 124 insertions(+), 111 deletions(-) diff --git a/book/src/rfcs/RFC0006-Dynamic-Databases.md b/book/src/rfcs/RFC0006-Dynamic-Databases.md index 49e9cd5e..9b1fc2b5 100644 --- a/book/src/rfcs/RFC0006-Dynamic-Databases.md +++ b/book/src/rfcs/RFC0006-Dynamic-Databases.md @@ -310,8 +310,8 @@ Therefore `QueryFunction` for example can become: ```rust,ignore pub trait QueryFunction: Query { - fn execute(db: &Self::DynDb, key: Self::Key) -> Self::Value; - fn recover(db: &Self::DynDb, cycle: &[DB::DatabaseKey], key: &Self::Key) -> Option { + fn execute(db: &>::DynDb, key: Self::Key) -> Self::Value; + fn recover(db: &>::DynDb, cycle: &[DB::DatabaseKey], key: &Self::Key) -> Option { let _ = (db, cycle, key); None } @@ -455,7 +455,7 @@ we have to make a few changes: One downside of this proposal is that the `salsa::Database` trait now has a `'static` bound. This is a result of the lack of GATs -- in particular, the -queries expect a `Q::DynDb` as argument. In the query definition, we have +queries expect a `>::DynDb` as argument. In the query definition, we have something like `type DynDb = dyn QueryGroupDatabase`, which in turn defaults to `dyn::QueryGroupDatabase + 'static`. diff --git a/components/salsa-macros/src/query_group.rs b/components/salsa-macros/src/query_group.rs index 917dea2d..00265113 100644 --- a/components/salsa-macros/src/query_group.rs +++ b/components/salsa-macros/src/query_group.rs @@ -373,7 +373,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream /// single input. You can also use it to invoke this query, though /// it's more common to use the trait method on the database /// itself. - #trait_vis fn in_db(self, db: &#dyn_db) -> salsa::QueryTable<'_, Self> + #trait_vis fn in_db(self, db: &#dyn_db) -> salsa::QueryTable<'_, Self, #dyn_db> { salsa::plumbing::get_query_table::<#qt>(db) } @@ -416,6 +416,11 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream } } + impl<'d> salsa::QueryDb<'d> for #qt + { + type DynDb = #dyn_db + 'd; + } + // ANCHOR:Query_impl impl salsa::Query for #qt { @@ -424,7 +429,6 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream type Storage = #storage; type Group = #group_struct; type GroupStorage = #group_storage; - type DynDb = #dyn_db; const QUERY_INDEX: u16 = #query_index; @@ -454,7 +458,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream let recover = if let Some(cycle_recovery_fn) = &query.cycle { quote! { - fn recover(db: &Self::DynDb, cycle: &[salsa::DatabaseKeyIndex], #key_pattern: &::Key) + fn recover(db: &>::DynDb, cycle: &[salsa::DatabaseKeyIndex], #key_pattern: &::Key) -> Option<::Value> { Some(#cycle_recovery_fn( db, @@ -471,7 +475,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream // ANCHOR:QueryFunction_impl impl salsa::plumbing::QueryFunction for #qt { - fn execute(db: &Self::DynDb, #key_pattern: ::Key) + fn execute(db: &>::DynDb, #key_pattern: ::Key) -> ::Value { #invoke(db, #(#key_names),*) } diff --git a/src/debug.rs b/src/debug.rs index f95516ab..8f067eeb 100644 --- a/src/debug.rs +++ b/src/debug.rs @@ -4,7 +4,7 @@ use crate::durability::Durability; use crate::plumbing::QueryStorageOps; use crate::Query; -use crate::QueryTable; +use crate::{QueryDb, QueryTable}; use std::iter::FromIterator; /// Additional methods on queries that can be used to "peek into" @@ -49,9 +49,9 @@ impl TableEntry { } } -impl DebugQueryTable for QueryTable<'_, Q> +impl DebugQueryTable for QueryTable<'_, Q, DB> where - Q: Query, + Q: Query + for<'d> QueryDb<'d, DynDb = DB>, { type Key = Q::Key; type Value = Q::Value; diff --git a/src/derived.rs b/src/derived.rs index 4d632282..f8a34b7a 100644 --- a/src/derived.rs +++ b/src/derived.rs @@ -7,7 +7,7 @@ use crate::plumbing::QueryFunction; use crate::plumbing::QueryStorageMassOps; use crate::plumbing::QueryStorageOps; use crate::runtime::{FxIndexMap, StampedValue}; -use crate::{CycleError, Database, DatabaseKeyIndex, Revision, Runtime, SweepStrategy}; +use crate::{CycleError, Database, DatabaseKeyIndex, QueryDb, Revision, Runtime, SweepStrategy}; use parking_lot::RwLock; use std::convert::TryFrom; use std::marker::PhantomData; @@ -126,7 +126,7 @@ where fn fmt_index( &self, - _db: &Q::DynDb, + _db: &>::DynDb, index: DatabaseKeyIndex, fmt: &mut std::fmt::Formatter<'_>, ) -> std::fmt::Result { @@ -139,7 +139,7 @@ where fn maybe_changed_since( &self, - db: &Q::DynDb, + db: &>::DynDb, input: DatabaseKeyIndex, revision: Revision, ) -> bool { @@ -157,7 +157,7 @@ where fn try_fetch( &self, - db: &Q::DynDb, + db: &>::DynDb, key: &Q::Key, ) -> Result> { let slot = self.slot(key); @@ -177,11 +177,11 @@ where Ok(value) } - fn durability(&self, db: &Q::DynDb, key: &Q::Key) -> Durability { + fn durability(&self, db: &>::DynDb, key: &Q::Key) -> Durability { self.slot(key).durability(db) } - fn entries(&self, _db: &Q::DynDb) -> C + fn entries(&self, _db: &>::DynDb) -> C where C: std::iter::FromIterator>, { @@ -222,7 +222,7 @@ where Q: QueryFunction, MP: MemoizationPolicy, { - fn invalidate(&self, db: &mut Q::DynDb, key: &Q::Key) { + fn invalidate(&self, db: &mut >::DynDb, key: &Q::Key) { db.salsa_runtime_mut() .with_incremented_revision(&mut |_new_revision| { let map_read = self.slot_map.read(); diff --git a/src/derived/slot.rs b/src/derived/slot.rs index ac750a76..bf490f21 100644 --- a/src/derived/slot.rs +++ b/src/derived/slot.rs @@ -11,7 +11,8 @@ use crate::runtime::Runtime; use crate::runtime::RuntimeId; use crate::runtime::StampedValue; use crate::{ - CycleError, Database, DatabaseKeyIndex, DiscardIf, DiscardWhat, Event, EventKind, SweepStrategy, + CycleError, Database, DatabaseKeyIndex, DiscardIf, DiscardWhat, Event, EventKind, QueryDb, + SweepStrategy, }; use log::{debug, info}; use parking_lot::Mutex; @@ -125,7 +126,7 @@ where pub(super) fn read( &self, - db: &Q::DynDb, + db: &>::DynDb, ) -> Result, CycleError> { let runtime = db.salsa_runtime(); @@ -153,7 +154,7 @@ where /// shows a potentially out of date value. fn read_upgrade( &self, - db: &Q::DynDb, + db: &>::DynDb, revision_now: Revision, ) -> Result, CycleError> { let runtime = db.salsa_runtime(); @@ -326,7 +327,7 @@ where /// Note that in case `ProbeState::UpToDate`, the lock will have been released. fn probe( &self, - db: &Q::DynDb, + db: &>::DynDb, state: StateGuard, runtime: &Runtime, revision_now: Revision, @@ -419,7 +420,7 @@ where ProbeState::StaleOrAbsent(state) } - pub(super) fn durability(&self, db: &Q::DynDb) -> Durability { + pub(super) fn durability(&self, db: &>::DynDb) -> Durability { match &*self.state.read() { QueryState::NotComputed => Durability::LOW, QueryState::InProgress { .. } => panic!("query in progress"), @@ -532,7 +533,11 @@ where } } - pub(super) fn maybe_changed_since(&self, db: &Q::DynDb, revision: Revision) -> bool { + pub(super) fn maybe_changed_since( + &self, + db: &>::DynDb, + revision: Revision, + ) -> bool { let runtime = db.salsa_runtime(); let revision_now = runtime.current_revision(); @@ -721,7 +726,7 @@ where /// computed (but first drop the lock on the map). fn register_with_in_progress_thread( &self, - _db: &Q::DynDb, + _db: &>::DynDb, runtime: &Runtime, other_id: RuntimeId, waiting: &Mutex>; 2]>>, @@ -886,7 +891,7 @@ where { fn validate_memoized_value( &mut self, - db: &Q::DynDb, + db: &>::DynDb, revision_now: Revision, ) -> Option> { // If we don't have a memoized value, nothing to validate. @@ -1032,7 +1037,7 @@ where #[allow(dead_code)] fn check_static() where - Q: QueryFunction, + Q: QueryFunction + 'static, MP: MemoizationPolicy, Q::Key: 'static, Q::Value: 'static, diff --git a/src/input.rs b/src/input.rs index d9473a93..93001989 100644 --- a/src/input.rs +++ b/src/input.rs @@ -8,7 +8,7 @@ use crate::runtime::{FxIndexMap, StampedValue}; use crate::CycleError; use crate::Database; use crate::Query; -use crate::{DatabaseKeyIndex, Runtime, SweepStrategy}; +use crate::{DatabaseKeyIndex, QueryDb, Runtime, SweepStrategy}; use indexmap::map::Entry; use log::debug; use parking_lot::RwLock; @@ -65,7 +65,7 @@ where fn fmt_index( &self, - _db: &Q::DynDb, + _db: &>::DynDb, index: DatabaseKeyIndex, fmt: &mut std::fmt::Formatter<'_>, ) -> std::fmt::Result { @@ -78,7 +78,7 @@ where fn maybe_changed_since( &self, - db: &Q::DynDb, + db: &>::DynDb, input: DatabaseKeyIndex, revision: Revision, ) -> bool { @@ -96,7 +96,7 @@ where fn try_fetch( &self, - db: &Q::DynDb, + db: &>::DynDb, key: &Q::Key, ) -> Result> { let slot = self @@ -115,14 +115,14 @@ where Ok(value) } - fn durability(&self, _db: &Q::DynDb, key: &Q::Key) -> Durability { + fn durability(&self, _db: &>::DynDb, key: &Q::Key) -> Durability { match self.slot(key) { Some(slot) => slot.stamped_value.read().durability, None => panic!("no value set for {:?}({:?})", Q::default(), key), } } - fn entries(&self, _db: &Q::DynDb) -> C + fn entries(&self, _db: &>::DynDb) -> C where C: std::iter::FromIterator>, { @@ -143,7 +143,7 @@ impl Slot where Q: Query, { - fn maybe_changed_since(&self, _db: &Q::DynDb, revision: Revision) -> bool { + fn maybe_changed_since(&self, _db: &>::DynDb, revision: Revision) -> bool { debug!( "maybe_changed_since(slot={:?}, revision={:?})", self, revision, @@ -168,7 +168,13 @@ impl InputQueryStorageOps for InputStorage where Q: Query, { - fn set(&self, db: &mut Q::DynDb, key: &Q::Key, value: Q::Value, durability: Durability) { + fn set( + &self, + db: &mut >::DynDb, + key: &Q::Key, + value: Q::Value, + durability: Durability, + ) { log::debug!( "{:?}({:?}) = {:?} ({:?})", Q::default(), @@ -254,7 +260,7 @@ where #[allow(dead_code)] fn check_static() where - Q: Query, + Q: Query + 'static, Q::Key: 'static, Q::Value: 'static, { diff --git a/src/interned.rs b/src/interned.rs index c0464680..65c42ed4 100644 --- a/src/interned.rs +++ b/src/interned.rs @@ -6,7 +6,7 @@ use crate::plumbing::QueryStorageMassOps; use crate::plumbing::QueryStorageOps; use crate::revision::Revision; use crate::Query; -use crate::{CycleError, Database, DatabaseKeyIndex, DiscardIf, Runtime, SweepStrategy}; +use crate::{CycleError, Database, DatabaseKeyIndex, DiscardIf, QueryDb, Runtime, SweepStrategy}; use crossbeam_utils::atomic::AtomicCell; use parking_lot::RwLock; use rustc_hash::FxHashMap; @@ -35,13 +35,6 @@ where Q: Query, Q::Key: InternKey, Q::Value: Eq + Hash, - IQ: Query< - Key = Q::Value, - Value = Q::Key, - Group = Q::Group, - DynDb = Q::DynDb, - GroupStorage = Q::GroupStorage, - >, { phantom: std::marker::PhantomData<(Q::Key, IQ)>, } @@ -186,7 +179,7 @@ where /// In either case, the `accessed_at` field of the slot is updated /// to the current revision, ensuring that the slot cannot be GC'd /// while the current queries execute. - fn intern_index(&self, db: &Q::DynDb, key: &Q::Key) -> Arc> { + fn intern_index(&self, db: &>::DynDb, key: &Q::Key) -> Arc> { if let Some(i) = self.intern_check(db, key) { return i; } @@ -268,7 +261,11 @@ where slot } - fn intern_check(&self, db: &Q::DynDb, key: &Q::Key) -> Option>> { + fn intern_check( + &self, + db: &>::DynDb, + key: &Q::Key, + ) -> Option>> { let revision_now = db.salsa_runtime().current_revision(); let slot = self.tables.read().slot_for_key(key, revision_now)?; Some(slot) @@ -276,7 +273,7 @@ where /// Given an index, lookup and clone its value, updating the /// `accessed_at` time if necessary. - fn lookup_value(&self, db: &Q::DynDb, index: InternId) -> Arc> { + fn lookup_value(&self, db: &>::DynDb, index: InternId) -> Arc> { let revision_now = db.salsa_runtime().current_revision(); self.tables.read().slot_for_index(index, revision_now) } @@ -296,7 +293,7 @@ where fn fmt_index( &self, - db: &Q::DynDb, + db: &>::DynDb, index: DatabaseKeyIndex, fmt: &mut std::fmt::Formatter<'_>, ) -> std::fmt::Result { @@ -309,7 +306,7 @@ where fn maybe_changed_since( &self, - db: &Q::DynDb, + db: &>::DynDb, input: DatabaseKeyIndex, revision: Revision, ) -> bool { @@ -322,7 +319,7 @@ where fn try_fetch( &self, - db: &Q::DynDb, + db: &>::DynDb, key: &Q::Key, ) -> Result> { let slot = self.intern_index(db, key); @@ -336,11 +333,11 @@ where Ok(::from_intern_id(index)) } - fn durability(&self, _db: &Q::DynDb, _key: &Q::Key) -> Durability { + fn durability(&self, _db: &>::DynDb, _key: &Q::Key) -> Durability { INTERN_DURABILITY } - fn entries(&self, _db: &Q::DynDb) -> C + fn entries(&self, _db: &>::DynDb) -> C where C: std::iter::FromIterator>, { @@ -409,19 +406,19 @@ where } } -impl QueryStorageOps for LookupInternedStorage +impl QueryStorageOps for LookupInternedStorage where - Q: Query, + Q: Query + for<'d> QueryDb<'d, DynDb = DB>, Q::Key: InternKey, Q::Value: Eq + Hash, IQ: Query< - Key = Q::Value, - Value = Q::Key, - Storage = InternedStorage, - Group = Q::Group, - DynDb = Q::DynDb, - GroupStorage = Q::GroupStorage, - >, + Key = Q::Value, + Value = Q::Key, + Storage = InternedStorage, + Group = Q::Group, + GroupStorage = Q::GroupStorage, + > + for<'d> QueryDb<'d, DynDb = DB>, + DB: ?Sized + Database + HasQueryGroup, { fn new(_group_index: u16) -> Self { LookupInternedStorage { @@ -431,33 +428,27 @@ where fn fmt_index( &self, - db: &Q::DynDb, + db: &DB, index: DatabaseKeyIndex, fmt: &mut std::fmt::Formatter<'_>, ) -> std::fmt::Result { - let group_storage = >::group_storage(db); + let group_storage = + <>::DynDb as HasQueryGroup>::group_storage(db); let interned_storage = IQ::query_storage(group_storage); interned_storage.fmt_index(db, index, fmt) } - fn maybe_changed_since( - &self, - db: &Q::DynDb, - input: DatabaseKeyIndex, - revision: Revision, - ) -> bool { - let group_storage = >::group_storage(db); + fn maybe_changed_since(&self, db: &DB, input: DatabaseKeyIndex, revision: Revision) -> bool { + let group_storage = + <>::DynDb as HasQueryGroup>::group_storage(db); let interned_storage = IQ::query_storage(group_storage); interned_storage.maybe_changed_since(db, input, revision) } - fn try_fetch( - &self, - db: &Q::DynDb, - key: &Q::Key, - ) -> Result> { + fn try_fetch(&self, db: &DB, key: &Q::Key) -> Result> { let index = key.as_intern_id(); - let group_storage = >::group_storage(db); + let group_storage = + <>::DynDb as HasQueryGroup>::group_storage(db); let interned_storage = IQ::query_storage(group_storage); let slot = interned_storage.lookup_value(db, index); let value = slot.value.clone(); @@ -470,15 +461,16 @@ where Ok(value) } - fn durability(&self, _db: &Q::DynDb, _key: &Q::Key) -> Durability { + fn durability(&self, _db: &DB, _key: &Q::Key) -> Durability { INTERN_DURABILITY } - fn entries(&self, db: &Q::DynDb) -> C + fn entries(&self, db: &DB) -> C where C: std::iter::FromIterator>, { - let group_storage = >::group_storage(db); + let group_storage = + <>::DynDb as HasQueryGroup>::group_storage(db); let interned_storage = IQ::query_storage(group_storage); let tables = interned_storage.tables.read(); tables @@ -496,13 +488,7 @@ where Q: Query, Q::Key: InternKey, Q::Value: Eq + Hash, - IQ: Query< - Key = Q::Value, - Value = Q::Key, - Group = Q::Group, - DynDb = Q::DynDb, - GroupStorage = Q::GroupStorage, - >, + IQ: Query, { fn sweep(&self, _: &Runtime, _strategy: SweepStrategy) {} } diff --git a/src/lib.rs b/src/lib.rs index b3fe8b8f..1c255a54 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -45,7 +45,7 @@ pub use crate::storage::Storage; /// The base trait which your "query context" must implement. Gives /// access to the salsa runtime, which you must embed into your query /// context (along with whatever other state you may require). -pub trait Database: 'static + plumbing::DatabaseOps { +pub trait Database: plumbing::DatabaseOps { /// Iterates through all query storage and removes any values that /// have not been used since the last revision was created. The /// intended use-cycle is that you first execute all of your @@ -418,9 +418,13 @@ where } } +pub trait QueryDb<'d>: Sized { + type DynDb: ?Sized + Database + 'd; +} + /// Trait implements by all of the "special types" associated with /// each of your queries. -pub trait Query: Debug + Default + Sized + 'static { +pub trait Query: Debug + Default + Sized + for<'d> QueryDb<'d> { /// Type that you you give as a parameter -- for queries with zero /// or more than one input, this will be a tuple. type Key: Clone + Debug + Hash + Eq; @@ -432,14 +436,11 @@ pub trait Query: Debug + Default + Sized + 'static { type Storage: plumbing::QueryStorageOps; /// Associate query group struct. - type Group: plumbing::QueryGroup; + type Group: plumbing::QueryGroup; /// Generated struct that contains storage for all queries in a group. type GroupStorage; - /// Dyn version of the associated trait for this query group. - type DynDb: ?Sized + Database + HasQueryGroup; - /// A unique index identifying this query within the group. const QUERY_INDEX: u16; @@ -454,20 +455,21 @@ pub trait Query: Debug + Default + Sized + 'static { /// Gives access to various less common operations on queries. /// /// [the `query` method]: trait.Database.html#method.query -pub struct QueryTable<'me, Q> +pub struct QueryTable<'me, Q, DB> where - Q: Query + 'me, + Q: Query, + DB: ?Sized, { - db: &'me Q::DynDb, + db: &'me DB, storage: &'me Q::Storage, } -impl<'me, Q> QueryTable<'me, Q> +impl<'me, 'd, Q> QueryTable<'me, Q, >::DynDb> where Q: Query, { /// Constructs a new `QueryTable`. - pub fn new(db: &'me Q::DynDb, storage: &'me Q::Storage) -> Self { + pub fn new(db: &'me >::DynDb, storage: &'me Q::Storage) -> Self { Self { db, storage } } @@ -502,7 +504,7 @@ pub struct QueryTableMut<'me, Q> where Q: Query + 'me, { - db: &'me mut Q::DynDb, + db: &'me mut >::DynDb, storage: Arc, } @@ -511,7 +513,7 @@ where Q: Query, { /// Constructs a new `QueryTableMut`. - pub fn new(db: &'me mut Q::DynDb, storage: Arc) -> Self { + pub fn new(db: &'me mut >::DynDb, storage: Arc) -> Self { Self { db, storage } } diff --git a/src/plumbing.rs b/src/plumbing.rs index 036274b4..47d51ed6 100644 --- a/src/plumbing.rs +++ b/src/plumbing.rs @@ -17,7 +17,7 @@ pub use crate::derived::MemoizedStorage; pub use crate::input::InputStorage; pub use crate::interned::InternedStorage; pub use crate::interned::LookupInternedStorage; -pub use crate::{revision::Revision, DatabaseKeyIndex, Runtime}; +pub use crate::{revision::Revision, DatabaseKeyIndex, QueryDb, Runtime}; #[derive(Clone, Debug)] pub struct CycleDetected { @@ -71,10 +71,10 @@ pub trait QueryStorageMassOps { pub trait DatabaseKey: Clone + Debug + Eq + Hash {} pub trait QueryFunction: Query { - fn execute(db: &Self::DynDb, key: Self::Key) -> Self::Value; + fn execute(db: &>::DynDb, key: Self::Key) -> Self::Value; fn recover( - db: &Self::DynDb, + db: &>::DynDb, cycle: &[DatabaseKeyIndex], key: &Self::Key, ) -> Option { @@ -85,9 +85,10 @@ pub trait QueryFunction: Query { /// Create a query table, which has access to the storage for the query /// and offers methods like `get`. -pub fn get_query_table(db: &Q::DynDb) -> QueryTable<'_, Q> +pub fn get_query_table<'me, Q, DB>(db: &'me DB) -> QueryTable<'me, Q, DB> where - Q: Query, + Q: Query + for<'d> QueryDb<'d, DynDb = DB> + 'me, + DB: HasQueryGroup, { let group_storage: &Q::GroupStorage = HasQueryGroup::group_storage(db); let query_storage: &Q::Storage = Q::query_storage(group_storage); @@ -96,9 +97,12 @@ where /// Create a mutable query table, which has access to the storage /// for the query and offers methods like `set`. -pub fn get_query_table_mut(db: &mut Q::DynDb) -> QueryTableMut<'_, Q> +pub fn get_query_table_mut<'me, Q>( + db: &'me mut >::DynDb, +) -> QueryTableMut<'me, Q> where Q: Query, + >::DynDb: HasQueryGroup, { let group_storage: &Q::GroupStorage = HasQueryGroup::group_storage(db); let query_storage = Q::query_storage(group_storage).clone(); @@ -132,7 +136,7 @@ where /// Format a database key index in a suitable way. fn fmt_index( &self, - db: &Q::DynDb, + db: &>::DynDb, index: DatabaseKeyIndex, fmt: &mut std::fmt::Formatter<'_>, ) -> std::fmt::Result; @@ -141,7 +145,7 @@ where /// changed since the given revision. fn maybe_changed_since( &self, - db: &Q::DynDb, + db: &>::DynDb, input: DatabaseKeyIndex, revision: Revision, ) -> bool; @@ -155,15 +159,15 @@ where /// itself. fn try_fetch( &self, - db: &Q::DynDb, + db: &>::DynDb, key: &Q::Key, ) -> Result>; /// Returns the durability associated with a given key. - fn durability(&self, db: &Q::DynDb, key: &Q::Key) -> Durability; + fn durability(&self, db: &>::DynDb, key: &Q::Key) -> Durability; /// Get the (current) set of the entries in the query storage - fn entries(&self, db: &Q::DynDb) -> C + fn entries(&self, db: &>::DynDb) -> C where C: std::iter::FromIterator>; } @@ -175,7 +179,13 @@ pub trait InputQueryStorageOps where Q: Query, { - fn set(&self, db: &mut Q::DynDb, key: &Q::Key, new_value: Q::Value, durability: Durability); + fn set( + &self, + db: &mut >::DynDb, + key: &Q::Key, + new_value: Q::Value, + durability: Durability, + ); } /// An optional trait that is implemented for "user mutable" storage: @@ -189,5 +199,5 @@ pub trait DerivedQueryStorageOps where Q: Query, { - fn invalidate(&self, db: &mut Q::DynDb, key: &Q::Key); + fn invalidate(&self, db: &mut >::DynDb, key: &Q::Key); } From e2ab6c8bfa7b381c0301c57dbc8c111e6521f6c6 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Wed, 22 Jul 2020 11:36:20 +0200 Subject: [PATCH 2/3] feat: Allow the dynamic db to be non-static --- components/salsa-macros/src/query_group.rs | 18 ++--- examples/compiler/compiler.rs | 6 +- src/debug.rs | 5 +- src/derived.rs | 2 +- src/derived/slot.rs | 2 +- src/interned.rs | 82 ++++++++++++++++------ src/lib.rs | 31 +++++--- src/plumbing.rs | 14 ++-- 8 files changed, 103 insertions(+), 57 deletions(-) diff --git a/components/salsa-macros/src/query_group.rs b/components/salsa-macros/src/query_group.rs index 00265113..8ca133c7 100644 --- a/components/salsa-macros/src/query_group.rs +++ b/components/salsa-macros/src/query_group.rs @@ -217,7 +217,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream // query crate. Our experiments revealed that this makes a big // difference in total compilation time in rust-analyzer, though // it's not totally obvious why that should be. - fn __shim(db: &dyn #trait_name, #(#key_names: #keys),*) -> #value { + fn __shim(db: &(dyn #trait_name + '_), #(#key_names: #keys),*) -> #value { salsa::plumbing::get_query_table::<#qt>(db).get((#(#key_names),*)) } __shim(self, #(#key_names),*) @@ -373,7 +373,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream /// single input. You can also use it to invoke this query, though /// it's more common to use the trait method on the database /// itself. - #trait_vis fn in_db(self, db: &#dyn_db) -> salsa::QueryTable<'_, Self, #dyn_db> + #trait_vis fn in_db<'me, 'd>(self, db: &'me (#dyn_db + 'd)) -> salsa::QueryTable<'me, Self, (#dyn_db + 'd)> { salsa::plumbing::get_query_table::<#qt>(db) } @@ -419,6 +419,8 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream impl<'d> salsa::QueryDb<'d> for #qt { type DynDb = #dyn_db + 'd; + type Group = #group_struct; + type GroupStorage = #group_storage; } // ANCHOR:Query_impl @@ -427,16 +429,14 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream type Key = (#(#keys),*); type Value = #value; type Storage = #storage; - type Group = #group_struct; - type GroupStorage = #group_storage; const QUERY_INDEX: u16 = #query_index; const QUERY_NAME: &'static str = #query_name; - fn query_storage( - group_storage: &Self::GroupStorage, - ) -> &std::sync::Arc { + fn query_storage<'a>( + group_storage: &'a >::GroupStorage, + ) -> &'a std::sync::Arc { &group_storage.#fn_name } } @@ -539,7 +539,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream impl #group_storage { #trait_vis fn fmt_index( &self, - db: &#dyn_db, + db: &(#dyn_db + '_), input: salsa::DatabaseKeyIndex, fmt: &mut std::fmt::Formatter<'_>, ) -> std::fmt::Result { @@ -551,7 +551,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream #trait_vis fn maybe_changed_since( &self, - db: &#dyn_db, + db: &(#dyn_db + '_), input: salsa::DatabaseKeyIndex, revision: salsa::Revision, ) -> bool { diff --git a/examples/compiler/compiler.rs b/examples/compiler/compiler.rs index f0d5592a..319cc241 100644 --- a/examples/compiler/compiler.rs +++ b/examples/compiler/compiler.rs @@ -28,7 +28,7 @@ pub trait Compiler: Interner { /// dolor,sit,amet, /// consectetur,adipiscing,elit /// ``` -fn all_classes(db: &dyn Compiler) -> Arc> { +fn all_classes<'d>(db: &(dyn Compiler + 'd)) -> Arc> { let string = db.input_string(); let rows = string.split('\n'); @@ -53,13 +53,13 @@ fn all_classes(db: &dyn Compiler) -> Arc> { Arc::new(classes) } -fn fields(db: &dyn Compiler, class: Class) -> Arc> { +fn fields<'d>(db: &(dyn Compiler + 'd), class: Class) -> Arc> { let class = db.lookup_intern_class(class); let fields = class.fields.clone(); Arc::new(fields) } -fn all_fields(db: &dyn Compiler) -> Arc> { +fn all_fields<'d>(db: &(dyn Compiler + 'd)) -> Arc> { Arc::new( db.all_classes() .iter() diff --git a/src/debug.rs b/src/debug.rs index 8f067eeb..3db589fe 100644 --- a/src/debug.rs +++ b/src/debug.rs @@ -49,9 +49,10 @@ impl TableEntry { } } -impl DebugQueryTable for QueryTable<'_, Q, DB> +impl<'d, Q> DebugQueryTable for QueryTable<'_, Q, >::DynDb> where - Q: Query + for<'d> QueryDb<'d, DynDb = DB>, + Q: Query, + Q::Storage: QueryStorageOps, { type Key = Q::Key; type Value = Q::Value; diff --git a/src/derived.rs b/src/derived.rs index f8a34b7a..e1515fa3 100644 --- a/src/derived.rs +++ b/src/derived.rs @@ -48,7 +48,7 @@ where { } -pub trait MemoizationPolicy: Send + Sync + 'static +pub trait MemoizationPolicy: Send + Sync where Q: QueryFunction, { diff --git a/src/derived/slot.rs b/src/derived/slot.rs index bf490f21..0900ed79 100644 --- a/src/derived/slot.rs +++ b/src/derived/slot.rs @@ -1038,7 +1038,7 @@ where fn check_static() where Q: QueryFunction + 'static, - MP: MemoizationPolicy, + MP: MemoizationPolicy + 'static, Q::Key: 'static, Q::Value: 'static, { diff --git a/src/interned.rs b/src/interned.rs index 65c42ed4..61fc96fd 100644 --- a/src/interned.rs +++ b/src/interned.rs @@ -406,19 +406,46 @@ where } } -impl QueryStorageOps for LookupInternedStorage +// Workaround for +// ``` +// IQ: for<'d> QueryDb< +// 'd, +// DynDb = >::DynDb, +// Group = >::Group, +// GroupStorage = >::GroupStorage, +// >, +// ``` +// not working to make rustc know DynDb, Group and GroupStorage being the same in `Q` and `IQ` +#[doc(hidden)] +pub trait EqualDynDb<'d, IQ>: QueryDb<'d> where - Q: Query + for<'d> QueryDb<'d, DynDb = DB>, + IQ: QueryDb<'d>, +{ + fn convert_db(d: &Self::DynDb) -> &IQ::DynDb; + fn convert_group_storage(d: &Self::GroupStorage) -> &IQ::GroupStorage; +} + +impl<'d, IQ, Q> EqualDynDb<'d, IQ> for Q +where + Q: QueryDb<'d, DynDb = IQ::DynDb, Group = IQ::Group, GroupStorage = IQ::GroupStorage>, + Q::DynDb: HasQueryGroup, + IQ: QueryDb<'d>, +{ + fn convert_db(d: &Self::DynDb) -> &IQ::DynDb { + d + } + fn convert_group_storage(d: &Self::GroupStorage) -> &IQ::GroupStorage { + d + } +} + +impl QueryStorageOps for LookupInternedStorage +where + Q: Query, Q::Key: InternKey, Q::Value: Eq + Hash, - IQ: Query< - Key = Q::Value, - Value = Q::Key, - Storage = InternedStorage, - Group = Q::Group, - GroupStorage = Q::GroupStorage, - > + for<'d> QueryDb<'d, DynDb = DB>, - DB: ?Sized + Database + HasQueryGroup, + IQ: Query>, + for<'d> Q: EqualDynDb<'d, IQ>, { fn new(_group_index: u16) -> Self { LookupInternedStorage { @@ -428,29 +455,38 @@ where fn fmt_index( &self, - db: &DB, + db: &>::DynDb, index: DatabaseKeyIndex, fmt: &mut std::fmt::Formatter<'_>, ) -> std::fmt::Result { let group_storage = <>::DynDb as HasQueryGroup>::group_storage(db); - let interned_storage = IQ::query_storage(group_storage); - interned_storage.fmt_index(db, index, fmt) + let interned_storage = IQ::query_storage(Q::convert_group_storage(group_storage)); + interned_storage.fmt_index(Q::convert_db(db), index, fmt) } - fn maybe_changed_since(&self, db: &DB, input: DatabaseKeyIndex, revision: Revision) -> bool { + fn maybe_changed_since( + &self, + db: &>::DynDb, + input: DatabaseKeyIndex, + revision: Revision, + ) -> bool { let group_storage = <>::DynDb as HasQueryGroup>::group_storage(db); - let interned_storage = IQ::query_storage(group_storage); - interned_storage.maybe_changed_since(db, input, revision) + let interned_storage = IQ::query_storage(Q::convert_group_storage(group_storage)); + interned_storage.maybe_changed_since(Q::convert_db(db), input, revision) } - fn try_fetch(&self, db: &DB, key: &Q::Key) -> Result> { + fn try_fetch( + &self, + db: &>::DynDb, + key: &Q::Key, + ) -> Result> { let index = key.as_intern_id(); let group_storage = <>::DynDb as HasQueryGroup>::group_storage(db); - let interned_storage = IQ::query_storage(group_storage); - let slot = interned_storage.lookup_value(db, index); + let interned_storage = IQ::query_storage(Q::convert_group_storage(group_storage)); + let slot = interned_storage.lookup_value(Q::convert_db(db), index); let value = slot.value.clone(); let interned_at = slot.interned_at; db.salsa_runtime().report_query_read( @@ -461,17 +497,17 @@ where Ok(value) } - fn durability(&self, _db: &DB, _key: &Q::Key) -> Durability { + fn durability(&self, _db: &>::DynDb, _key: &Q::Key) -> Durability { INTERN_DURABILITY } - fn entries(&self, db: &DB) -> C + fn entries(&self, db: &>::DynDb) -> C where C: std::iter::FromIterator>, { let group_storage = <>::DynDb as HasQueryGroup>::group_storage(db); - let interned_storage = IQ::query_storage(group_storage); + let interned_storage = IQ::query_storage(Q::convert_group_storage(group_storage)); let tables = interned_storage.tables.read(); tables .map @@ -488,7 +524,7 @@ where Q: Query, Q::Key: InternKey, Q::Value: Eq + Hash, - IQ: Query, + IQ: Query, { fn sweep(&self, _: &Runtime, _strategy: SweepStrategy) {} } diff --git a/src/lib.rs b/src/lib.rs index 1c255a54..a5743417 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -418,8 +418,19 @@ where } } +/// Trait implements by all of the "special types" associated with +/// each of your queries. +/// +/// Base trait of `Query` that has a lifetime parameter to allow the `DynDb` to be non-'static. pub trait QueryDb<'d>: Sized { - type DynDb: ?Sized + Database + 'd; + /// Dyn version of the associated trait for this query group. + type DynDb: ?Sized + Database + HasQueryGroup + 'd; + + /// Associate query group struct. + type Group: plumbing::QueryGroup; + + /// Generated struct that contains storage for all queries in a group. + type GroupStorage; } /// Trait implements by all of the "special types" associated with @@ -433,13 +444,8 @@ pub trait Query: Debug + Default + Sized + for<'d> QueryDb<'d> { type Value: Clone + Debug; /// Internal struct storing the values for the query. - type Storage: plumbing::QueryStorageOps; - - /// Associate query group struct. - type Group: plumbing::QueryGroup; - - /// Generated struct that contains storage for all queries in a group. - type GroupStorage; + // type Storage: plumbing::QueryStorageOps; + type Storage; /// A unique index identifying this query within the group. const QUERY_INDEX: u16; @@ -448,7 +454,9 @@ pub trait Query: Debug + Default + Sized + for<'d> QueryDb<'d> { const QUERY_NAME: &'static str; /// Extact storage for this query from the storage for its group. - fn query_storage(group_storage: &Self::GroupStorage) -> &Arc; + fn query_storage<'a>( + group_storage: &'a >::GroupStorage, + ) -> &'a Arc; } /// Return value from [the `query` method] on `Database`. @@ -467,6 +475,7 @@ where impl<'me, 'd, Q> QueryTable<'me, Q, >::DynDb> where Q: Query, + Q::Storage: QueryStorageOps, { /// Constructs a new `QueryTable`. pub fn new(db: &'me >::DynDb, storage: &'me Q::Storage) -> Self { @@ -504,7 +513,7 @@ pub struct QueryTableMut<'me, Q> where Q: Query + 'me, { - db: &'me mut >::DynDb, + db: &'me mut >::DynDb, storage: Arc, } @@ -513,7 +522,7 @@ where Q: Query, { /// Constructs a new `QueryTableMut`. - pub fn new(db: &'me mut >::DynDb, storage: Arc) -> Self { + pub fn new(db: &'me mut >::DynDb, storage: Arc) -> Self { Self { db, storage } } diff --git a/src/plumbing.rs b/src/plumbing.rs index 47d51ed6..e5d4e590 100644 --- a/src/plumbing.rs +++ b/src/plumbing.rs @@ -85,10 +85,13 @@ pub trait QueryFunction: Query { /// Create a query table, which has access to the storage for the query /// and offers methods like `get`. -pub fn get_query_table<'me, Q, DB>(db: &'me DB) -> QueryTable<'me, Q, DB> +pub fn get_query_table<'d, 'me, Q>( + db: &'me >::DynDb, +) -> QueryTable<'me, Q, >::DynDb> where - Q: Query + for<'d> QueryDb<'d, DynDb = DB> + 'me, - DB: HasQueryGroup, + 'd: 'me, + Q: Query + 'me, + Q::Storage: QueryStorageOps, { let group_storage: &Q::GroupStorage = HasQueryGroup::group_storage(db); let query_storage: &Q::Storage = Q::query_storage(group_storage); @@ -97,12 +100,9 @@ where /// Create a mutable query table, which has access to the storage /// for the query and offers methods like `set`. -pub fn get_query_table_mut<'me, Q>( - db: &'me mut >::DynDb, -) -> QueryTableMut<'me, Q> +pub fn get_query_table_mut<'me, Q>(db: &'me mut >::DynDb) -> QueryTableMut<'me, Q> where Q: Query, - >::DynDb: HasQueryGroup, { let group_storage: &Q::GroupStorage = HasQueryGroup::group_storage(db); let query_storage = Q::query_storage(group_storage).clone(); From 3a84a77ebc692c3e93b22b90d434c95c698d1d8a Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Fri, 24 Jul 2020 11:23:41 +0200 Subject: [PATCH 3/3] Remove some unnecessary changes --- components/salsa-macros/src/query_group.rs | 2 +- src/debug.rs | 4 ++-- src/lib.rs | 9 ++++----- src/plumbing.rs | 5 +---- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/components/salsa-macros/src/query_group.rs b/components/salsa-macros/src/query_group.rs index 8ca133c7..511ada91 100644 --- a/components/salsa-macros/src/query_group.rs +++ b/components/salsa-macros/src/query_group.rs @@ -373,7 +373,7 @@ pub(crate) fn query_group(args: TokenStream, input: TokenStream) -> TokenStream /// single input. You can also use it to invoke this query, though /// it's more common to use the trait method on the database /// itself. - #trait_vis fn in_db<'me, 'd>(self, db: &'me (#dyn_db + 'd)) -> salsa::QueryTable<'me, Self, (#dyn_db + 'd)> + #trait_vis fn in_db(self, db: &#dyn_db) -> salsa::QueryTable<'_, Self> { salsa::plumbing::get_query_table::<#qt>(db) } diff --git a/src/debug.rs b/src/debug.rs index 3db589fe..f5f06973 100644 --- a/src/debug.rs +++ b/src/debug.rs @@ -4,7 +4,7 @@ use crate::durability::Durability; use crate::plumbing::QueryStorageOps; use crate::Query; -use crate::{QueryDb, QueryTable}; +use crate::QueryTable; use std::iter::FromIterator; /// Additional methods on queries that can be used to "peek into" @@ -49,7 +49,7 @@ impl TableEntry { } } -impl<'d, Q> DebugQueryTable for QueryTable<'_, Q, >::DynDb> +impl<'d, Q> DebugQueryTable for QueryTable<'_, Q> where Q: Query, Q::Storage: QueryStorageOps, diff --git a/src/lib.rs b/src/lib.rs index a5743417..4f296f5e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -463,22 +463,21 @@ pub trait Query: Debug + Default + Sized + for<'d> QueryDb<'d> { /// Gives access to various less common operations on queries. /// /// [the `query` method]: trait.Database.html#method.query -pub struct QueryTable<'me, Q, DB> +pub struct QueryTable<'me, Q> where Q: Query, - DB: ?Sized, { - db: &'me DB, + db: &'me >::DynDb, storage: &'me Q::Storage, } -impl<'me, 'd, Q> QueryTable<'me, Q, >::DynDb> +impl<'me, Q> QueryTable<'me, Q> where Q: Query, Q::Storage: QueryStorageOps, { /// Constructs a new `QueryTable`. - pub fn new(db: &'me >::DynDb, storage: &'me Q::Storage) -> Self { + pub fn new(db: &'me >::DynDb, storage: &'me Q::Storage) -> Self { Self { db, storage } } diff --git a/src/plumbing.rs b/src/plumbing.rs index e5d4e590..b82ceacb 100644 --- a/src/plumbing.rs +++ b/src/plumbing.rs @@ -85,11 +85,8 @@ pub trait QueryFunction: Query { /// Create a query table, which has access to the storage for the query /// and offers methods like `get`. -pub fn get_query_table<'d, 'me, Q>( - db: &'me >::DynDb, -) -> QueryTable<'me, Q, >::DynDb> +pub fn get_query_table<'me, Q>(db: &'me >::DynDb) -> QueryTable<'me, Q> where - 'd: 'me, Q: Query + 'me, Q::Storage: QueryStorageOps, {