From 2aeb21d4d3494e85af0caf0e417cf01201367c52 Mon Sep 17 00:00:00 2001 From: Harrison Booth Date: Wed, 14 Oct 2026 20:20:27 -0500 Subject: [PATCH 1/5] Formulas: flat operator chains as long as Excel allows (#916) along: =1+1+... with 4,096 operands (8,192 characters, exactly Excel's limit, which Excel accepts and calculates) overflowed the stack when entered, because the derived Clone of Expr recursed once per operand down the chain's left spine. Saving hit the same in the XLSX writer. - Expr: Clone, PartialEq, walk and map follow the left spine in a loop and recurse only into right operands and real nesting. - XLSX: _xlpm. prefixing and the implicit-intersection scan follow the spine too. The scan's depth limit of 64 wrote any chain of more than about 64 operands as a dynamic-array formula; such sums are plain formulas again (ranges in the chain still get array evaluation). - Evaluate Formula borrows sub-expressions instead of cloning each operator's subtree (quadratic memory on a long chain). - Spanish formula entry and display no longer refuse chains of more than 256 operands; the left spine does not count as nesting. Tests run on a 1 MiB stack (the Windows main thread) with Excel's observed results: formula (clone/eq/walk/map/adjust at 4,096), calc (401 and 4,096 operands, -, *, /, ^, &, %, comparisons, 1,500 refs), xlsx (long chains written as scalar formulas), engine (cell.set, fill, copy/paste, insert rows/columns, undo/redo, Evaluate Formula, XLSX save and reopen), Spanish entry. Each fails without the fix. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 4 +- crates/calc/src/tests.rs | 35 ++++++ crates/engine/src/cmd/formulas.rs | 8 +- crates/engine/tests/long-operator-chains.rs | 87 ++++++++++++++ crates/formula/src/ast.rs | 124 +++++++++++++++++--- crates/formula/src/input.rs | 19 ++- crates/formula/src/tests.rs | 34 ++++++ crates/formula/tests/spanish_input.rs | 31 ++++- crates/xlsx/src/array_formula.rs | 30 ++++- crates/xlsx/src/fmla.rs | 16 ++- crates/xlsx/src/tests/array_export.rs | 25 ++++ docs/function-parity.md | 4 +- 12 files changed, 375 insertions(+), 42 deletions(-) create mode 100644 crates/engine/tests/long-operator-chains.rs diff --git a/ROADMAP.md b/ROADMAP.md index 070552d07..2ec3b5a99 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~25 points of "ready for real work" and ~160–240 h away -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (rows auto-fit oversized fonts on open and whole-row/column formatting) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-14 · **Change:** minor (long flat operator chains up to Excel's formula limit) · **Target:** Microsoft Excel (Microsoft 365) GridCraft aims at full Microsoft Excel parity, and at being better in places: faster, open (XLSX is the native format), fully scriptable by agents (MCP, control channel, CLI), and on the web. @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-14 | Long flat operator chains (`=A1+A2+…`) up to Excel's 8,192-character limit (4,096 operands): entering, filling, inserting rows, undo, Evaluate Formula and XLSX save no longer overflow the stack; chains past 64 operands are saved as plain formulas, not dynamic arrays; Spanish formula entry accepts chains past 256 operands (#916) | | 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) | | 2026-10-12 | Number formats: parsing a format code is linear in its length (long runs of `[`, `,` or `/` no longer take quadratic time), and an unclosed `[` is a literal when splitting sections as well as when tokenizing (#810) | | 2026-10-12 | Number formats: a fraction denominator with 65,536 zeros no longer crashes; format codes are capped at 255 characters like Excel (longer codes display as General, `TEXT` gives `#VALUE!`) (#807) | @@ -193,6 +194,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | Change | Summary | |---|---|---| +| 2026-10-14 | minor | Progress log: long flat operator chains (#916) | | 2026-10-12 | minor | Rows auto-fit oversized fonts on open and whole-row/column formatting (#833); progress log row | | 2026-10-12 | minor | Progress log: multi-area row/column delete (#758) | | 2026-10-12 | trivial | Progress log: XLSX chart sheets kept on save (#727) | diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 97f7a8350..5f843f66e 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -702,6 +702,41 @@ fn long_operator_chains() { assert_eq!(t.num("B2"), 5.0); } +#[test] +fn flat_chains_as_long_as_excel_allows() { + // Values observed in Excel (#916): a flat chain has no operand limit below the 8,192-character + // formula limit, which `=1+1+…` with 4,096 operands meets exactly. On a 1 MiB stack, as on + // the Windows main thread. + std::thread::Builder::new() + .stack_size(1 << 20) + .spawn(|| { + let chain = |op: &str, item: &str, n: usize| format!("={}", vec![item; n].join(op)); + let mut t = T::new(); + for r in 1..=1500 { + t.set(&format!("A{r}"), "1"); + } + t.set("B1", &chain("+", "1", 401)); + t.set("B2", &chain("+", "1", 4096)); + t.set("B3", &chain("-", "1", 2000)); + t.set("B4", &chain("*", "1", 4096)); + t.set("B5", &chain("/", "2", 4096)); + t.set("B6", &chain("^", "1", 4096)); + t.set("B7", &chain("+", "1%", 2000)); + t.set("B8", "=1=1=1"); + t.set("B9", &format!("={}", (1..=1500).map(|r| format!("A{r}")).collect::>().join("+"))); + t.set("B10", &chain("&", "1", 2000)); + for (at, want) in [("B1", 401.0), ("B2", 4096.0), ("B3", -1998.0), ("B4", 1.0), ("B5", 0.0), ("B6", 1.0), ("B9", 1500.0)] { + assert_eq!(t.num(at), want, "{at}"); + } + assert!((t.num("B7") - 20.0).abs() < 1e-9); + assert_eq!(t.get("B8"), Value::Bool(false)); + assert_eq!(t.get("B10"), Value::text("1".repeat(2000))); + }) + .expect("spawn") + .join() + .expect("long chains must not overflow the stack"); +} + #[test] fn subtotal_skips_hidden() { let mut t = T::new(); diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index 454c545d0..eb8b1a1ab 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -528,13 +528,15 @@ fn evaluate_formula(s: &mut Session, p: &Json) -> Result { let mut steps = Vec::new(); let mut nodes = Vec::new(); expr.walk(&mut |e| { + // Borrowed, not cloned: copying every operator of a long chain A1+A2+… copies the + // rest of the chain each time (quadratic memory). if matches!(e, gridcraft_formula::Expr::Call(..) | gridcraft_formula::Expr::Binary(..) | gridcraft_formula::Expr::Ref(_)) { - nodes.push(e.clone()); + nodes.push(e); } }); for e in nodes.into_iter().rev().take(200) { - let v = gridcraft_calc::recalc::evaluate_expr(&d.wb, sheet, at, &e); - steps.push(json!({"expression": gridcraft_formula::print(&e), "value": crate::cmd::inspect::value_json(&v)})); + let v = gridcraft_calc::recalc::evaluate_expr(&d.wb, sheet, at, e); + steps.push(json!({"expression": gridcraft_formula::print(e), "value": crate::cmd::inspect::value_json(&v)})); } let result = gridcraft_calc::recalc::evaluate_expr(&d.wb, sheet, at, &expr); Ok(json!({"formula": format!("={text}"), "steps": steps, "result": crate::cmd::inspect::value_json(&result)})) diff --git a/crates/engine/tests/long-operator-chains.rs b/crates/engine/tests/long-operator-chains.rs new file mode 100644 index 000000000..97c95ce38 --- /dev/null +++ b/crates/engine/tests/long-operator-chains.rs @@ -0,0 +1,87 @@ +//! Flat operator chains as long as Excel allows (#916). `=1+1+…` with 4,096 operands is +//! exactly 8,192 characters, Excel's formula limit, and builds a tree 4,095 levels deep on its +//! left. Entering, editing, saving and reopening such a formula must neither overflow the stack +//! nor fall back to an error: Excel computes it. + +use gridcraft_core::{CellRef, Value}; +use gridcraft_engine::{DocState, Session, io}; +use serde_json::json; + +/// Excel's longest flat chain of literals: 4,096 operands, 8,192 characters with the `=`. +fn literal_chain(op: &str) -> String { + format!("={}", vec!["1"; 4096].join(op)) +} + +/// `=A1+A2+…+A1500`: 7,893 characters, inside Excel's limit. +fn reference_chain() -> String { + format!("={}", (1..=1500).map(|r| format!("A{r}")).collect::>().join("+")) +} + +fn value(session: &Session, cell: &str) -> Value { + let at = CellRef::parse(cell).expect("cell"); + session.doc().expect("doc").wb.sheet(0).expect("sheet").value(at) +} + +/// Runs `f` on a 1 MiB stack, the size of the Windows main thread the desktop app runs on. +fn on_small_stack(f: impl FnOnce() + Send + 'static) { + std::thread::Builder::new().stack_size(1 << 20).spawn(f).expect("spawn").join().expect("long chains must not overflow the stack"); +} + +#[test] +fn longest_flat_chains_survive_editing_saving_and_reopening() { + on_small_stack(|| { + let mut session = Session::new(); + session.new_workbook(); + session.execute("range.setValues", json!({"range": "A1:A1500", "values": vec![[1]; 1500]})).expect("values"); + session.execute("cell.set", json!({"cell": "C1", "input": literal_chain("+")})).expect("plus chain"); + session.execute("cell.set", json!({"cell": "C2", "input": literal_chain("-")})).expect("minus chain"); + session.execute("cell.set", json!({"cell": "C3", "input": literal_chain("&")})).expect("concat chain"); + session.execute("cell.set", json!({"cell": "C4", "input": reference_chain()})).expect("reference chain"); + assert_eq!(value(&session, "C1"), Value::Number(4096.0)); + assert_eq!(value(&session, "C2"), Value::Number(-4094.0)); + assert_eq!(value(&session, "C3"), Value::text("1".repeat(4096))); + assert_eq!(value(&session, "C4"), Value::Number(1500.0)); + + // Precedents of a long reference chain still drive recalculation. + session.execute("cell.set", json!({"cell": "A750", "input": "101"})).expect("edit precedent"); + assert_eq!(value(&session, "C4"), Value::Number(1600.0)); + + // Evaluate Formula walks every operator of the chain. + let steps = session.execute("formulas.evaluateFormula", json!({"cell": "C4"})).expect("evaluate formula"); + assert_eq!(steps["steps"].as_array().map(Vec::len), Some(200)); + assert_eq!(steps["result"], json!(1600.0)); + assert_eq!(session.execute("formulas.evaluate", json!({"formula": literal_chain("+")})).expect("evaluate"), json!(4096.0)); + + // Copy and paste move every reference (Excel: the pasted C4 sums A2:A1501). + session.execute("edit.copy", json!({"range": "C4"})).expect("copy"); + session.execute("edit.paste", json!({"at": "C5"})).expect("paste"); + assert_eq!(value(&session, "C5"), Value::Number(1599.0)); + session.execute("edit.undo", json!({})).expect("undo paste"); + + // Fill and structural edits rewrite every reference in the chain (Excel: an inserted + // row above shifts A1:A1500 to A2:A1501; the sum is unchanged). + session.execute("edit.fillRight", json!({"range": "C1:D4"})).expect("fill right"); + assert_eq!(value(&session, "D1"), Value::Number(4096.0)); + assert_eq!(value(&session, "D4"), Value::Number(0.0)); + session.execute("home.insertRows", json!({"rows": "1:1"})).expect("insert row"); + assert_eq!(value(&session, "C5"), Value::Number(1600.0)); + session.execute("home.insertColumns", json!({"cols": "A:A"})).expect("insert column"); + assert_eq!(value(&session, "D5"), Value::Number(1600.0)); + session.execute("edit.undo", json!({})).expect("undo"); + session.execute("edit.undo", json!({})).expect("undo"); + assert_eq!(value(&session, "C4"), Value::Number(1600.0)); + session.execute("edit.redo", json!({})).expect("redo"); + assert_eq!(value(&session, "C5"), Value::Number(1600.0)); + + // XLSX round trip keeps the formulas and their values. + let wb = &session.doc().expect("doc").wb; + let bytes = io::save_bytes(wb, "chains.xlsx").expect("save"); + let (reopened, _) = io::open_bytes("chains.xlsx", &bytes).expect("open"); + let mut session = Session::new(); + session.add_document(DocState::new(reopened, None, "chains".into())); + session.execute("formulas.calculateNow", json!({})).expect("recalc"); + assert_eq!(value(&session, "C2"), Value::Number(4096.0)); + assert_eq!(value(&session, "C3"), Value::Number(-4094.0)); + assert_eq!(value(&session, "C5"), Value::Number(1600.0)); + }); +} diff --git a/crates/formula/src/ast.rs b/crates/formula/src/ast.rs index 9b04140cc..7302b4d04 100644 --- a/crates/formula/src/ast.rs +++ b/crates/formula/src/ast.rs @@ -182,7 +182,13 @@ impl BinOp { } } -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +/// A formula's syntax tree. +/// +/// A flat operator chain such as `=A1+A2+…` nests one [`Expr::Binary`] per operator on its +/// left, so a formula as long as Excel allows (8,192 characters) is about 4,000 levels deep. +/// `Clone`, `PartialEq`, [`Expr::walk`] and [`Expr::map`] therefore follow that left spine in a +/// loop and recurse only into right-hand operands and real nesting, which the parser bounds. +#[derive(Debug, Serialize, Deserialize)] pub enum Expr { Number(f64), Text(Arc), @@ -206,36 +212,52 @@ pub enum Expr { } impl Expr { - /// Visits every node depth-first. + /// Visits every node depth-first, parents before children, left to right. pub fn walk<'a>(&'a self, f: &mut dyn FnMut(&'a Expr)) { - f(self); - match self { - Expr::Array(rows) => rows.iter().flatten().for_each(|e| e.walk(f)), - Expr::Unary(_, e) | Expr::Paren(e) => e.walk(f), - Expr::Binary(_, a, b) => { - a.walk(f); - b.walk(f); + // An explicit stack: a long operator chain must not recurse once per operand. + let mut pending = vec![self]; + while let Some(e) = pending.pop() { + f(e); + match e { + Expr::Array(rows) => pending.extend(rows.iter().flatten().rev()), + Expr::Unary(_, e) | Expr::Paren(e) => pending.push(e), + Expr::Binary(_, a, b) => { + pending.push(b); + pending.push(a); + } + Expr::Call(_, args) => pending.extend(args.iter().rev()), + Expr::Invoke(c, args) => { + pending.extend(args.iter().rev()); + pending.push(c); + } + _ => {} } - Expr::Call(_, args) => args.iter().for_each(|e| e.walk(f)), - Expr::Invoke(c, args) => { - c.walk(f); - args.iter().for_each(|e| e.walk(f)); - } - _ => {} } } - /// Rewrites every node bottom-up. + /// Rewrites every node bottom-up, children left to right before their parent. pub fn map(self, f: &mut dyn FnMut(Expr) -> Expr) -> Expr { - let e = match self { + // Unwind the left spine of a long operator chain into a list, then rebuild it from the + // innermost operand outwards, in the order a recursive rewrite would visit the nodes. + let mut spine = Vec::new(); + let mut node = self; + while let Expr::Binary(op, a, b) = node { + spine.push((op, b)); + node = *a; + } + let e = match node { Expr::Array(rows) => Expr::Array(rows.into_iter().map(|r| r.into_iter().map(|e| e.map(f)).collect()).collect()), Expr::Unary(op, e) => Expr::Unary(op, Box::new(e.map(f))), Expr::Paren(e) => Expr::Paren(Box::new(e.map(f))), - Expr::Binary(op, a, b) => Expr::Binary(op, Box::new(a.map(f)), Box::new(b.map(f))), Expr::Call(n, args) => Expr::Call(n, args.into_iter().map(|e| e.map(f)).collect()), Expr::Invoke(c, args) => Expr::Invoke(Box::new(c.map(f)), args.into_iter().map(|e| e.map(f)).collect()), other => other, }; - f(e) + let mut acc = f(e); + while let Some((op, b)) = spine.pop() { + let b = b.map(f); + acc = f(Expr::Binary(op, Box::new(acc), Box::new(b))); + } + acc } /// All references in the formula. pub fn references(&self) -> Vec<&Reference> { @@ -259,6 +281,70 @@ impl Expr { } } +impl Clone for Expr { + fn clone(&self) -> Self { + // Copy the left spine of an operator chain in a loop (see [`Expr`]). + let mut spine = Vec::new(); + let mut node = self; + while let Expr::Binary(op, a, b) = node { + spine.push((*op, b)); + node = a; + } + let mut acc = match node { + Expr::Number(n) => Expr::Number(*n), + Expr::Text(t) => Expr::Text(t.clone()), + Expr::Bool(b) => Expr::Bool(*b), + Expr::Error(e) => Expr::Error(*e), + Expr::Array(rows) => Expr::Array(rows.clone()), + Expr::Ref(r) => Expr::Ref(r.clone()), + Expr::Name(n) => Expr::Name(n.clone()), + Expr::Struct(s) => Expr::Struct(s.clone()), + Expr::Unary(op, e) => Expr::Unary(*op, e.clone()), + Expr::Binary(op, a, b) => Expr::Binary(*op, a.clone(), b.clone()), + Expr::Call(n, args) => Expr::Call(n.clone(), args.clone()), + Expr::Invoke(c, args) => Expr::Invoke(c.clone(), args.clone()), + Expr::Missing => Expr::Missing, + Expr::Paren(e) => Expr::Paren(e.clone()), + }; + while let Some((op, b)) = spine.pop() { + acc = Expr::Binary(op, Box::new(acc), b.clone()); + } + acc + } +} + +impl PartialEq for Expr { + fn eq(&self, other: &Self) -> bool { + // Compare the left spines of operator chains in a loop (see [`Expr`]). + let (mut x, mut y) = (self, other); + loop { + match (x, y) { + (Expr::Binary(o1, a1, b1), Expr::Binary(o2, a2, b2)) => { + if o1 != o2 || b1 != b2 { + return false; + } + x = a1; + y = a2; + } + (Expr::Number(a), Expr::Number(b)) => return a == b, + (Expr::Text(a), Expr::Text(b)) => return a == b, + (Expr::Bool(a), Expr::Bool(b)) => return a == b, + (Expr::Error(a), Expr::Error(b)) => return a == b, + (Expr::Array(a), Expr::Array(b)) => return a == b, + (Expr::Ref(a), Expr::Ref(b)) => return a == b, + (Expr::Name(a), Expr::Name(b)) => return a == b, + (Expr::Struct(a), Expr::Struct(b)) => return a == b, + (Expr::Unary(o1, a), Expr::Unary(o2, b)) => return o1 == o2 && a == b, + (Expr::Call(n1, a), Expr::Call(n2, b)) => return n1 == n2 && a == b, + (Expr::Invoke(c1, a), Expr::Invoke(c2, b)) => return c1 == c2 && a == b, + (Expr::Missing, Expr::Missing) => return true, + (Expr::Paren(a), Expr::Paren(b)) => return a == b, + _ => return false, + } + } + } +} + pub(crate) fn clamp_anchor(row: i64, col: i64) -> Option<(u32, u32)> { if (0..MAX_ROWS as i64).contains(&row) && (0..MAX_COLS as i64).contains(&col) { Some((row as u32, col as u32)) } else { None } } diff --git a/crates/formula/src/input.rs b/crates/formula/src/input.rs index 8568023ed..7c0fe8b53 100644 --- a/crates/formula/src/input.rs +++ b/crates/formula/src/input.rs @@ -63,8 +63,9 @@ pub fn parse_input(src: &str, locale: FormulaLocale, known_name: impl Fn(&str) - /// Prints canonical expressions for editing. Name collisions retain the canonical spelling /// so displaying then committing a formula never changes a defined-name reference. pub fn print_input(expr: &Expr, locale: FormulaLocale, known_name: impl Fn(&str) -> bool) -> Result { - // Check before cloning/printing: a flat operator chain can produce a deep AST even - // though the parser itself did not recurse deeply. + // Check before cloning/printing. A flat operator chain such as A1+A2+… nests on its left + // without nesting in the text: cloning, printing and `normalize` follow that left spine in + // a loop, so it does not count (Excel accepts 4,096 operands, #916). Other nesting does. let mut pending = vec![(expr, 0usize)]; while let Some((node, depth)) = pending.pop() { if depth > parser::MAX_DEPTH { @@ -73,7 +74,7 @@ pub fn print_input(expr: &Expr, locale: FormulaLocale, known_name: impl Fn(&str) match node { Expr::Unary(_, child) | Expr::Paren(child) => pending.push((child, depth + 1)), Expr::Binary(_, left, right) => { - pending.push((left, depth + 1)); + pending.push((left, depth)); pending.push((right, depth + 1)); } Expr::Call(_, args) => pending.extend(args.iter().map(|e| (e, depth + 1))), @@ -185,9 +186,15 @@ fn normalize(expr: &mut Expr, known_name: &impl Fn(&str) -> bool, bindings: &mut } } Expr::Unary(_, child) | Expr::Paren(child) => normalize(child, known_name, bindings, depth + 1, display)?, - Expr::Binary(_, left, right) => { - normalize(left, known_name, bindings, depth + 1, display)?; - normalize(right, known_name, bindings, depth + 1, display)?; + Expr::Binary(..) => { + // Follow the left spine of a flat chain (A1+A2+…) in a loop: it is not nesting. + // Each call leaves `bindings` as it found them, so the order does not matter. + let mut node = expr; + while let Expr::Binary(_, left, right) = node { + normalize(right, known_name, bindings, depth + 1, display)?; + node = left; + } + normalize(node, known_name, bindings, depth + 1, display)?; } Expr::Array(rows) if display => { for row in rows { diff --git a/crates/formula/src/tests.rs b/crates/formula/src/tests.rs index d23eba0e2..185ffdb23 100644 --- a/crates/formula/src/tests.rs +++ b/crates/formula/src/tests.rs @@ -401,3 +401,37 @@ fn formulas_longer_than_excel_allows_are_refused() { assert!(text.len() > crate::MAX_FORMULA_LEN); assert!(parse(&text).is_ok()); } + +#[test] +fn longest_flat_chains_clone_compare_walk_and_adjust_without_deep_recursion() { + // `=1+1+…` with 4,096 operands is 8,192 characters, Excel's formula limit, and nests 4,095 + // levels on its left. On a 1 MiB stack (the Windows main thread) the derived, recursive + // Clone overflowed when the engine stored such a formula (#916). + let handle = std::thread::Builder::new() + .stack_size(1 << 20) + .spawn(|| { + let plus = vec!["1"; 4096].join("+"); + let e = parse(&plus).expect("parse"); + let copy = e.clone(); + assert!(copy == e); + let other = parse(&format!("{}-1", vec!["1"; 4095].join("+"))).expect("parse"); + assert!(other != e); + let mut nodes = 0; + e.walk(&mut |_| nodes += 1); + assert_eq!(nodes, 2 * 4096 - 1); + assert_eq!(print(©.map(&mut |x| x)), plus); + // A chain of references: walk finds them in order, adjust rewrites every one. + let refs = (1..=1000).map(|r| format!("A{r}")).collect::>().join("+"); + let e = parse(&refs).expect("parse"); + let found: Vec = e.references().iter().map(|r| printer::reference_a1(r)).collect(); + assert_eq!(found.first().map(String::as_str), Some("A1")); + assert_eq!(found.last().map(String::as_str), Some("A1000")); + assert_eq!(found.len(), 1000); + let ins = Edit::Insert { axis: Axis::Rows, at: 0, count: 1 }; + let moved = print(&adjust(e.clone(), "S", "S", &ins)); + assert_eq!(moved, (2..=1001).map(|r| format!("A{r}")).collect::>().join("+")); + assert_eq!(print(&shift_relative(e, 0, 1)), (1..=1000).map(|r| format!("B{r}")).collect::>().join("+")); + }) + .expect("spawn"); + handle.join().expect("long chains must not overflow the stack"); +} diff --git a/crates/formula/tests/spanish_input.rs b/crates/formula/tests/spanish_input.rs index fee688a86..8c6053c43 100644 --- a/crates/formula/tests/spanish_input.rs +++ b/crates/formula/tests/spanish_input.rs @@ -1,4 +1,4 @@ -use gridcraft_formula::{FormulaLocale, parse, parse_input, print, print_input}; +use gridcraft_formula::{Expr, FormulaLocale, parse, parse_input, print, print_input}; fn input(src: &str) -> String { print(&parse_input(src, FormulaLocale::Es, |_| false).unwrap()) @@ -45,10 +45,31 @@ fn spanish_input_preserves_reference_and_array_syntax() { assert_eq!(print(&parse("SUMA(1,2)+FALSO").unwrap()), "SUMA(1,2)+FALSO"); assert!(parse("SUM(1;2)").is_err()); assert!(parse("{VERDADERO}").is_err()); - // Pratt parsing can produce a deep left-hand AST without deep parser recursion. - let long_chain = "FALSO+".repeat(300) + "VERDADERO"; - assert!(parse_input(&long_chain, FormulaLocale::Es, |_| false).is_err()); - assert!(print_input(&parse(&long_chain).unwrap(), FormulaLocale::Es, |_| false).is_err()); + // Real nesting past the parser's limit is still refused. + let nested = "(".repeat(300) + "1" + &")".repeat(300); + assert!(parse_input(&nested, FormulaLocale::Es, |_| false).is_err()); + let deep = (0..300).fold(Expr::Number(1.0), |e, _| Expr::Paren(Box::new(e))); + assert!(print_input(&deep, FormulaLocale::Es, |_| false).is_err()); +} + +#[test] +fn spanish_input_accepts_flat_chains_as_long_as_excel() { + // A flat chain nests on its left without nesting in the text. Excel accepts 4,096 + // operands (8,192 characters); Spanish entry refused more than 256 (#916). On a 1 MiB + // stack, as on the Windows main thread. + std::thread::Builder::new() + .stack_size(1 << 20) + .spawn(|| { + let long_chain = "FALSO+".repeat(1360) + "VERDADERO"; + let english = "FALSE+".repeat(1360) + "TRUE"; + assert_eq!(input(&long_chain), english); + assert_eq!(print_input(&parse(&english).unwrap(), FormulaLocale::Es, |_| false).unwrap(), long_chain); + let sums = vec!["SUMA(1;2)"; 800].join("+"); + assert_eq!(input(&sums), vec!["SUM(1,2)"; 800].join("+")); + }) + .unwrap() + .join() + .unwrap(); } #[test] diff --git a/crates/xlsx/src/array_formula.rs b/crates/xlsx/src/array_formula.rs index caa5e8e9f..bafe3ac08 100644 --- a/crates/xlsx/src/array_formula.rs +++ b/crates/xlsx/src/array_formula.rs @@ -22,7 +22,9 @@ impl Shape { pub(crate) fn needs_array(wb: &Workbook, sheet: usize, at: CellRef, formula: &Formula) -> bool { let Some(expr) = formula.expr_arc() else { return false }; - let mut scan = Scan { wb, at, remaining: 4096, locals: Vec::new() }; + // The budget covers every node of a formula as long as Excel allows (8,192 characters make + // at most about 8,200 nodes), so only names that expand a lot fall back to array evaluation. + let mut scan = Scan { wb, at, remaining: 16_384, locals: Vec::new() }; let shape = scan.expr(&expr, sheet, false, 0); // An explicit `@` is written as `_xlfn.SINGLE`, which only dynamic-array formulas carry. let mut at = false; @@ -89,11 +91,27 @@ impl Scan<'_> { } } Expr::Binary(op, a, b) => { - let a = self.expr(a, sheet, array_context, depth + 1); - let b = self.expr(b, sheet, array_context, depth + 1); - let reference = matches!(op, BinOp::Range | BinOp::Union | BinOp::Intersect); - let array = reference || a.array || b.array; - Shape { array, affected: a.affected || b.affected || (!reference && array && !array_context) } + // A flat chain such as A1+A2+… nests one level per operator on its left: follow + // it in a loop, so a long chain is neither deep recursion nor past the depth + // limit (which would write a plain sum as an array formula). + let mut spine = vec![(*op, b.as_ref())]; + let mut left = a.as_ref(); + while let Expr::Binary(op, a, b) = left { + if self.remaining == 0 { + return Shape { array: true, affected: true }; + } + self.remaining -= 1; + spine.push((*op, b.as_ref())); + left = a.as_ref(); + } + let mut acc = self.expr(left, sheet, array_context, depth + 1); + while let Some((op, b)) = spine.pop() { + let b = self.expr(b, sheet, array_context, depth + 1); + let reference = matches!(op, BinOp::Range | BinOp::Union | BinOp::Intersect); + let array = reference || acc.array || b.array; + acc = Shape { array, affected: acc.affected || b.affected || (!reference && array && !array_context) }; + } + acc } Expr::Call(name, args) => self.call(name, args, sheet, array_context, depth + 1), Expr::Invoke(_, _) => Shape { array: true, affected: false }, diff --git a/crates/xlsx/src/fmla.rs b/crates/xlsx/src/fmla.rs index ef263873e..bef43a5aa 100644 --- a/crates/xlsx/src/fmla.rs +++ b/crates/xlsx/src/fmla.rs @@ -30,7 +30,21 @@ fn prefix_params(e: Expr, scope: &[String]) -> Expr { Expr::Name(n) if scope.iter().any(|s| s.eq_ignore_ascii_case(&n)) => Expr::Name(format!("{PARAM_PREFIX}{n}")), Expr::Unary(op, x) => Expr::Unary(op, sub(x, scope)), Expr::Paren(x) => Expr::Paren(sub(x, scope)), - Expr::Binary(op, a, b) => Expr::Binary(op, sub(a, scope), sub(b, scope)), + Expr::Binary(op, a, b) => { + // A long chain such as A1+A2+… nests on its left: follow it in a loop, not one + // level of recursion per operand. + let mut spine = vec![(op, b)]; + let mut left = *a; + while let Expr::Binary(op, a, b) = left { + spine.push((op, b)); + left = *a; + } + let mut acc = prefix_params(left, scope); + while let Some((op, b)) = spine.pop() { + acc = Expr::Binary(op, Box::new(acc), sub(b, scope)); + } + acc + } Expr::Invoke(c, args) => Expr::Invoke(sub(c, scope), args.into_iter().map(|a| prefix_params(a, scope)).collect()), Expr::Call(name, args) if name == "LET" || name == "LAMBDA" => { let last = args.len().saturating_sub(1); diff --git a/crates/xlsx/src/tests/array_export.rs b/crates/xlsx/src/tests/array_export.rs index b90b98668..0dcc9d81c 100644 --- a/crates/xlsx/src/tests/array_export.rs +++ b/crates/xlsx/src/tests/array_export.rs @@ -409,3 +409,28 @@ fn implicit_intersection_metadata_distinguishes_table_rows_and_columns() { &[("Data[@Input]*2", false), ("SUM(Data[Input])", false), ("SUM(Data[Input]*2)", true), ("SQRT(Data[Input])", true)], ); } + +#[test] +fn long_flat_chains_are_scalar_formulas() { + // `=A1+A2+…` nests one level per operator. Past the scan's depth limit of 64, a plain sum + // of 66 cells was written as a dynamic-array formula; Excel writes it as a plain formula. + let mut wb = Workbook::new(); + let sh = wb.sheet_mut(0).unwrap(); + let refs = (1..=1500).map(|r| format!("A{r}")).collect::>().join("+"); + let mixed = (1..=1000).map(|r| format!("A{r}*2")).collect::>().join("-"); + let literals = vec!["1"; 4096].join("&"); + let ranges = format!("{refs}+A1:A3"); + sh.cells.set(at("B1"), formula(&refs, 0.0)); + sh.cells.set(at("B2"), formula(&mixed, 0.0)); + sh.cells.set(at("B3"), formula(&literals, 0.0)); + sh.cells.set(at("B4"), formula(&ranges, 0.0)); + let bytes = write_xlsx(&wb).unwrap(); + let mut pkg = Package::open(&bytes).unwrap(); + let sheet = pkg.read_xml("xl/worksheets/sheet1.xml").unwrap().unwrap(); + for address in ["B1", "B2", "B3"] { + assert_scalar(&sheet, address); + } + // A range anywhere in the chain still needs array evaluation. + assert_dynamic(&sheet, "B4", "B4"); + assert_eq!(cell(&sheet, "B1").child("f").unwrap().text, refs); +} diff --git a/docs/function-parity.md b/docs/function-parity.md index 2343e0de9..babf85630 100644 --- a/docs/function-parity.md +++ b/docs/function-parity.md @@ -1,6 +1,6 @@ # Function parity: worksheet functions and formulas -> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (MEDIAN and AGGREGATE median avoid intermediate overflow) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (flat operator chains as long as Excel allows: 4,096 operands no longer overflow the stack, Spanish entry accepts them, files keep them as plain formulas); previously minor (MEDIAN and AGGREGATE median avoid intermediate overflow) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Excel's worksheet functions, category by category, against GridCraft's function library. Part of [target-app-parity.md](target-app-parity.md); gaps feed [gaps.md](gaps.md). @@ -84,6 +84,7 @@ PIVOTBY, REGEXTEST…) are all present. | 15-significant-digit comparison semantics | **missing** (#57: `=0.1+0.2=0.3` is FALSE) | | External workbook references (`[Book.xlsx]Sheet1!A1`) | parsed; links dropped on open, no update | | Localized function names and `;` separators | **missing** (#45); en-US only | +| Long flat operator chains (`=A1+A2+…`, `-`, `*`, `/`, `^`, `&`, comparisons) | done: no operand limit below Excel's 8,192-character formula limit (Excel, observed: `=1+1+…` with 4,096 operands is exactly 8,192 characters with the `=` and calculates; one more character is refused). Evaluated (#484), printed (#628), stored, copied, adjusted, saved and reopened without recursing once per operand, on a 1 MiB stack (#916); written to XLSX as plain formulas, not dynamic arrays; Spanish entry accepts them | | Error values | the seven classic errors plus `#SPILL!`, `#CALC!`, `#GETTING_DATA` done; `#FIELD!`, `#BLOCKED!`, `#CONNECT!`, `#BUSY!`, `#UNKNOWN!`, `#PYTHON!` missing (minor: they come from data types, cloud and Python features) | ## Depth (estimated) @@ -114,6 +115,7 @@ workbooks; results checked in as data, not Excel files) and fixing what it finds | Date | Change | Summary | |---|---|---| +| 2026-10-14 | minor | Long flat operator chains (#916): 4,096 operands (Excel's 8,192-character limit) no longer overflow the stack when entered, filled, adjusted or saved; chains past 64 operands are no longer saved as dynamic-array formulas; Spanish entry no longer refuses chains past 256 operands | | 2026-10-15 | minor | MEDIAN and AGGREGATE median use an overflow-safe midpoint (#1579); regressions cover finite extremes, tiny values and dependent recalculation | | 2026-10-13 | minor | Clear reports changed stored cells sparsely, including those beyond the first million coordinates; dependent formulas and removed spills update, while clearing empty merges still unblocks spills (#1032) | | 2026-10-12 | minor | USDOLLAR added as a Compatibility function (not on Microsoft's list, so the counts don't change); MIRR with no positive flow is -1, COUNTIF's bare `"<="` / `">="` skip empty strings, INDIRECT with `a1` = FALSE gives `#REF!` for A1-style text (#738) | From b3e0a7e8d372d9b7107de01572b22d7e11aff5b7 Mon Sep 17 00:00:00 2001 From: Harrison Booth Date: Mon, 19 Oct 2026 16:06:47 -0500 Subject: [PATCH 2/5] Formulas: count the leftmost operand of a chain at the same depth The left end of a flat operator chain is not nesting, so (((...1)))+1 is accepted whenever (((...1))) is, in Spanish entry and display. Co-Authored-By: Claude Sonnet 5.5 --- crates/formula/src/input.rs | 2 +- crates/formula/src/tests.rs | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/formula/src/input.rs b/crates/formula/src/input.rs index 7c0fe8b53..38a87cd11 100644 --- a/crates/formula/src/input.rs +++ b/crates/formula/src/input.rs @@ -194,7 +194,7 @@ fn normalize(expr: &mut Expr, known_name: &impl Fn(&str) -> bool, bindings: &mut normalize(right, known_name, bindings, depth + 1, display)?; node = left; } - normalize(node, known_name, bindings, depth + 1, display)?; + normalize(node, known_name, bindings, depth, display)?; } Expr::Array(rows) if display => { for row in rows { diff --git a/crates/formula/src/tests.rs b/crates/formula/src/tests.rs index 185ffdb23..4177fd055 100644 --- a/crates/formula/src/tests.rs +++ b/crates/formula/src/tests.rs @@ -435,3 +435,21 @@ fn longest_flat_chains_clone_compare_walk_and_adjust_without_deep_recursion() { .expect("spawn"); handle.join().expect("long chains must not overflow the stack"); } + +#[test] +fn spanish_input_counts_left_operand_of_a_chain_at_the_same_depth() { + // The leftmost operand of a flat chain is not nested, so `(((…1)))+1` is accepted whenever + // `(((…1)))` is, in both the pre-check and `normalize`. + let wrap = |n: usize| { + let mut e = Expr::Number(1.0); + for _ in 0..n { + e = Expr::Paren(Box::new(e)); + } + e + }; + let es = |e: &Expr| crate::print_input(e, FormulaLocale::Es, |_| false); + let deepest = (0..400).take_while(|&n| es(&wrap(n)).is_ok()).last().expect("depth 0 is accepted"); + assert!(deepest > 1 && deepest < 400); + let chain = Expr::Binary(BinOp::Add, Box::new(wrap(deepest)), Box::new(Expr::Number(1.0))); + assert!(es(&chain).is_ok()); +} From 4ce8dc55ecebd4e50cd31379e0be891a65bc9fa9 Mon Sep 17 00:00:00 2001 From: Harrison Booth Date: Mon, 19 Oct 2026 18:59:35 -0500 Subject: [PATCH 3/5] Docs: date the long-chains revision entry 2026-10-19 Co-Authored-By: Claude Sonnet 5.5 --- docs/function-parity.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/function-parity.md b/docs/function-parity.md index babf85630..c5108dcfd 100644 --- a/docs/function-parity.md +++ b/docs/function-parity.md @@ -1,6 +1,6 @@ # Function parity: worksheet functions and formulas -> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (flat operator chains as long as Excel allows: 4,096 operands no longer overflow the stack, Spanish entry accepts them, files keep them as plain formulas); previously minor (MEDIAN and AGGREGATE median avoid intermediate overflow) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-19 · **Change:** minor (flat operator chains as long as Excel allows: 4,096 operands no longer overflow the stack, Spanish entry accepts them, files keep them as plain formulas); previously minor (MEDIAN and AGGREGATE median avoid intermediate overflow) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Excel's worksheet functions, category by category, against GridCraft's function library. Part of [target-app-parity.md](target-app-parity.md); gaps feed [gaps.md](gaps.md). @@ -115,7 +115,7 @@ workbooks; results checked in as data, not Excel files) and fixing what it finds | Date | Change | Summary | |---|---|---| -| 2026-10-14 | minor | Long flat operator chains (#916): 4,096 operands (Excel's 8,192-character limit) no longer overflow the stack when entered, filled, adjusted or saved; chains past 64 operands are no longer saved as dynamic-array formulas; Spanish entry no longer refuses chains past 256 operands | +| 2026-10-19 | minor | Long flat operator chains (#916): 4,096 operands (Excel's 8,192-character limit) no longer overflow the stack when entered, filled, adjusted or saved; chains past 64 operands are no longer saved as dynamic-array formulas; Spanish entry no longer refuses chains past 256 operands | | 2026-10-15 | minor | MEDIAN and AGGREGATE median use an overflow-safe midpoint (#1579); regressions cover finite extremes, tiny values and dependent recalculation | | 2026-10-13 | minor | Clear reports changed stored cells sparsely, including those beyond the first million coordinates; dependent formulas and removed spills update, while clearing empty merges still unblocks spills (#1032) | | 2026-10-12 | minor | USDOLLAR added as a Compatibility function (not on Microsoft's list, so the counts don't change); MIRR with no positive flow is -1, COUNTIF's bare `"<="` / `">="` skip empty strings, INDIRECT with `a1` = FALSE gives `#REF!` for A1-style text (#738) | From 199e5f8e05260ead5887b20a862172f7e112f7d4 Mon Sep 17 00:00:00 2001 From: Harrison Booth Date: Mon, 19 Oct 2026 20:56:26 -0500 Subject: [PATCH 4/5] Formulas: test the order walk and map visit nodes; date ROADMAP like the parity doc Co-Authored-By: Claude Sonnet 5.5 --- ROADMAP.md | 6 +++--- crates/formula/src/tests.rs | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 2ec3b5a99..e993ae660 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~25 points of "ready for real work" and ~160–240 h away -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-14 · **Change:** minor (long flat operator chains up to Excel's formula limit) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-19 · **Change:** minor (long flat operator chains up to Excel's formula limit) · **Target:** Microsoft Excel (Microsoft 365) GridCraft aims at full Microsoft Excel parity, and at being better in places: faster, open (XLSX is the native format), fully scriptable by agents (MCP, control channel, CLI), and on the web. @@ -132,7 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| -| 2026-10-14 | Long flat operator chains (`=A1+A2+…`) up to Excel's 8,192-character limit (4,096 operands): entering, filling, inserting rows, undo, Evaluate Formula and XLSX save no longer overflow the stack; chains past 64 operands are saved as plain formulas, not dynamic arrays; Spanish formula entry accepts chains past 256 operands (#916) | +| 2026-10-19 | Long flat operator chains (`=A1+A2+…`) up to Excel's 8,192-character limit (4,096 operands): entering, filling, inserting rows, undo, Evaluate Formula and XLSX save no longer overflow the stack; chains past 64 operands are saved as plain formulas, not dynamic arrays; Spanish formula entry accepts chains past 256 operands (#916) | | 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) | | 2026-10-12 | Number formats: parsing a format code is linear in its length (long runs of `[`, `,` or `/` no longer take quadratic time), and an unclosed `[` is a literal when splitting sections as well as when tokenizing (#810) | | 2026-10-12 | Number formats: a fraction denominator with 65,536 zeros no longer crashes; format codes are capped at 255 characters like Excel (longer codes display as General, `TEXT` gives `#VALUE!`) (#807) | @@ -194,7 +194,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | Change | Summary | |---|---|---| -| 2026-10-14 | minor | Progress log: long flat operator chains (#916) | +| 2026-10-19 | minor | Progress log: long flat operator chains (#916) | | 2026-10-12 | minor | Rows auto-fit oversized fonts on open and whole-row/column formatting (#833); progress log row | | 2026-10-12 | minor | Progress log: multi-area row/column delete (#758) | | 2026-10-12 | trivial | Progress log: XLSX chart sheets kept on save (#727) | diff --git a/crates/formula/src/tests.rs b/crates/formula/src/tests.rs index 4177fd055..8c7d85ea7 100644 --- a/crates/formula/src/tests.rs +++ b/crates/formula/src/tests.rs @@ -453,3 +453,20 @@ fn spanish_input_counts_left_operand_of_a_chain_at_the_same_depth() { let chain = Expr::Binary(BinOp::Add, Box::new(wrap(deepest)), Box::new(Expr::Number(1.0))); assert!(es(&chain).is_ok()); } + +#[test] +fn walk_is_preorder_and_map_is_postorder_left_to_right() { + // `(A1+B1)+SUM(C1,D1)-E1`: a chain with a call on its right. walk visits parents before + // children, map rewrites children (left to right) before their parent. + let e = parse("(A1+B1)+SUM(C1,D1)-E1").unwrap(); + let mut walked = Vec::new(); + e.walk(&mut |n| walked.push(print(n))); + assert_eq!(walked, ["(A1+B1)+SUM(C1,D1)-E1", "(A1+B1)+SUM(C1,D1)", "(A1+B1)", "A1+B1", "A1", "B1", "SUM(C1,D1)", "C1", "D1", "E1"]); + let mut mapped = Vec::new(); + let out = e.map(&mut |n| { + mapped.push(print(&n)); + n + }); + assert_eq!(mapped, ["A1", "B1", "A1+B1", "(A1+B1)", "C1", "D1", "SUM(C1,D1)", "(A1+B1)+SUM(C1,D1)", "E1", "(A1+B1)+SUM(C1,D1)-E1"]); + assert_eq!(print(&out), "(A1+B1)+SUM(C1,D1)-E1"); +} From ed48389c46942e2b3783c9ab73eee81a8c276f25 Mon Sep 17 00:00:00 2001 From: Harrison Booth Date: Tue, 20 Oct 2026 08:38:12 -0500 Subject: [PATCH 5/5] Formulas: compare right operands only after the left spine of a chain matches Co-Authored-By: Claude Sonnet 5.5 --- crates/formula/src/ast.rs | 41 ++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/crates/formula/src/ast.rs b/crates/formula/src/ast.rs index 7302b4d04..94756c09f 100644 --- a/crates/formula/src/ast.rs +++ b/crates/formula/src/ast.rs @@ -315,33 +315,38 @@ impl Clone for Expr { impl PartialEq for Expr { fn eq(&self, other: &Self) -> bool { - // Compare the left spines of operator chains in a loop (see [`Expr`]). + // Compare the left spines of operator chains in a loop (see [`Expr`]). The right-hand + // operands are compared only once the whole spine matches, so a mismatch on the left + // returns before any (possibly deep) right operand is walked. let (mut x, mut y) = (self, other); - loop { + let mut right_operands = Vec::new(); + let equal = loop { match (x, y) { (Expr::Binary(o1, a1, b1), Expr::Binary(o2, a2, b2)) => { - if o1 != o2 || b1 != b2 { + if o1 != o2 { return false; } + right_operands.push((b1, b2)); x = a1; y = a2; } - (Expr::Number(a), Expr::Number(b)) => return a == b, - (Expr::Text(a), Expr::Text(b)) => return a == b, - (Expr::Bool(a), Expr::Bool(b)) => return a == b, - (Expr::Error(a), Expr::Error(b)) => return a == b, - (Expr::Array(a), Expr::Array(b)) => return a == b, - (Expr::Ref(a), Expr::Ref(b)) => return a == b, - (Expr::Name(a), Expr::Name(b)) => return a == b, - (Expr::Struct(a), Expr::Struct(b)) => return a == b, - (Expr::Unary(o1, a), Expr::Unary(o2, b)) => return o1 == o2 && a == b, - (Expr::Call(n1, a), Expr::Call(n2, b)) => return n1 == n2 && a == b, - (Expr::Invoke(c1, a), Expr::Invoke(c2, b)) => return c1 == c2 && a == b, - (Expr::Missing, Expr::Missing) => return true, - (Expr::Paren(a), Expr::Paren(b)) => return a == b, - _ => return false, + (Expr::Number(a), Expr::Number(b)) => break a == b, + (Expr::Text(a), Expr::Text(b)) => break a == b, + (Expr::Bool(a), Expr::Bool(b)) => break a == b, + (Expr::Error(a), Expr::Error(b)) => break a == b, + (Expr::Array(a), Expr::Array(b)) => break a == b, + (Expr::Ref(a), Expr::Ref(b)) => break a == b, + (Expr::Name(a), Expr::Name(b)) => break a == b, + (Expr::Struct(a), Expr::Struct(b)) => break a == b, + (Expr::Unary(o1, a), Expr::Unary(o2, b)) => break o1 == o2 && a == b, + (Expr::Call(n1, a), Expr::Call(n2, b)) => break n1 == n2 && a == b, + (Expr::Invoke(c1, a), Expr::Invoke(c2, b)) => break c1 == c2 && a == b, + (Expr::Missing, Expr::Missing) => break true, + (Expr::Paren(a), Expr::Paren(b)) => break a == b, + _ => break false, } - } + }; + equal && right_operands.into_iter().rev().all(|(b1, b2)| b1 == b2) } }