From 632e4a1bc51ee86b3f0a55e95ee78def391aab30 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 22:57:52 -0500 Subject: [PATCH] formula, calc: LET/LAMBDA names looked up through a map; ragged array literals refused before reserving (#3312) --- crates/calc/src/eval.rs | 5 ++++ crates/formula/src/input.rs | 48 ++++++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 7a957f3a4..7f10354da 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -325,6 +325,11 @@ impl<'h> Evaluator<'h> { Expr::Array(rows) => { let h = rows.len(); let w = rows.first().map_or(0, Vec::len); + // A ragged literal is an error either way; one wide row and many empty ones must + // not reserve their rows-times-width product first. + if rows.iter().any(|row| row.len() != w) { + return err(CellError::Value); + } let mut data = Vec::with_capacity(h * w); for row in rows { for el in row { diff --git a/crates/formula/src/input.rs b/crates/formula/src/input.rs index 1c8929739..6d95587a7 100644 --- a/crates/formula/src/input.rs +++ b/crates/formula/src/input.rs @@ -61,7 +61,7 @@ impl FormulaLocale { pub fn parse_input(src: &str, locale: FormulaLocale, known_name: impl Fn(&str) -> bool) -> Result { let mut expr = parser::parse_syntax(src, locale)?; if locale == FormulaLocale::Es { - normalize(&mut expr, &known_name, &mut Vec::new(), 0, false)?; + normalize(&mut expr, &known_name, &mut Bindings::default(), 0, false)?; } Ok(expr) } @@ -96,7 +96,7 @@ pub fn print_input(expr: &Expr, locale: FormulaLocale, known_name: impl Fn(&str) return Ok(crate::print(expr)); } let mut expr = expr.clone(); - normalize(&mut expr, &known_name, &mut Vec::new(), 0, true)?; + normalize(&mut expr, &known_name, &mut Bindings::default(), 0, true)?; let canonical = crate::print(&expr); let tokens = crate::lexer::tokenize(&canonical).map_err(|e| ParseError { msg: e.msg, pos: e.pos })?; let mut out = String::new(); @@ -131,11 +131,47 @@ pub(crate) fn boolean_alias(name: &str) -> Option { } } -fn normalize(expr: &mut Expr, known_name: &impl Fn(&str) -> bool, bindings: &mut Vec, depth: usize, display: bool) -> Result<(), ParseError> { +/// The LET/LAMBDA names in scope, with how many times each is bound, so a lookup is one probe +/// instead of a scan of every binding for each call. +#[derive(Default)] +struct Bindings { + stack: Vec, + counts: std::collections::HashMap, +} + +impl Bindings { + fn len(&self) -> usize { + self.stack.len() + } + + fn push(&mut self, name: &str) { + let key = crate::fold_name(name); + *self.counts.entry(key.clone()).or_default() += 1; + self.stack.push(key); + } + + fn truncate(&mut self, len: usize) { + while self.stack.len() > len { + let Some(key) = self.stack.pop() else { break }; + if let Some(n) = self.counts.get_mut(&key) { + *n -= 1; + if *n == 0 { + self.counts.remove(&key); + } + } + } + } + + fn contains(&self, name: &str) -> bool { + self.counts.contains_key(&crate::fold_name(name)) + } +} + +fn normalize(expr: &mut Expr, known_name: &impl Fn(&str) -> bool, bindings: &mut Bindings, depth: usize, display: bool) -> Result<(), ParseError> { if depth > parser::MAX_DEPTH { return Err(ParseError { msg: "formula is nested too deeply".into(), pos: 0 }); } - let bound = |name: &str, bindings: &[String]| bindings.iter().rev().any(|n| crate::same_name(n, name)) || known_name(name); + let bound = |name: &str, bindings: &Bindings| bindings.contains(name) || known_name(name); match expr { Expr::Bool(value) if display => { let alias = if *value { "VERDADERO" } else { "FALSO" }; @@ -156,7 +192,7 @@ fn normalize(expr: &mut Expr, known_name: &impl Fn(&str) -> bool, bindings: &mut // The new name is not in scope in its own value expression. normalize(value, known_name, bindings, depth + 1, display)?; if let Expr::Name(name) = declaration { - bindings.push(name.clone()); + bindings.push(name); } } } @@ -169,7 +205,7 @@ fn normalize(expr: &mut Expr, known_name: &impl Fn(&str) -> bool, bindings: &mut if let Some((body, params)) = args.split_last_mut() { for param in params { if let Expr::Name(name) = param { - bindings.push(name.clone()); + bindings.push(name); } } normalize(body, known_name, bindings, depth + 1, display)?;