From dde7341f973d72a37686d9a4c536494c26a12b24 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 12 Jul 2024 08:21:23 -0400 Subject: [PATCH] create macro-rules macros to encapsulate output This is a new idea for how to manage procedural macros, which I kind of hate. --- Cargo.toml | 3 +- components/salsa-macro-rules/Cargo.toml | 6 + components/salsa-macro-rules/src/lib.rs | 200 ++++++++++++++++++++++++ src/function.rs | 10 +- src/function/execute.rs | 2 +- src/function/interned.rs | 88 ----------- src/lib.rs | 3 + tests/hello_world_tracked_fn.rs | 138 +++------------- 8 files changed, 244 insertions(+), 206 deletions(-) create mode 100644 components/salsa-macro-rules/Cargo.toml create mode 100644 components/salsa-macro-rules/src/lib.rs delete mode 100644 src/function/interned.rs diff --git a/Cargo.toml b/Cargo.toml index deb4f449..f006e438 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ log = "0.4.5" orx-concurrent-vec = "1.10.0" parking_lot = "0.12.1" rustc-hash = "1.1.0" +salsa-macro-rules = { version = "0.1.0", path = "components/salsa-macro-rules" } salsa-macros = { path = "components/salsa-macros" } smallvec = "1.0.0" @@ -32,4 +33,4 @@ test-log = "0.2.11" trybuild = "1.0" [workspace] -members = ["components/salsa-macros"] +members = [ "components/salsa-macro-rules","components/salsa-macros"] diff --git a/components/salsa-macro-rules/Cargo.toml b/components/salsa-macro-rules/Cargo.toml new file mode 100644 index 00000000..12829c4d --- /dev/null +++ b/components/salsa-macro-rules/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "salsa-macro-rules" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/components/salsa-macro-rules/src/lib.rs b/components/salsa-macro-rules/src/lib.rs new file mode 100644 index 00000000..b7a97dbb --- /dev/null +++ b/components/salsa-macro-rules/src/lib.rs @@ -0,0 +1,200 @@ +//! This crate defines various `macro_rules` macros +//! used as part of Salsa's internal plumbing. +//! +//! The procedural macros typically emit calls to these +//! `macro_rules` macros. +//! +//! Modifying `macro_rules` macro definitions is generally +//! more ergonomic and also permits true hygiene. + +// Macro that generates the body of the cycle recovery function +// for the case where no cycle recovery is possible. This has to be +// a macro because it can take a variadic number of arguments. +#[macro_export] +macro_rules! unexpected_cycle_recovery { + ($db:ident, $cycle:ident, $($other_inputs:ident),*) => { + { + std::mem::drop($db); + std::mem::drop(($($other_inputs),*)); + panic!("cannot recover from cycle `{:?}`", $cycle) + } + } +} + +/// Macro for setting up a function that must intern its arguments. +#[macro_export] +macro_rules! setup_interned_fn { + ( + // Visibility of the function + vis: $vis:vis, + + // Name of the function + fn_name: $fn_name:ident, + + // Name of the `'db` lifetime that the user gave; if they didn't, then defaults to `'db` + db_lt: $db_lt:lifetime, + + // Path to the database trait that the user's database parameter used + Db: $Db:path, + + // Name of the database parameter given by the user. + db: $db:ident, + + // An identifier for each function argument EXCEPT the database. + // We prefer to use the identifier the user gave, but if the user gave a pattern + // (e.g., `(a, b): (u32, u32)`) we will synthesize an identifier. + input_ids: [$($input_id:ident),*], + + // Patterns that the user gave for each argument EXCEPT the database. + // May be identifiers, but could be something else. + input_pats: [$($input_pat:pat),*], + + // Types of the function arguments (may reference `$generics`). + input_tys: [$($input_ty:ty),*], + + // Return type of the function (may reference `$generics`). + output_ty: $output_ty:ty, + + // Function body, may reference identifiers defined in `$input_pats` and the generics from `$generics` + body: $body:block, + + // Path to the cycle recovery function to use. + cycle_recovery_fn: ($($cycle_recovery_fn:tt)*), + + // Name of cycle recovery strategy variant to use. + cycle_recovery_strategy: $cycle_recovery_strategy:ident, + + // 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, + $Configuration:ident, + $InternedData:ident, + $FN_CACHE:ident, + $INTERN_CACHE:ident, + $inner:ident, + ] + ) => { + $vis fn $fn_name<$db_lt>( + $db: &$db_lt dyn $Db, + $($input_id: $input_ty,)* + ) -> $output_ty { + use salsa::plumbing as $zalsa; + + struct $Configuration; + + #[derive(Copy, Clone)] + struct $InternedData<'db>( + std::ptr::NonNull<$zalsa::interned::ValueStruct<$Configuration>>, + std::marker::PhantomData<&'db $zalsa::interned::ValueStruct<$Configuration>>, + ); + + static $FN_CACHE: $zalsa::IngredientCache<$zalsa::function::IngredientImpl<$Configuration>> = + $zalsa::IngredientCache::new(); + + static $INTERN_CACHE: $zalsa::IngredientCache<$zalsa::interned::IngredientImpl<$Configuration>> = + $zalsa::IngredientCache::new(); + + impl $zalsa::SalsaStructInDb for $InternedData<'_> { + fn register_dependent_fn(_db: &dyn $Db, _index: $zalsa::IngredientIndex) {} + } + + impl $zalsa::function::Configuration for $Configuration { + const DEBUG_NAME: &'static str = stringify!($fn_name); + + type DbView = dyn $Db; + + type SalsaStruct<$db_lt> = $InternedData<$db_lt>; + + type Input<$db_lt> = ($($input_ty),*); + + type Output<$db_lt> = $output_ty; + + const CYCLE_STRATEGY: $zalsa::CycleRecoveryStrategy = $zalsa::CycleRecoveryStrategy::$cycle_recovery_strategy; + + fn should_backdate_value( + old_value: &Self::Output<'_>, + new_value: &Self::Output<'_>, + ) -> bool { + old_value == new_value + } + + fn execute<'db>($db: &'db Self::DbView, ($($input_id),*): ($($input_ty),*)) -> Self::Output<'db> { + $vis fn $inner<$db_lt>( + $db: &$db_lt dyn $Db, + $($input_pat: $input_ty,)* + ) -> $output_ty { + $body + } + + $inner($db, $($input_id),*) + } + + fn recover_from_cycle<'db>( + db: &$db_lt dyn $Db, + cycle: &$zalsa::Cycle, + ($($input_id),*): ($($input_ty),*) + ) -> Self::Output<'db> { + $($cycle_recovery_fn)*(db, cycle, $($input_id),*) + } + + fn id_to_input<'db>(db: &'db Self::DbView, key: salsa::Id) -> Self::Input<'db> { + let ingredient = $INTERN_CACHE.get_or_create(db.as_salsa_database(), || { + db.add_or_lookup_jar_by_type(&$Configuration) + 1 + }); + ingredient.data(key).clone() + } + } + + impl $zalsa::interned::Configuration for $Configuration { + const DEBUG_NAME: &'static str = "Configuration"; + + type Data<$db_lt> = ($($input_ty),*); + + type Struct<$db_lt> = $InternedData<$db_lt>; + + unsafe fn struct_from_raw<'db>( + ptr: std::ptr::NonNull<$zalsa::interned::ValueStruct>, + ) -> Self::Struct<'db> { + $InternedData(ptr, std::marker::PhantomData) + } + + fn deref_struct(s: Self::Struct<'_>) -> &$zalsa::interned::ValueStruct { + unsafe { s.0.as_ref() } + } + } + + impl $zalsa::Jar for $Configuration { + fn create_ingredients( + &self, + first_index: $zalsa::IngredientIndex, + ) -> Vec> { + vec![ + Box::new(<$zalsa::function::IngredientImpl<$Configuration>>::new( + first_index, + )), + Box::new(<$zalsa::interned::IngredientImpl<$Configuration>>::new( + first_index + 1, + )), + ] + } + } + + let intern_ingredient = $INTERN_CACHE.get_or_create($db.as_salsa_database(), || { + $db.add_or_lookup_jar_by_type(&$Configuration) + 1 + }); + let key = intern_ingredient.intern_id($db.runtime(), ($($input_id),*)); + + let fn_ingredient = $FN_CACHE.get_or_create($db.as_salsa_database(), || { + $db.add_or_lookup_jar_by_type(&$Configuration) + }); + fn_ingredient.fetch($db, key).clone() + } + }; +} + +#[macro_export] +macro_rules! setup_fn { + () => {}; +} diff --git a/src/function.rs b/src/function.rs index 85f6928d..58066612 100644 --- a/src/function.rs +++ b/src/function.rs @@ -5,7 +5,7 @@ use crossbeam::atomic::AtomicCell; use crate::{ cycle::CycleRecoveryStrategy, ingredient::{fmt_index, IngredientRequiresReset}, - key::{DatabaseKeyIndex, DependencyIndex}, + key::DatabaseKeyIndex, runtime::local_state::QueryOrigin, salsa_struct::SalsaStructInDb, storage::IngredientIndex, @@ -30,8 +30,6 @@ mod specify; mod store; mod sync; -pub mod interned; - pub trait Configuration: Any { const DEBUG_NAME: &'static str; @@ -75,7 +73,11 @@ pub trait Configuration: Any { /// in a cycle to find out what value it should have. /// /// This invokes the recovery function given by the user. - fn recover_from_cycle<'db>(db: &'db Self::DbView, cycle: &Cycle, key: Id) -> Self::Output<'db>; + fn recover_from_cycle<'db>( + db: &'db Self::DbView, + cycle: &Cycle, + input: Self::Input<'db>, + ) -> Self::Output<'db>; } /// Function ingredients are the "workhorse" of salsa. diff --git a/src/function/execute.rs b/src/function/execute.rs index 79fdb20e..9374ee85 100644 --- a/src/function/execute.rs +++ b/src/function/execute.rs @@ -56,7 +56,7 @@ where crate::cycle::CycleRecoveryStrategy::Fallback => { if let Some(c) = active_query.take_cycle() { assert!(c.is(&cycle)); - C::recover_from_cycle(db, &cycle, key) + C::recover_from_cycle(db, &cycle, C::id_to_input(db, key)) } else { // we are not a participant in this cycle debug_assert!(!cycle diff --git a/src/function/interned.rs b/src/function/interned.rs deleted file mode 100644 index 44547984..00000000 --- a/src/function/interned.rs +++ /dev/null @@ -1,88 +0,0 @@ -//! Helper code for tracked functions that take arbitrary arguments. -//! These arguments must be interned to create a salsa id before the -//! salsa machinery can execute. - -use std::{any::Any, fmt, hash::Hash, marker::PhantomData}; - -use crate::{ - function, interned, plumbing::CycleRecoveryStrategy, salsa_struct::SalsaStructInDb, Cycle, Id, -}; - -pub trait Configuration: Any + Copy { - const DEBUG_NAME: &'static str; - type DbView: ?Sized + crate::Database; - type SalsaStruct<'db>: SalsaStructInDb; - type Input<'db>: Send + Sync + Clone + Hash + Eq; - type Output<'db>: fmt::Debug + Send + Sync; - const CYCLE_STRATEGY: CycleRecoveryStrategy; - fn should_backdate_value(old_value: &Self::Output<'_>, new_value: &Self::Output<'_>) -> bool; - fn id_to_input<'db>(db: &'db Self::DbView, key: Id) -> Self::Input<'db>; - fn execute<'db>(db: &'db Self::DbView, input: Self::Input<'db>) -> Self::Output<'db>; - fn recover_from_cycle<'db>(db: &'db Self::DbView, cycle: &Cycle, key: Id) -> Self::Output<'db>; -} - -pub struct InterningConfiguration { - phantom: PhantomData, -} - -#[derive(Copy, Clone)] -pub struct InternedData<'db, C: Configuration>( - std::ptr::NonNull>, - std::marker::PhantomData<&'db interned::ValueStruct>, -); - -impl SalsaStructInDb for InternedData<'_, C> { - fn register_dependent_fn(_db: &C::DbView, _index: crate::storage::IngredientIndex) {} -} - -impl interned::Configuration for C { - const DEBUG_NAME: &'static str = C::DEBUG_NAME; - - type Data<'db> = C::Input<'db>; - - type Struct<'db> = InternedData<'db, C>; - - unsafe fn struct_from_raw<'db>( - ptr: std::ptr::NonNull>, - ) -> Self::Struct<'db> { - InternedData(ptr, std::marker::PhantomData) - } - - fn deref_struct(s: Self::Struct<'_>) -> &interned::ValueStruct { - unsafe { s.0.as_ref() } - } -} - -impl function::Configuration for C { - const DEBUG_NAME: &'static str = C::DEBUG_NAME; - - type DbView = C::DbView; - - type SalsaStruct<'db> = InternedData<'db, C>; - - type Input<'db> = C::Input<'db>; - - type Output<'db> = C::Output<'db>; - - const CYCLE_STRATEGY: crate::plumbing::CycleRecoveryStrategy = C::CYCLE_STRATEGY; - - fn should_backdate_value(old_value: &Self::Output<'_>, new_value: &Self::Output<'_>) -> bool { - C::should_backdate_value(old_value, new_value) - } - - fn id_to_input<'db>(db: &'db Self::DbView, key: crate::Id) -> Self::Input<'db> { - todo!() - } - - fn execute<'db>(db: &'db Self::DbView, input: Self::Input<'db>) -> Self::Output<'db> { - todo!() - } - - fn recover_from_cycle<'db>( - db: &'db Self::DbView, - cycle: &crate::Cycle, - key: crate::Id, - ) -> Self::Output<'db> { - todo!() - } -} diff --git a/src/lib.rs b/src/lib.rs index 9ec677f9..af8358a7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -66,6 +66,9 @@ pub mod plumbing { pub use crate::storage::IngredientIndex; pub use crate::storage::Storage; + pub use salsa_macro_rules::setup_interned_fn; + pub use salsa_macro_rules::unexpected_cycle_recovery; + pub mod input { pub use crate::input::Configuration; pub use crate::input::IngredientImpl; diff --git a/tests/hello_world_tracked_fn.rs b/tests/hello_world_tracked_fn.rs index 4c972e87..24bb4ddf 100644 --- a/tests/hello_world_tracked_fn.rs +++ b/tests/hello_world_tracked_fn.rs @@ -30,119 +30,33 @@ impl HasLogger for Database { } // #[salsa::tracked] -// fn identity(db: &dyn Db, input: u32) -> u32 { +// fn final_result(db: &dyn Db, input: i32, (b, c): (i32, i32)) -> i32 { // db.push_log(format!("final_result({:?})", input)); // input // } -fn identity(db: &dyn Db, input: u32) -> u32 { - use salsa::plumbing as zalsa; - - struct Configuration; - - #[derive(Copy, Clone)] - struct InternedData<'db>( - std::ptr::NonNull>, - std::marker::PhantomData<&'db zalsa::interned::ValueStruct>, - ); - - static FN_CACHE: zalsa::IngredientCache> = - zalsa::IngredientCache::new(); - - static INTERN_CACHE: zalsa::IngredientCache> = - zalsa::IngredientCache::new(); - - impl zalsa::SalsaStructInDb for InternedData<'_> { - fn register_dependent_fn(_db: &dyn Db, _index: zalsa::IngredientIndex) {} - } - - impl zalsa::function::Configuration for Configuration { - const DEBUG_NAME: &'static str = "identity"; - - type DbView = dyn Db; - - type SalsaStruct<'db> = InternedData<'db>; - - type Input<'db> = u32; - - type Output<'db> = u32; - - const CYCLE_STRATEGY: zalsa::CycleRecoveryStrategy = zalsa::CycleRecoveryStrategy::Panic; - - fn should_backdate_value( - old_value: &Self::Output<'_>, - new_value: &Self::Output<'_>, - ) -> bool { - old_value == new_value - } - - fn execute<'db>(db: &'db Self::DbView, input: u32) -> Self::Output<'db> { - fn inner(db: &dyn Db, input: u32) -> u32 { - db.push_log(format!("final_result({:?})", input)); - input - } - - inner(db, input) - } - - fn recover_from_cycle<'db>( - _db: &'db Self::DbView, - _cycle: &zalsa::Cycle, - _key: zalsa::Id, - ) -> Self::Output<'db> { - panic!() - } - - fn id_to_input<'db>(db: &'db Self::DbView, key: salsa::Id) -> Self::Input<'db> { - let ingredient = INTERN_CACHE.get_or_create(db.as_salsa_database(), || { - db.add_or_lookup_jar_by_type(&Configuration) + 1 - }); - ingredient.data(key).clone() - } - } - - impl zalsa::interned::Configuration for Configuration { - const DEBUG_NAME: &'static str = "Configuration"; - - type Data<'db> = u32; - - type Struct<'db> = InternedData<'db>; - - unsafe fn struct_from_raw<'db>( - ptr: std::ptr::NonNull>, - ) -> Self::Struct<'db> { - InternedData(ptr, std::marker::PhantomData) - } - - fn deref_struct(s: Self::Struct<'_>) -> &zalsa::interned::ValueStruct { - unsafe { s.0.as_ref() } - } - } - - impl zalsa::Jar for Configuration { - fn create_ingredients( - &self, - first_index: zalsa::IngredientIndex, - ) -> Vec> { - vec![ - Box::new(>::new( - first_index, - )), - Box::new(>::new( - first_index + 1, - )), - ] - } - } - - let fn_ingredient = FN_CACHE.get_or_create(db.as_salsa_database(), || { - db.add_or_lookup_jar_by_type(&Configuration) - }); - let intern_ingredient = INTERN_CACHE.get_or_create(db.as_salsa_database(), || { - db.add_or_lookup_jar_by_type(&Configuration) + 1 - }); - - let key = intern_ingredient.intern_id(db.runtime(), input); - - fn_ingredient.fetch(db, key).clone() -} +salsa::plumbing::setup_interned_fn!( + vis: , + fn_name: identity, + db_lt: 'db, + Db: Db, + db: dbx, + input_ids: [input1, input2], + input_pats: [a, (b, c)], + input_tys: [i32, (i32, i32)], + output_ty: i32, + body: { + dbx.push_log(format!("final_result({a}, {b}, {c})")); + a + b * c + }, + cycle_recovery_fn: (salsa::plumbing::unexpected_cycle_recovery!), + cycle_recovery_strategy: Panic, + unused_names: [ + zalsa1, + Configuration1, + InternedData1, + FN_CACHE1, + INTERN_CACHE1, + inner1, + ] +);