Merge pull request #2693 from vangel76/perf/calc-expr-arc-2685

calc: rebuild the dependency graph from shared syntax trees

crates/calc/src/recalc.rs

@@ -128,9 +128,8 @@ impl Calc {
         for (si, sheet) in wb.sheets.iter().enumerate() {
             for (c, cell) in sheet.cells.iter() {
                 if let Some(f) = &cell.formula
-                    && let Some(e) = f.expr()
+                    && let Some((areas, dynamic)) = f.with_expr(|e| precedents(wb, si, e))
                 {
-                    let (areas, dynamic) = precedents(wb, si, &e);
                     self.graph.insert((si, c), Node { areas, dynamic });
                     if cell.value == Value::Error(CellError::Circ) {
                         circ.push((si, c));
@@ -209,14 +208,14 @@ impl Calc {
     pub fn cells_changed(&mut self, wb: &mut Workbook, changed: &[Key]) {
         let mut cleared: Vec<(usize, RangeRef)> = Vec::new();
         for &k in changed {
-            let formula = wb.sheet(k.0).and_then(|s| s.cell(k.1)).and_then(|c| c.formula.clone());
+            let precedent =
+                wb.sheet(k.0).and_then(|s| s.cell(k.1)).and_then(|c| c.formula.as_ref()).and_then(|f| f.with_expr(|e| precedents(wb, k.0, e)));
             let py = wb.sheet(k.0).and_then(|s| s.cell(k.1)).and_then(|c| c.py.as_ref());
             let python = py.is_some();
             // A Python cell whose result (if any) is not an array spills nothing.
             let spills = py.and_then(|p| p.result_value()).is_some_and(|v| matches!(v, Value::Array(_)));
-            match formula.and_then(|f| f.expr()) {
-                Some(e) => {
-                    let (areas, dynamic) = precedents(wb, k.0, &e);
+            match precedent {
+                Some((areas, dynamic)) => {
                     self.graph.insert(k, Node { areas, dynamic });
                 }
                 // See `rebuild`: its spill is laid out again from its result when it is calculated.

crates/calc/src/tests.rs

@@ -2379,6 +2379,45 @@ fn loan_schedule_residue_keeps_excels_sign() {
     assert_eq!(t.get("P7"), Value::Text("".into()));
 }
 
+/// `rebuild` and `cells_changed` read formulas through `Formula::with_expr`, which borrows the
+/// cached tree or parses on the fly. The dependency graph must come out the same for formulas that
+/// carry their tree and for formulas that only carry their text (as after loading a file).
+#[test]
+fn graph_built_from_text_only_formulas_matches_cached() {
+    fn run(cached: bool) -> Vec<Value> {
+        let mut t = T::new();
+        t.set("A1", "2");
+        for (at, f) in [("B1", "=A1*10"), ("C1", "=B1+A1"), ("D1", "=SUM(A1:C1)"), ("E1", "=NOPE(")] {
+            // E1 does not parse, so it has no tree either way and only covers the `None` branch.
+            let mut formula = Formula::new(f);
+            if !cached {
+                formula.expr = None;
+            }
+            t.wb.sheet_mut(0).unwrap().cells.set(c(at), Cell::formula(formula));
+        }
+        t.calc.rebuild(&t.wb);
+        t.calc.recalc_all(&mut t.wb);
+        let mut out: Vec<Value> = ["B1", "C1", "D1", "E1"].iter().map(|a| t.get(a)).collect();
+        // Editing the input must reach the dependents through the rebuilt graph.
+        t.set("A1", "5");
+        out.extend(["B1", "C1", "D1"].iter().map(|a| t.get(a)));
+        // A text-only formula that is rewritten through `cells_changed` joins the graph too.
+        let mut formula = Formula::new("=A1+100");
+        if !cached {
+            formula.expr = None;
+        }
+        t.wb.sheet_mut(0).unwrap().cells.set(c("F1"), Cell::formula(formula));
+        t.calc.cells_changed(&mut t.wb, &[(0, c("F1"))]);
+        t.set("A1", "7");
+        out.push(t.get("F1"));
+        out
+    }
+    let cached = run(true);
+    assert_eq!(cached, run(false));
+    assert_eq!(cached[4], Value::Number(50.0));
+    assert_eq!(cached[7], Value::Number(107.0));
+}
+
 /// Makes `at` a Python cell showing `cached` (its value before this calculation), with `result`
 /// as its kept result, and returns the cell's key for `Calc::cells_changed`.
 fn plant_python(t: &mut T, at: &str, cached: Value, result: Value) -> (usize, CellRef) {

crates/model/src/cell.rs

@@ -47,6 +47,16 @@ impl Formula {
     pub fn expr_arc(&self) -> Option<Arc<Expr>> {
         self.expr.clone().or_else(|| gridcraft_formula::parse(&self.text).ok().map(Arc::new))
     }
+    /// Runs `f` on the parsed expression, borrowing the cached tree or, after deserialization,
+    /// a freshly parsed one that is dropped afterwards. Unlike [`expr`](Self::expr) it never copies
+    /// the tree, and unlike [`expr_arc`](Self::expr_arc) it needs no reference count or `Arc`
+    /// allocation. `None` when the text does not parse.
+    pub fn with_expr<R>(&self, f: impl FnOnce(&Expr) -> R) -> Option<R> {
+        match &self.expr {
+            Some(e) => Some(f(e)),
+            None => gridcraft_formula::parse(&self.text).ok().map(|e| f(&e)),
+        }
+    }
 }
 
 /// A static picture stored as cell content, independent of floating drawing anchors.