diff --git a/crates/calc/src/recalc.rs b/crates/calc/src/recalc.rs index b391821d7..634e391ba 100644 --- a/crates/calc/src/recalc.rs +++ b/crates/calc/src/recalc.rs @@ -60,6 +60,12 @@ const WIDE: u32 = 64; /// Insertions an [`Intervals`] keeps aside (scanned by every query) before merging them in. const PENDING: usize = 32; +/// Formulas a formula may calculate on demand one inside the other (each needing one another's +/// value before it is done). Each level takes stack, so a chain deeper than this is suspended and +/// calculated from the top instead (see [`PassHost::calculate`]): long dependency chains calculate +/// whatever order they come in, with bounded stack. +const MAX_NESTED: usize = 64; + /// Formula evaluations iterative calculation may spend on one recalculation (passes × formulas /// evaluated again), however high Maximum Iterations is. const MAX_ITERATION_WORK: usize = 10_000_000; @@ -395,6 +401,17 @@ struct PassHost<'a> { cycle: bool, cycles: Vec, depth: usize, + /// A formula needed at [`MAX_NESTED`] levels deep: the evaluations in progress are abandoned + /// and it is calculated first. + suspend: Option, + /// Formulas whose evaluation was abandoned for another one, to be calculated again after it. + /// They all need it, so reading one of them while it is calculated is a circular reference, + /// as reading a formula in progress is. + waiting: HashSet, + /// The formulas abandoned since the last suspension. + abandoned: Vec, + /// The formulas in `cycles`. + cycle_seen: HashSet, /// Iterative calculation: a circular reference reads the cell's previous value instead of /// giving `#CIRC!`. iterate: bool, @@ -425,6 +442,10 @@ impl<'a> PassHost<'a> { cycle: false, cycles: vec![], depth: 0, + suspend: None, + waiting: HashSet::default(), + abandoned: Vec::new(), + cycle_seen: HashSet::default(), iterate: false, prev: HashMap::default(), stack: Vec::new(), @@ -457,6 +478,36 @@ fn spill_blocked(host: &PassHost<'_>, (sheet, anchor): Key, a: &Array) -> bool { } impl PassHost<'_> { + /// Calculates `k` (and the formulas it reads) with bounded stack: when [`Self::compute`] would + /// go deeper than [`MAX_NESTED`], it suspends what is in progress, the formula it needed is + /// calculated from here, then the suspended ones again (reading the results kept so far). + fn calculate(&mut self, k: Key) { + // Formulas to calculate, innermost last, each with the ones abandoned to calculate the + // next. + let mut work: Vec<(Key, Vec)> = vec![(k, Vec::new())]; + while let Some(&(top, _)) = work.last() { + self.compute(top); + match self.suspend.take() { + Some(next) => { + let held = std::mem::take(&mut self.abandoned); + self.waiting.extend(held.iter().copied()); + if let Some(entry) = work.last_mut() { + entry.1 = held; + } + work.push((next, Vec::new())); + } + None => { + work.pop(); + if let Some(entry) = work.last_mut() { + for a in entry.1.drain(..) { + self.waiting.remove(&a); + } + } + } + } + } + } + fn compute(&mut self, k: Key) -> Value { if let Some(v) = self.results.get(&k) { let v = v.clone(); @@ -468,14 +519,25 @@ impl PassHost<'_> { let Some(expr) = self.exprs.get(&k) else { return self.wb.sheet(k.0).map(|s| s.value(k.1)).unwrap_or_default(); }; - if self.in_progress.contains(&k) || self.depth > 2000 { + if self.suspend.is_some() { + // Unwinding to `calculate`: the value is thrown away. + return Value::Error(CellError::Calc); + } + if self.in_progress.contains(&k) || self.waiting.contains(&k) { self.cycle = true; if self.iterate { return self.circular_read(k); } - self.cycles.push(k); + // Once each, though a suspended evaluation meets it again. + if self.cycle_seen.insert(k) { + self.cycles.push(k); + } return Value::Error(CellError::Circ); } + if self.depth >= MAX_NESTED { + self.suspend = Some(k); + return Value::Error(CellError::Calc); + } self.in_progress.insert(k); self.depth += 1; let outer_taint = std::mem::replace(&mut self.taint, false); @@ -487,6 +549,17 @@ impl PassHost<'_> { let mut ev = Evaluator::new(self, k.0, k.1); ev.value(&expr) }; + if self.suspend.is_some() { + // Abandoned: `calculate` evaluates this formula again once the one it needs is done. + self.depth -= 1; + self.in_progress.remove(&k); + self.abandoned.push(k); + if self.iterate { + self.stack.pop(); + } + self.taint = outer_taint; + return Value::Error(CellError::Calc); + } // A legacy array formula fills the range it was entered in. let legacy = self.wb.sheet(k.0).and_then(|s| s.cell(k.1)).and_then(|c| c.formula.as_ref()).and_then(|f| f.array); let v = match legacy { @@ -577,7 +650,7 @@ impl PassHost<'_> { self.pending = set.clone(); self.spills.retain(|a, _| !set.contains(a)); for k in &again { - self.compute(*k); + self.calculate(*k); } passes += 1; } @@ -969,7 +1042,7 @@ impl Calc { let mut host = PassHost::new(wb, &exprs, &mut rng, now); host.iterate = wb.calc.iterative; for k in &dirty { - host.compute(*k); + host.calculate(*k); } if host.iterate && !host.members.is_empty() { let passes = host.iterate_loops(&dirty, wb.calc.max_iterations, wb.calc.max_change); diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 196c57de7..ef83116e2 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -1266,3 +1266,91 @@ fn offset_extreme_arguments_return_ref() { assert_eq!(crate::evaluate(&wb, 0, c("D1"), formula), Value::Number(expected), "{formula}"); } } + +/// A formula reading one further down (or on a later sheet) is calculated on demand, inside the +/// formula that reads it. Chains of thousands of such formulas, common in financial models, used +/// to overflow the stack (or give `#CIRC!` past 2,000 levels); they now calculate in full. +#[test] +fn long_dependency_chains_against_row_order_calculate() { + const N: u32 = 20_000; + let mut t = T::new(); + { + let sh = t.wb.sheet_mut(0).unwrap(); + for r in 1..N { + sh.cells.set(c(&format!("A{r}")), Cell::formula(Formula::new(&format!("=A{}+1", r + 1)))); + } + sh.cells.set(c(&format!("A{N}")), Cell::value(Value::Number(1.0))); + } + t.calc.recalc_all(&mut t.wb); + assert_eq!(t.num("A1"), N as f64); + assert_eq!(t.num("A2"), (N - 1) as f64); + assert!(t.calc.circular.is_empty()); + // An edit at the far end recalculates the whole chain. + t.set(&format!("A{N}"), "10"); + assert_eq!(t.num("A1"), (N + 9) as f64); + assert!(t.calc.circular.is_empty()); + + // Hopping between two sheets at every step, through a range read (SUM) as well. + let mut t = T::new(); + t.wb.sheets.push(std::sync::Arc::new(gridcraft_model::Sheet::new("Data"))); + for r in 1..N { + t.wb.sheet_mut(0).unwrap().cells.set(c(&format!("A{r}")), Cell::formula(Formula::new(&format!("=Data!A{r}+1")))); + let next = format!("=SUM(Sheet1!A{}:A{})", r + 1, r + 1); + t.wb.sheet_mut(1).unwrap().cells.set(c(&format!("A{r}")), Cell::formula(Formula::new(&next))); + } + t.wb.sheet_mut(0).unwrap().cells.set(c(&format!("A{N}")), Cell::value(Value::Number(0.0))); + t.calc.recalc_all(&mut t.wb); + assert_eq!(t.num("A1"), (N - 1) as f64); + assert_eq!(t.wb.sheet(1).unwrap().value(c("A1")), Value::Number((N - 2) as f64)); + assert!(t.calc.circular.is_empty()); +} + +/// A loop longer than the on-demand nesting limit is still a circular reference (and still ends). +#[test] +fn long_circular_chain_is_circular() { + const N: u32 = 5_000; + let mut t = T::new(); + { + let sh = t.wb.sheet_mut(0).unwrap(); + for r in 1..N { + sh.cells.set(c(&format!("A{r}")), Cell::formula(Formula::new(&format!("=A{}+1", r + 1)))); + } + sh.cells.set(c(&format!("A{N}")), Cell::formula(Formula::new("=A1+1"))); + sh.cells.set(c("B1"), Cell::formula(Formula::new("=1+2"))); + } + t.calc.recalc_all(&mut t.wb); + assert_eq!(t.get("A1"), Value::Error(CellError::Circ)); + assert_eq!(t.get(&format!("A{N}")), Value::Error(CellError::Circ)); + assert_eq!(t.num("B1"), 3.0); + assert!(!t.calc.circular.is_empty()); + // Each formula is listed once at most. + let mut seen = t.calc.circular.clone(); + seen.sort_by_key(|(s, c)| (*s, c.row, c.col)); + seen.dedup(); + assert_eq!(seen.len(), t.calc.circular.len()); + // With iterative calculation the loop settles to numbers instead (no hang, no crash). + t.wb.calc.iterative = true; + t.wb.calc.max_iterations = 3; + t.calc.recalc_all(&mut t.wb); + assert!(t.get("A1").as_f64().is_some()); +} + +/// A loop closing back into the middle of a chain deeper than the on-demand nesting limit is found +/// where calculating it in one go finds it: at the formula read while it was being calculated. +#[test] +fn deep_loop_is_found_where_it_closes() { + let mut t = T::new(); + { + let sh = t.wb.sheet_mut(0).unwrap(); + for r in 1..300 { + sh.cells.set(c(&format!("A{r}")), Cell::formula(Formula::new(&format!("=A{}+1", r + 1)))); + } + sh.cells.set(c("A300"), Cell::formula(Formula::new("=A30+1"))); + // Reads the loop through IFERROR, so its value shows where the loop was cut. + sh.cells.set(c("B1"), Cell::formula(Formula::new("=IFERROR(A1,-1)"))); + } + t.calc.recalc_all(&mut t.wb); + assert_eq!(t.calc.circular, vec![(0, c("A30"))]); + assert_eq!(t.get("A1"), Value::Error(CellError::Circ)); + assert_eq!(t.num("B1"), -1.0); +}