[RFC] fixpoint iteration support

components/salsa-macro-rules/src/setup_tracked_fn.rs

@@ -37,6 +37,9 @@ macro_rules! setup_tracked_fn {
         // Path to the cycle recovery function to use.
         cycle_recovery_fn: ($($cycle_recovery_fn:tt)*),
 
+        // Path to function to get the initial value to use for cycle recovery.
+        cycle_recovery_initial: ($($cycle_recovery_initial:tt)*),
+
         // Name of cycle recovery strategy variant to use.
         cycle_recovery_strategy: $cycle_recovery_strategy:ident,
 
@@ -160,15 +163,15 @@ macro_rules! setup_tracked_fn {
 
                 const CYCLE_STRATEGY: $zalsa::CycleRecoveryStrategy = $zalsa::CycleRecoveryStrategy::$cycle_recovery_strategy;
 
-                fn should_backdate_value(
+                fn values_equal(
                     old_value: &Self::Output<'_>,
                     new_value: &Self::Output<'_>,
                 ) -> bool {
                     $zalsa::macro_if! {
                         if $no_eq {
                             false
                         } else {
-                            $zalsa::should_backdate_value(old_value, new_value)
+                            $zalsa::values_equal(old_value, new_value)
                         }
                     }
                 }
@@ -179,12 +182,16 @@ macro_rules! setup_tracked_fn {
                     $inner($db, $($input_id),*)
                 }
 
+                fn cycle_initial<$db_lt>(db: &$db_lt dyn $Db) -> Self::Output<$db_lt> {
+                    $($cycle_recovery_initial)*(db)
+                }
+
                 fn recover_from_cycle<$db_lt>(
                     db: &$db_lt dyn $Db,
-                    cycle: &$zalsa::Cycle,
-                    ($($input_id),*): ($($input_ty),*)
-                ) -> Self::Output<$db_lt> {
-                    $($cycle_recovery_fn)*(db, cycle, $($input_id),*)
+                    value: &Self::Output<$db_lt>,
+                    count: u32,
+                ) -> $zalsa::CycleRecoveryAction<Self::Output<$db_lt>> {
+                    $($cycle_recovery_fn)*(db, value, count)
                 }
 
                 fn id_to_input<$db_lt>(db: &$db_lt Self::DbView, key: salsa::Id) -> Self::Input<$db_lt> {

components/salsa-macro-rules/src/unexpected_cycle_recovery.rs

@@ -1,13 +1,18 @@
 // 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.
+// for the case where no cycle recovery is possible. Must be a macro
+// because the signature types must match the particular tracked function.
 #[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)
-        }
-    }
+    ($db:ident, $value:ident, $count:ident) => {{
+        std::mem::drop($db);
+        panic!("cannot recover from cycle")
+    }};
+}
+
+#[macro_export]
+macro_rules! unexpected_cycle_initial {
+    ($db:ident) => {{
+        std::mem::drop($db);
+        panic!("no cycle initial value")
+    }};
 }

components/salsa-macros/src/accumulator.rs

@@ -38,7 +38,8 @@ impl AllowedOptions for Accumulator {
     const SINGLETON: bool = false;
     const DATA: bool = false;
     const DB: bool = false;
-    const RECOVERY_FN: bool = false;
+    const CYCLE_FN: bool = false;
+    const CYCLE_INITIAL: bool = false;
     const LRU: bool = false;
     const CONSTRUCTOR_NAME: bool = false;
 }

components/salsa-macros/src/input.rs

@@ -49,7 +49,9 @@ impl crate::options::AllowedOptions for InputStruct {
 
     const DB: bool = false;
 
-    const RECOVERY_FN: bool = false;
+    const CYCLE_FN: bool = false;
+
+    const CYCLE_INITIAL: bool = false;
 
     const LRU: bool = false;
 

components/salsa-macros/src/interned.rs

@@ -50,7 +50,9 @@ impl crate::options::AllowedOptions for InternedStruct {
 
     const DB: bool = false;
 
-    const RECOVERY_FN: bool = false;
+    const CYCLE_FN: bool = false;
+
+    const CYCLE_INITIAL: bool = false;
 
     const LRU: bool = false;
 

components/salsa-macros/src/options.rs

@@ -44,10 +44,15 @@ pub(crate) struct Options<A: AllowedOptions> {
     /// If this is `Some`, the value is the `<path>`.
     pub db_path: Option<syn::Path>,
 
-    /// The `recovery_fn = <path>` option is used to indicate the recovery function.
+    /// The `cycle_fn = <path>` option is used to indicate the cycle recovery function.
     ///
     /// If this is `Some`, the value is the `<path>`.
-    pub recovery_fn: Option<syn::Path>,
+    pub cycle_fn: Option<syn::Path>,
+
+    /// The `cycle_initial = <path>` option is the initial value for cycle iteration.
+    ///
+    /// If this is `Some`, the value is the `<path>`.
+    pub cycle_initial: Option<syn::Path>,
 
     /// The `data = <ident>` option is used to define the name of the data type for an interned
     /// struct.
@@ -79,7 +84,8 @@ impl<A: AllowedOptions> Default for Options<A> {
             no_debug: Default::default(),
             no_clone: Default::default(),
             db_path: Default::default(),
-            recovery_fn: Default::default(),
+            cycle_fn: Default::default(),
+            cycle_initial: Default::default(),
             data: Default::default(),
             constructor_name: Default::default(),
             phantom: Default::default(),
@@ -99,7 +105,8 @@ pub(crate) trait AllowedOptions {
     const SINGLETON: bool;
     const DATA: bool;
     const DB: bool;
-    const RECOVERY_FN: bool;
+    const CYCLE_FN: bool;
+    const CYCLE_INITIAL: bool;
     const LRU: bool;
     const CONSTRUCTOR_NAME: bool;
 }
@@ -207,20 +214,39 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
                         "`db` option not allowed here",
                     ));
                 }
-            } else if ident == "recovery_fn" {
-                if A::RECOVERY_FN {
+            } else if ident == "cycle_fn" {
+                if A::CYCLE_FN {
+                    let _eq = Equals::parse(input)?;
+                    let path = syn::Path::parse(input)?;
+                    if let Some(old) = std::mem::replace(&mut options.cycle_fn, Some(path)) {
+                        return Err(syn::Error::new(
+                            old.span(),
+                            "option `cycle_fn` provided twice",
+                        ));
+                    }
+                } else {
+                    return Err(syn::Error::new(
+                        ident.span(),
+                        "`cycle_fn` option not allowed here",
+                    ));
+                }
+            } else if ident == "cycle_initial" {
+                if A::CYCLE_INITIAL {
+                    // TODO(carljm) should it be an error to give cycle_initial without cycle_fn,
+                    // or should we just allow this to fall into potentially infinite iteration, if
+                    // iteration never converges?
                     let _eq = Equals::parse(input)?;
                     let path = syn::Path::parse(input)?;
-                    if let Some(old) = std::mem::replace(&mut options.recovery_fn, Some(path)) {
+                    if let Some(old) = std::mem::replace(&mut options.cycle_initial, Some(path)) {
                         return Err(syn::Error::new(
                             old.span(),
-                            "option `recovery_fn` provided twice",
+                            "option `cycle_initial` provided twice",
                         ));
                     }
                 } else {
                     return Err(syn::Error::new(
                         ident.span(),
-                        "`recovery_fn` option not allowed here",
+                        "`cycle_initial` option not allowed here",
                     ));
                 }
             } else if ident == "data" {

components/salsa-macros/src/tracked_fn.rs

@@ -39,7 +39,9 @@ impl crate::options::AllowedOptions for TrackedFn {
 
     const DB: bool = false;
 
-    const RECOVERY_FN: bool = true;
+    const CYCLE_FN: bool = true;
+
+    const CYCLE_INITIAL: bool = true;
 
     const LRU: bool = true;
 
@@ -68,9 +70,20 @@ impl Macro {
         let input_ids = self.input_ids(&item);
         let input_tys = self.input_tys(&item)?;
         let output_ty = self.output_ty(&db_lt, &item)?;
-        let (cycle_recovery_fn, cycle_recovery_strategy) = self.cycle_recovery();
+        let (cycle_recovery_fn, cycle_recovery_initial, cycle_recovery_strategy) =
+            self.cycle_recovery()?;
         let is_specifiable = self.args.specify.is_some();
-        let no_eq = self.args.no_eq.is_some();
+        let no_eq = if let Some(token) = &self.args.no_eq {
+            if self.args.cycle_fn.is_some() {
+                return Err(syn::Error::new_spanned(
+                    token,
+                    "the `no_eq` option cannot be used with `cycle_fn`",
+                ));
+            }
+            true
+        } else {
+            false
+        };
 
         let mut inner_fn = item.clone();
         inner_fn.vis = syn::Visibility::Inherited;
@@ -127,6 +140,7 @@ impl Macro {
                 output_ty: #output_ty,
                 inner_fn: #inner_fn,
                 cycle_recovery_fn: #cycle_recovery_fn,
+                cycle_recovery_initial: #cycle_recovery_initial,
                 cycle_recovery_strategy: #cycle_recovery_strategy,
                 is_specifiable: #is_specifiable,
                 no_eq: #no_eq,
@@ -160,14 +174,26 @@ 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))
-        } else {
-            (
+    fn cycle_recovery(&self) -> syn::Result<(TokenStream, TokenStream, TokenStream)> {
+        match (&self.args.cycle_fn, &self.args.cycle_initial) {
+            (Some(cycle_fn), Some(cycle_initial)) => Ok((
+                quote!((#cycle_fn)),
+                quote!((#cycle_initial)),
+                quote!(Fixpoint),
+            )),
+            (None, None) => Ok((
                 quote!((salsa::plumbing::unexpected_cycle_recovery!)),
+                quote!((salsa::plumbing::unexpected_cycle_initial!)),
                 quote!(Panic),
-            )
+            )),
+            (Some(_), None) => Err(syn::Error::new_spanned(
+                self.args.cycle_fn.as_ref().unwrap(),
+                "must provide `cycle_initial` along with `cycle_fn`",
+            )),
+            (None, Some(_)) => Err(syn::Error::new_spanned(
+                self.args.cycle_initial.as_ref().unwrap(),
+                "must provide `cycle_fn` along with `cycle_initial`",
+            )),
         }
     }
 

components/salsa-macros/src/tracked_struct.rs

@@ -45,7 +45,9 @@ impl crate::options::AllowedOptions for TrackedStruct {
 
     const DB: bool = false;
 
-    const RECOVERY_FN: bool = false;
+    const CYCLE_FN: bool = false;
+
+    const CYCLE_INITIAL: bool = false;
 
     const LRU: bool = false;
 

src/active_query.rs

@@ -1,4 +1,4 @@
-use rustc_hash::FxHashMap;
+use rustc_hash::{FxHashMap, FxHashSet};
 
 use super::zalsa_local::{EdgeKind, QueryEdges, QueryOrigin, QueryRevisions};
 use crate::tracked_struct::IdentityHash;
@@ -9,7 +9,7 @@ use crate::{
     key::{DatabaseKeyIndex, DependencyIndex},
     tracked_struct::{Disambiguator, Identity},
     zalsa_local::EMPTY_DEPENDENCIES,
-    Cycle, Id, Revision,
+    Id, Revision,
 };
 
 #[derive(Debug)]
@@ -35,9 +35,6 @@ pub(crate) struct ActiveQuery {
     /// True if there was an untracked read.
     untracked_read: bool,
 
-    /// Stores the entire cycle, if one is found and this query is part of it.
-    pub(crate) cycle: Option<Cycle>,
-
     /// When new tracked structs are created, their data is hashed, and the resulting
     /// hash is added to this map. If it is not present, then the disambiguator is 0.
     /// Otherwise it is 1 more than the current value (which is incremented).
@@ -54,6 +51,9 @@ pub(crate) struct ActiveQuery {
     /// Stores the values accumulated to the given ingredient.
     /// The type of accumulated value is erased but known to the ingredient.
     pub(crate) accumulated: AccumulatedMap,
+
+    /// Provisional cycle results that this query depends on.
+    pub(crate) cycle_heads: FxHashSet<DatabaseKeyIndex>,
 }
 
 impl ActiveQuery {
@@ -64,10 +64,10 @@ impl ActiveQuery {
             changed_at: Revision::start(),
             input_outputs: FxIndexSet::default(),
             untracked_read: false,
-            cycle: None,
             disambiguator_map: Default::default(),
             tracked_struct_ids: Default::default(),
             accumulated: Default::default(),
+            cycle_heads: Default::default(),
         }
     }
 
@@ -76,10 +76,12 @@ impl ActiveQuery {
         input: DependencyIndex,
         durability: Durability,
         revision: Revision,
+        cycle_heads: &FxHashSet<DatabaseKeyIndex>,
     ) {
         self.input_outputs.insert((EdgeKind::Input, input));
         self.durability = self.durability.min(durability);
         self.changed_at = self.changed_at.max(revision);
+        self.cycle_heads.extend(cycle_heads);
     }
 
     pub(super) fn add_untracked_read(&mut self, changed_at: Revision) {
@@ -125,36 +127,11 @@ impl ActiveQuery {
             durability: self.durability,
             tracked_struct_ids: self.tracked_struct_ids,
             accumulated: self.accumulated,
+            cycle_ignore: !self.cycle_heads.is_empty(),
+            cycle_heads: self.cycle_heads,
         }
     }
 
-    /// Adds any dependencies from `other` into `self`.
-    /// Used during cycle recovery, see [`Runtime::unblock_cycle_and_maybe_throw`].
-    pub(super) fn add_from(&mut self, other: &ActiveQuery) {
-        self.changed_at = self.changed_at.max(other.changed_at);
-        self.durability = self.durability.min(other.durability);
-        self.untracked_read |= other.untracked_read;
-        self.input_outputs
-            .extend(other.input_outputs.iter().copied());
-    }
-
-    /// Removes the participants in `cycle` from my dependencies.
-    /// Used during cycle recovery, see [`Runtime::unblock_cycle_and_maybe_throw`].
-    pub(super) fn remove_cycle_participants(&mut self, cycle: &Cycle) {
-        for p in cycle.participant_keys() {
-            let p: DependencyIndex = p.into();
-            self.input_outputs.shift_remove(&(EdgeKind::Input, p));
-        }
-    }
-
-    /// Copy the changed-at, durability, and dependencies from `cycle_query`.
-    /// Used during cycle recovery, see [`Runtime::unblock_cycle_and_maybe_throw`].
-    pub(crate) fn take_inputs_from(&mut self, cycle_query: &ActiveQuery) {
-        self.changed_at = cycle_query.changed_at;
-        self.durability = cycle_query.durability;
-        self.input_outputs.clone_from(&cycle_query.input_outputs);
-    }
-
     pub(super) fn disambiguate(&mut self, key: IdentityHash) -> Disambiguator {
         let disambiguator = self
             .disambiguator_map