daniel543x/gridcraft
1
0 Fork 0
mirror of https://github.com/storytold/gridcraft.git synced 2026-11-15 14:52:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

calc: linear dependency extraction for range-operator chains (#2183) (#2198)

Browse Source 
* calc: linear dependency extraction for range-operator chains (#2183)

precedents() handled a range operator with non-reference operands by recursing into both operands and then letting Expr::walk descend into them again, so a left-associative chain of k operands cost 2^k (a 60-operand INDEX(...):INDEX(...) chain would hang on open). It is now a hand-written collector that visits every node once, growing the per-sheet bounding boxes of the enclosing range operator as it goes. Past a depth (4096) or node (1,000,000) bound it marks the formula dynamic (always recalculated) instead of recursing further.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* calc: drop wall-clock timing from precedents tests (clippy)

std::time::Instant::now is a disallowed method (clippy.toml: traps on wasm).
The regression test only needs to finish; the old 2^k walk would never return.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* calc: bound precedents to 100 ms and box plain-reference range chains (#2183)

- range_operator_chain_precedents_are_linear times the direct precedents
  call against the 100 ms bound in the issue.
- A Range node with two plain Ref operands (A1:B2:C3, which the parser
  can't fold) now collects its bounding box like any other range node, so
  an edit to C1 recalculates.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Michael HigginsandClaude Opus 5.5 authored and GitHub committed 2026-11-14 16:53:03 +01:00
1 parent 10849d5003
commit fdba683246
2 files changed
+216 -46
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/calc/src/eval/special.rs
+137 -46
View File
@@ -27,62 +27,153 @@ pub fn resized_sum_range(sum: RangeRef, height: u32, width: u32) -> RangeRef {
RangeRef::new(sum.start, CellRef::new(row, col))
}
/// Most expression nodes [`precedents`] visits before giving up and marking the formula dynamic.
const MAX_PRECEDENT_NODES: usize = 1_000_000;
/// Collects the static references of an expression with their sheets resolved; `dynamic` is
/// set when the formula uses INDIRECT/OFFSET/names/tables whose targets can change.
///
/// Every node is visited once (#2183: range-operator chains used to re-walk shared operands,
/// costing 2^k for k operands), with an explicit stack, so a flat chain as long as Excel allows
/// (`1+1+…`, about 4,000 levels deep) never recurses. Past [`MAX_PRECEDENT_NODES`] the result
/// is conservative: `dynamic` is set so the formula recalculates every time.
pub fn precedents(wb: &Workbook, sheet: usize, e: &Expr) -> (Vec<Area>, bool) {
let mut out = Vec::new();
let mut dynamic = false;
let push = |out: &mut Vec<Area>, r: &Reference, range: RangeRef| match &r.sheet {
SheetSel::Current => out.push(Area { sheet, range }),
SheetSel::Named(n) => {
if let Some(i) = wb.sheet_index(n) {
out.push(Area { sheet: i, range });
}
let mut c = Precedents { wb, sheet, out: Vec::new(), dynamic: false, boxes: Vec::new() };
c.collect(e);
(c.out, c.dynamic)
}
/// Grows the per-sheet bounding box in `boxes` to cover `area`.
fn union_box(boxes: &mut Vec<Area>, area: Area) {
match boxes.iter_mut().find(|x| x.sheet == area.sheet) {
Some(x) => x.range = x.range.union(&area.range),
None => boxes.push(area),
}
}
/// One entry of [`Precedents::collect`]'s explicit stack. `boxes` indexes
/// [`Precedents::boxes`]: the bounding boxes of the innermost enclosing range operator, if any.
enum PrecedentStep<'e> {
/// Visit an expression and queue its children.
Visit(&'e Expr, Option<usize>),
/// Both operands of a range operator have been visited: record the bounding boxes they
/// collected in `inner`, growing the `outer` operator's boxes too.
Close { inner: usize, outer: Option<usize> },
}
struct Precedents<'w> {
wb: &'w Workbook,
sheet: usize,
out: Vec<Area>,
dynamic: bool,
/// Per-sheet bounding boxes, one list per range operator (`A1:INDEX(…)`) met so far.
boxes: Vec<Vec<Area>>,
}
impl Precedents<'_> {
/// Records `area`, also growing the bounding boxes of the enclosing range operator, if any.
fn add(&mut self, area: Area, boxes: Option<usize>) {
if let Some(b) = boxes.and_then(|i| self.boxes.get_mut(i)) {
union_box(b, area);
}
SheetSel::Span(a, b) => {
if let (Some(i), Some(j)) = (wb.sheet_index(a), wb.sheet_index(b)) {
for s in i.min(j)..=i.max(j) {
out.push(Area { sheet: s, range });
self.out.push(area);
}
fn push(&mut self, r: &Reference, range: RangeRef, boxes: Option<usize>) {
match &r.sheet {
SheetSel::Current => self.add(Area { sheet: self.sheet, range }, boxes),
SheetSel::Named(n) => {
if let Some(i) = self.wb.sheet_index(n) {
self.add(Area { sheet: i, range }, boxes);
}
}
SheetSel::Span(a, b) => {
if let (Some(i), Some(j)) = (self.wb.sheet_index(a), self.wb.sheet_index(b)) {
for s in i.min(j)..=i.max(j) {
self.add(Area { sheet: s, range }, boxes);
}
}
}
}
};
e.walk(&mut |x| match x {
Expr::Ref(r) => push(&mut out, r, r.range()),
// The cells SUMIF/AVERAGEIF really read from the sum range (see `resized_sum_range`); when
// the criteria range's size isn't known statically, recalculate the formula every time.
Expr::Call(n, args) if resizes_sum_range(n, args.len()) => match (args.first(), args.get(2)) {
(Some(Expr::Ref(c)), Some(Expr::Ref(s))) => {
let (crit, sum) = (c.range(), s.range());
push(&mut out, s, resized_sum_range(sum, crit.height(), crit.width()));
}
(_, Some(Expr::Number(_) | Expr::Text(_) | Expr::Bool(_) | Expr::Error(_) | Expr::Array(_) | Expr::Missing)) => {}
_ => dynamic = true,
},
Expr::Call(n, _)
if matches!(n.as_str(), "INDIRECT" | "OFFSET" | "NOW" | "TODAY" | "RAND" | "RANDBETWEEN" | "RANDARRAY" | "CELL" | "INFO") =>
{
dynamic = true
}
Expr::Name(_) | Expr::Struct(_) => dynamic = true,
Expr::Call(n, _) if gridcraft_functions::lookup(n).is_none() && !is_special(n) => dynamic = true,
// A range ending in a function's result (`A1:INDEX(B1:B9,n)`) covers the cells between
// the references inside it, not only those references.
Expr::Binary(BinOp::Range, a, b) if !matches!((&**a, &**b), (Expr::Ref(_), Expr::Ref(_))) => {
let mut inner = precedents(wb, sheet, a).0;
inner.extend(precedents(wb, sheet, b).0);
let mut boxes: Vec<Area> = Vec::new();
for area in inner {
match boxes.iter_mut().find(|x| x.sheet == area.sheet) {
Some(x) => x.range = x.range.union(&area.range),
None => boxes.push(area),
}
/// Visits every node of `root` once, parents before children, left to right. Areas found
/// inside a range operator (`A1:INDEX(…)`) also grow that operator's per-sheet bounding boxes.
fn collect(&mut self, root: &Expr) {
let mut pending = vec![PrecedentStep::Visit(root, None)];
let mut nodes = 0usize;
while let Some(step) = pending.pop() {
let (x, boxes) = match step {
PrecedentStep::Visit(x, boxes) => (x, boxes),
PrecedentStep::Close { inner, outer } => {
let areas = self.boxes.get_mut(inner).map(std::mem::take).unwrap_or_default();
for area in areas {
self.add(area, outer);
}
continue;
}
};
nodes = nodes.saturating_add(1);
if nodes > MAX_PRECEDENT_NODES {
self.dynamic = true;
return;
}
out.extend(boxes);
self.visit(x, boxes, &mut pending);
}
_ => {}
});
(out, dynamic)
}
/// Handles one node for [`Precedents::collect`], queueing the children still to visit.
fn visit<'e>(&mut self, x: &'e Expr, boxes: Option<usize>, pending: &mut Vec<PrecedentStep<'e>>) {
match x {
Expr::Ref(r) => self.push(r, r.range(), boxes),
// The cells SUMIF/AVERAGEIF really read from the sum range (see `resized_sum_range`); when
// the criteria range's size isn't known statically, recalculate the formula every time.
Expr::Call(n, args) if resizes_sum_range(n, args.len()) => match (args.first(), args.get(2)) {
(Some(Expr::Ref(c)), Some(Expr::Ref(s))) => {
let (crit, sum) = (c.range(), s.range());
self.push(s, resized_sum_range(sum, crit.height(), crit.width()), boxes);
}
(_, Some(Expr::Number(_) | Expr::Text(_) | Expr::Bool(_) | Expr::Error(_) | Expr::Array(_) | Expr::Missing)) => {}
_ => self.dynamic = true,
},
Expr::Call(n, _)
if matches!(n.as_str(), "INDIRECT" | "OFFSET" | "NOW" | "TODAY" | "RAND" | "RANDBETWEEN" | "RANDARRAY" | "CELL" | "INFO") =>
{
self.dynamic = true
}
Expr::Name(_) | Expr::Struct(_) => self.dynamic = true,
Expr::Call(n, _) if gridcraft_functions::lookup(n).is_none() && !is_special(n) => self.dynamic = true,
// A range ending in a function's result (`A1:INDEX(B1:B9,n)`), or a chain of plain
// references the parser can't fold into one (`A1:B2:C3`), covers the cells between the
// references inside it, not only those references. Both operands are visited once,
// collecting their bounding boxes on the way, which `Close` then records.
Expr::Binary(BinOp::Range, a, b) => {
let inner = self.boxes.len();
self.boxes.push(Vec::new());
pending.push(PrecedentStep::Close { inner, outer: boxes });
pending.push(PrecedentStep::Visit(b, Some(inner)));
pending.push(PrecedentStep::Visit(a, Some(inner)));
return;
}
_ => {}
}
// Children are pushed in reverse so they pop left to right.
let visit = |e: &'e Expr| PrecedentStep::Visit(e, boxes);
match x {
Expr::Array(rows) => pending.extend(rows.iter().flatten().rev().map(visit)),
Expr::Unary(_, e) | Expr::Paren(e) => pending.push(visit(e)),
Expr::Binary(_, a, b) => {
pending.push(visit(b));
pending.push(visit(a));
}
Expr::Call(_, args) => pending.extend(args.iter().rev().map(visit)),
Expr::Invoke(c, args) => {
pending.extend(args.iter().rev().map(visit));
pending.push(visit(c));
}
_ => {}
}
}
}
fn is_special(n: &str) -> bool {
crates/calc/src/tests.rs
+79
View File
@@ -2331,6 +2331,85 @@ fn median_large_values_recalculate_dependents() {
}
}
/// #2183: `INDEX(…):INDEX(…):…` used to re-walk each operand of the left-associative range chain,
/// costing 2^k for k operands (28 took 40 s; 60 would take millennia), on open and on every edit.
#[test]
fn range_operator_chain_precedents_are_linear() {
let operands: Vec<String> = (1..=60).map(|i| format!("INDEX(A{i},1)")).collect();
let src = format!("=SUM({})", operands.join(":"));
let e = gridcraft_formula::parse(&src).unwrap();
let wb = Workbook::new();
// With the old 2^k walk this call would never return; #2183 asks for under 100 ms.
#[allow(clippy::disallowed_methods)] // a native test: the clock is there
let t0 = std::time::Instant::now();
let (areas, dynamic) = crate::eval::precedents(&wb, 0, &e);
let took = t0.elapsed();
assert!(took < std::time::Duration::from_millis(100), "precedents took {took:?}");
assert!(!dynamic);
// The chain covers A1:A60, not only the 60 cells named in it.
let whole = gridcraft_core::RangeRef::new(c("A1"), c("A60"));
assert!(areas.iter().any(|a| a.sheet == 0 && a.range == whole), "{areas:?}");
// Each node is visited once: 60 references plus one box per range operator.
assert!(areas.len() < 200, "{} areas", areas.len());
// Through the recalculation engine: setting it finishes and editing a cell in the box recalculates.
let mut t = T::new();
for i in 1..=60 {
t.set(&format!("A{i}"), "1");
}
t.set("B1", &src);
assert_eq!(t.num("B1"), 60.0);
t.set("A30", "5");
assert_eq!(t.num("B1"), 64.0);
}
/// A chained range of plain references (`A1:B2:C3`, which the parser can't fold into one
/// reference) covers their bounding box, as the evaluator expands it: an edit inside the box but
/// outside each operand still recalculates.
#[test]
fn chained_plain_range_precedents_cover_the_bounding_box() {
let e = gridcraft_formula::parse("=SUM(A1:B2:C3)").unwrap();
let (areas, dynamic) = crate::eval::precedents(&Workbook::new(), 0, &e);
assert!(!dynamic);
let whole = gridcraft_core::RangeRef::new(c("A1"), c("C3"));
assert!(areas.iter().any(|a| a.sheet == 0 && a.range == whole), "{areas:?}");
let mut t = T::new();
t.set("A1", "1");
t.set("E1", "=SUM(A1:B2:C3)");
assert_eq!(t.num("E1"), 1.0);
t.set("C1", "4");
assert_eq!(t.num("E1"), 5.0);
}
/// `precedents` walks with an explicit stack: an expression nested deeper than any formula
/// within the length limit can build still finds its references, on a small stack.
#[test]
fn precedents_of_deep_nesting_do_not_recurse() {
let (areas, dynamic) = std::thread::Builder::new()
.stack_size(256 * 1024)
.spawn(|| {
let mut e = gridcraft_formula::parse("=A1").unwrap();
for _ in 0..10_000 {
e = gridcraft_formula::Expr::Paren(Box::new(e));
}
let found = crate::eval::precedents(&Workbook::new(), 0, &e);
// Dropping the deep tree recurses; leak it so only `precedents` is under test.
std::mem::forget(e);
found
})
.unwrap()
.join()
.unwrap();
assert!(!dynamic);
assert_eq!(areas.len(), 1);
// Shallow nesting stays static.
let e = gridcraft_formula::parse("=((((A1))))").unwrap();
let (areas, dynamic) = crate::eval::precedents(&Workbook::new(), 0, &e);
assert!(!dynamic);
assert_eq!(areas.len(), 1);
}
#[test]
fn cell_filename_names_the_workbook_file() {
// Excel returns `<folder>/[<file>]<sheet>`, and the classic sheet-name formula splits it on