From 5a84d9bed8e78db3c01a381fa0e33310e4b63f89 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 28 Oct 2026 10:52:53 +0000 Subject: [PATCH] Refactor calc for strict Clippy checks --- crates/calc/src/eval.rs | 2 +- crates/calc/src/eval/array.rs | 2 +- crates/calc/src/eval/call.rs | 305 +++++++++++++------------- crates/calc/src/eval/format.rs | 3 +- crates/calc/src/eval/lambda.rs | 4 +- crates/calc/src/eval/reference_fns.rs | 4 +- crates/calc/src/eval/special.rs | 6 +- crates/calc/src/recalc.rs | 217 +++++++++--------- crates/calc/src/recalc/graph.rs | 11 +- crates/calc/src/recalc/pass_host.rs | 4 +- 10 files changed, 289 insertions(+), 269 deletions(-) diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 45d81ccca..4972623fc 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -7,7 +7,7 @@ use std::sync::Arc; use gridcraft_core::{Array, CellError, CellRef, MAX_COLS, MAX_ROWS, RangeRef, Value, compare}; -use gridcraft_formula::{BinOp, Expr, RefKind, Reference, SheetSel, StructItem, StructRef, UnOp}; +use gridcraft_formula::{BinOp, Expr, Reference, SheetSel, StructItem, StructRef, UnOp}; use gridcraft_functions::{Arg, Ctx}; use gridcraft_model::Workbook; diff --git a/crates/calc/src/eval/array.rs b/crates/calc/src/eval/array.rs index b17a8c70f..ab6ac1b06 100644 --- a/crates/calc/src/eval/array.rs +++ b/crates/calc/src/eval/array.rs @@ -1,4 +1,4 @@ -use super::*; +use super::{Array, BinOp, CellError, MAX_CELLS, Value, compare}; /// The aggregates that can fold a reference without building an array. #[derive(Clone, Copy, PartialEq, Eq)] diff --git a/crates/calc/src/eval/call.rs b/crates/calc/src/eval/call.rs index 9013ca882..08d563d43 100644 --- a/crates/calc/src/eval/call.rs +++ b/crates/calc/src/eval/call.rs @@ -1,40 +1,13 @@ use super::array::{Acc, Agg, zip2, zip3}; use super::format::same_kind; use super::special::{ev_shape, resized_sum_range, resizes_sum_range}; -use super::*; +use super::{Area, Arg, Array, CellError, Ev, Evaluator, Expr, FnCtx, Lambda, Pipe, Value, compare, err}; +use std::sync::Arc; impl<'h> Evaluator<'h> { pub(super) fn call(&mut self, name: &str, args: &[Expr]) -> Ev { match name { - "IF" => { - if args.is_empty() || args.len() > 3 { - return err(CellError::Value); - } - let c = self.arg_val(args, 0).unwrap_or_default(); - if let Value::Array(arr) = &c { - // Array condition: element-wise. - let t = self.arg_val(args, 1).unwrap_or(Value::Bool(true)); - let f = if args.len() > 2 { self.arg_val(args, 2).unwrap_or(Value::Bool(false)) } else { Value::Bool(false) }; - return Ev::V(zip3(arr, &t, &f, |c, t, f| match c.to_bool() { - Ok(true) => t.clone(), - Ok(false) => f.clone(), - Err(e) => Value::Error(e), - })); - } - match c.to_bool() { - Ok(true) => match args.get(1) { - Some(Expr::Missing) => Ev::V(Value::Number(0.0)), - Some(e) => self.eval(e), - None => Ev::V(Value::Bool(true)), - }, - Ok(false) => match args.get(2) { - Some(Expr::Missing) => Ev::V(Value::Number(0.0)), - Some(e) => self.eval(e), - None => Ev::V(Value::Bool(false)), - }, - Err(e) => err(e), - } - } + "IF" => self.if_call(args), "IFS" => { if args.len() < 2 || !args.len().is_multiple_of(2) { return err(CellError::Value); @@ -50,101 +23,10 @@ impl<'h> Evaluator<'h> { } err(CellError::NA) } - "IFERROR" | "IFNA" => { - if args.len() != 2 { - return err(CellError::Value); - } - let v = self.arg_ev(args, 0).unwrap_or(Ev::V(Value::Empty)); - let na_only = name == "IFNA"; - let catch = |e: CellError| if na_only { e == CellError::NA } else { true }; - match &v { - Ev::V(Value::Error(e)) if catch(*e) => self.eval(&args[1]), - Ev::V(Value::Array(a)) if a.data.iter().any(|x| x.as_error().is_some_and(catch)) => { - let alt = self.arg_val(args, 1).unwrap_or_default(); - let data = a.data.iter().map(|x| if x.as_error().is_some_and(catch) { alt.scalar() } else { x.clone() }).collect(); - Ev::V(Array::new(a.rows, a.cols, data).map(Value::from).unwrap_or(Value::Error(CellError::Value))) - } - Ev::R(_) => { - let val = self.deref(v.clone()); - match val { - Value::Error(e) if catch(e) => self.eval(&args[1]), - Value::Array(ref a) if a.data.iter().any(|x| x.as_error().is_some_and(catch)) => { - let alt = self.arg_val(args, 1).unwrap_or_default(); - let data = a.data.iter().map(|x| if x.as_error().is_some_and(catch) { alt.scalar() } else { x.clone() }).collect(); - Ev::V(Array::new(a.rows, a.cols, data).map(Value::from).unwrap_or(Value::Error(CellError::Value))) - } - _ => v, - } - } - _ => v, - } - } - "CHOOSE" => { - if args.len() < 2 { - return err(CellError::Value); - } - let index = self.value(&args[0]); - if let Value::Array(_) = index { - // An array of indexes picks element by element: `CHOOSE({1,2},A1:A3,B1:B3)` - // is a two-column array. The library CHOOSE lifts over its arguments. - let Some(spec) = gridcraft_functions::lookup(name) else { return err(CellError::Name) }; - let mut evaluated = vec![Arg { value: index, from_ref: false }]; - for a in &args[1..] { - let value = self.value(a); - evaluated.push(Arg { value, from_ref: false }); - } - let mut ctx = FnCtx { ev: self, criteria: None, lambda: None }; - return gridcraft_functions::call(spec, &evaluated, &mut ctx).pipe(Ev::V); - } - let i = match index.to_number() { - Ok(n) => n.trunc() as i64, - Err(e) => return err(e), - }; - if i < 1 || i as usize >= args.len() { - return err(CellError::Value); - } - self.eval(&args[i as usize]) - } - "SWITCH" => { - if args.len() < 3 { - return err(CellError::Value); - } - let v = self.arg_val(args, 0).unwrap_or_default().scalar(); - if let Value::Error(e) = v { - return err(e); - } - let rest = &args[1..]; - let mut i = 0; - while i + 1 < rest.len() { - let cand = self.value(&rest[i]).scalar(); - if !cand.is_error() && compare(&v, &cand) == std::cmp::Ordering::Equal && same_kind(&v, &cand) { - return self.eval(&rest[i + 1]); - } - i += 2; - } - if rest.len() % 2 == 1 { rest.last().map_or(err(CellError::NA), |d| self.eval(d)) } else { err(CellError::NA) } - } - "ROW" | "COLUMN" => { - let is_row = name == "ROW"; - if args.is_empty() || matches!(args.first(), Some(Expr::Missing)) { - return Ev::V(Value::number(if is_row { self.at.row + 1 } else { self.at.col + 1 } as f64)); - } - match self.eval(&args[0]) { - Ev::R(a) => { - let Some(a) = a.first() else { return err(CellError::Ref) }; - let r = self.trim_lines(a); - if is_row { - let rows: Vec = (r.start.row..=r.end.row).map(|x| Value::number(x as f64 + 1.0)).collect(); - if rows.len() == 1 { Ev::V(rows.into_iter().next().unwrap_or_default()) } else { Ev::V(Array::column(rows).into()) } - } else { - let cols: Vec = (r.start.col..=r.end.col).map(|x| Value::number(x as f64 + 1.0)).collect(); - if cols.len() == 1 { Ev::V(cols.into_iter().next().unwrap_or_default()) } else { Ev::V(Array::row(cols).into()) } - } - } - Ev::V(Value::Error(e)) => err(e), - _ => err(CellError::Value), - } - } + "IFERROR" | "IFNA" => self.if_error_call(name, args), + "CHOOSE" => self.choose_call(args), + "SWITCH" => self.switch_call(args), + "ROW" | "COLUMN" => self.row_column_call(name, args), "ROWS" | "COLUMNS" => { if args.len() != 1 { return err(CellError::Value); @@ -180,28 +62,7 @@ impl<'h> Evaluator<'h> { } } } - "SHEET" | "SHEETS" => { - let wb = self.host.workbook(); - if args.is_empty() { - return Ev::V(Value::number(if name == "SHEET" { self.sheet + 1 } else { wb.sheets.len() } as f64)); - } - match self.eval(&args[0]) { - Ev::R(a) => { - if name == "SHEET" { - a.first().map_or(err(CellError::Ref), |a| Ev::V(Value::number(a.sheet as f64 + 1.0))) - } else { - let mut s: Vec = a.iter().map(|x| x.sheet).collect(); - s.dedup(); - Ev::V(Value::number(s.len() as f64)) - } - } - Ev::V(Value::Text(t)) if name == "SHEET" => { - let wb = self.host.workbook(); - wb.sheet_index(&t).map_or(err(CellError::NA), |i| Ev::V(Value::number(i as f64 + 1.0))) - } - _ => err(CellError::NA), - } - } + "SHEET" | "SHEETS" => self.sheet_call(name, args), "OFFSET" => self.offset(args), "INDIRECT" => self.indirect(args), "INDEX" => self.index(args), @@ -273,6 +134,156 @@ impl<'h> Evaluator<'h> { } } + fn if_call(&mut self, args: &[Expr]) -> Ev { + if args.is_empty() || args.len() > 3 { + return err(CellError::Value); + } + let condition = self.arg_val(args, 0).unwrap_or_default(); + if let Value::Array(array) = &condition { + let when_true = self.arg_val(args, 1).unwrap_or(Value::Bool(true)); + let when_false = if args.len() > 2 { self.arg_val(args, 2).unwrap_or(Value::Bool(false)) } else { Value::Bool(false) }; + return Ev::V(zip3(array, &when_true, &when_false, |condition, when_true, when_false| match condition.to_bool() { + Ok(true) => when_true.clone(), + Ok(false) => when_false.clone(), + Err(error) => Value::Error(error), + })); + } + match condition.to_bool() { + Ok(true) => match args.get(1) { + Some(Expr::Missing) => Ev::V(Value::Number(0.0)), + Some(expression) => self.eval(expression), + None => Ev::V(Value::Bool(true)), + }, + Ok(false) => match args.get(2) { + Some(Expr::Missing) => Ev::V(Value::Number(0.0)), + Some(expression) => self.eval(expression), + None => Ev::V(Value::Bool(false)), + }, + Err(error) => err(error), + } + } + + fn if_error_call(&mut self, name: &str, args: &[Expr]) -> Ev { + if args.len() != 2 { + return err(CellError::Value); + } + let value = self.arg_ev(args, 0).unwrap_or(Ev::V(Value::Empty)); + let na_only = name == "IFNA"; + let catches = |error: CellError| !na_only || error == CellError::NA; + match &value { + Ev::V(Value::Error(error)) if catches(*error) => self.eval(&args[1]), + Ev::V(Value::Array(array)) if array.data.iter().any(|item| item.as_error().is_some_and(catches)) => { + let alternate = self.arg_val(args, 1).unwrap_or_default(); + let data = + array.data.iter().map(|item| if item.as_error().is_some_and(catches) { alternate.scalar() } else { item.clone() }).collect(); + Ev::V(Array::new(array.rows, array.cols, data).map(Value::from).unwrap_or(Value::Error(CellError::Value))) + } + Ev::R(_) => self.if_error_reference(value.clone(), args, catches), + _ => value, + } + } + + fn if_error_reference(&mut self, value: Ev, args: &[Expr], catches: impl Fn(CellError) -> bool + Copy) -> Ev { + match self.deref(value.clone()) { + Value::Error(error) if catches(error) => self.eval(&args[1]), + Value::Array(ref array) if array.data.iter().any(|item| item.as_error().is_some_and(catches)) => { + let alternate = self.arg_val(args, 1).unwrap_or_default(); + let data = + array.data.iter().map(|item| if item.as_error().is_some_and(catches) { alternate.scalar() } else { item.clone() }).collect(); + Ev::V(Array::new(array.rows, array.cols, data).map(Value::from).unwrap_or(Value::Error(CellError::Value))) + } + _ => value, + } + } + + fn choose_call(&mut self, args: &[Expr]) -> Ev { + if args.len() < 2 { + return err(CellError::Value); + } + let index = self.value(&args[0]); + if let Value::Array(_) = index { + let Some(spec) = gridcraft_functions::lookup("CHOOSE") else { return err(CellError::Name) }; + let mut evaluated = vec![Arg { value: index, from_ref: false }]; + for argument in &args[1..] { + evaluated.push(Arg { value: self.value(argument), from_ref: false }); + } + let mut context = FnCtx { ev: self, criteria: None, lambda: None }; + return gridcraft_functions::call(spec, &evaluated, &mut context).pipe(Ev::V); + } + let index = match index.to_number() { + Ok(number) => number.trunc() as i64, + Err(error) => return err(error), + }; + if index < 1 || index as usize >= args.len() { + return err(CellError::Value); + } + self.eval(&args[index as usize]) + } + + fn switch_call(&mut self, args: &[Expr]) -> Ev { + if args.len() < 3 { + return err(CellError::Value); + } + let value = self.arg_val(args, 0).unwrap_or_default().scalar(); + if let Value::Error(error) = value { + return err(error); + } + let choices = &args[1..]; + for pair in choices.as_chunks::<2>().0 { + let (candidate, result) = (&pair[0], &pair[1]); + let candidate = self.value(candidate).scalar(); + if !candidate.is_error() && compare(&value, &candidate) == std::cmp::Ordering::Equal && same_kind(&value, &candidate) { + return self.eval(result); + } + } + if let Some(default) = choices.get(choices.len().saturating_sub(1)).filter(|_| choices.len() % 2 == 1) { + self.eval(default) + } else { + err(CellError::NA) + } + } + + fn row_column_call(&mut self, name: &str, args: &[Expr]) -> Ev { + let is_row = name == "ROW"; + if args.is_empty() || matches!(args.first(), Some(Expr::Missing)) { + return Ev::V(Value::number(if is_row { self.at.row + 1 } else { self.at.col + 1 } as f64)); + } + let areas = match self.eval(&args[0]) { + Ev::R(areas) => areas, + Ev::V(Value::Error(error)) => return err(error), + _ => return err(CellError::Value), + }; + let Some(area) = areas.first() else { return err(CellError::Ref) }; + let range = self.trim_lines(area); + if is_row { + let values: Vec = (range.start.row..=range.end.row).map(|row| Value::number(row as f64 + 1.0)).collect(); + if values.len() == 1 { Ev::V(values.into_iter().next().unwrap_or_default()) } else { Ev::V(Array::column(values).into()) } + } else { + let values: Vec = (range.start.col..=range.end.col).map(|col| Value::number(col as f64 + 1.0)).collect(); + if values.len() == 1 { Ev::V(values.into_iter().next().unwrap_or_default()) } else { Ev::V(Array::row(values).into()) } + } + } + + fn sheet_call(&mut self, name: &str, args: &[Expr]) -> Ev { + let is_sheet = name == "SHEET"; + if args.is_empty() { + let count = if is_sheet { self.sheet + 1 } else { self.host.workbook().sheets.len() }; + return Ev::V(Value::number(count as f64)); + } + match self.eval(&args[0]) { + Ev::R(areas) if is_sheet => areas.first().map_or(err(CellError::Ref), |area| Ev::V(Value::number(area.sheet as f64 + 1.0))), + Ev::R(areas) => { + let mut sheets: Vec = areas.iter().map(|area| area.sheet).collect(); + sheets.dedup(); + Ev::V(Value::number(sheets.len() as f64)) + } + Ev::V(Value::Text(name)) if is_sheet => { + self.host.workbook().sheet_index(&name).map_or(err(CellError::NA), |index| Ev::V(Value::number(index as f64 + 1.0))) + } + _ => err(CellError::NA), + } + } + pub(super) fn builtin(&mut self, name: &str, args: &[Expr]) -> Ev { let Some(spec) = gridcraft_functions::lookup(name) else { // A defined name holding a LAMBDA, or a LET-bound lambda. diff --git a/crates/calc/src/eval/format.rs b/crates/calc/src/eval/format.rs index a4c52b492..a288b410e 100644 --- a/crates/calc/src/eval/format.rs +++ b/crates/calc/src/eval/format.rs @@ -1,4 +1,5 @@ -use super::*; +use super::{CellError, Value}; +use gridcraft_core::{CellRef, MAX_COLS, MAX_ROWS}; /// CELL("format") code for a number format. pub(super) fn cell_format_code(code: &str) -> String { diff --git a/crates/calc/src/eval/lambda.rs b/crates/calc/src/eval/lambda.rs index 45f99b488..dfb30029a 100644 --- a/crates/calc/src/eval/lambda.rs +++ b/crates/calc/src/eval/lambda.rs @@ -1,5 +1,7 @@ +use super::MAX_LAMBDA_CALLS; use super::array::{broadcast_len, single}; -use super::*; +use super::{Array, CellError, Ev, Evaluator, Expr, Lambda, MAX_CELLS, Value, err}; +use std::sync::Arc; impl<'h> Evaluator<'h> { pub fn apply_lambda(&mut self, l: &Lambda, args: &[Expr]) -> Ev { diff --git a/crates/calc/src/eval/reference_fns.rs b/crates/calc/src/eval/reference_fns.rs index a21797c23..4f5154c2d 100644 --- a/crates/calc/src/eval/reference_fns.rs +++ b/crates/calc/src/eval/reference_fns.rs @@ -1,5 +1,7 @@ use super::format::{cell_format_code, r1c1_to_a1}; -use super::*; +use super::{Area, Arg, Array, CellError, Ev, Evaluator, Expr, FnCtx, MAX_CELLS, Value, err}; +use gridcraft_core::{CellRef, MAX_COLS, MAX_ROWS, RangeRef}; +use std::sync::Arc; impl<'h> Evaluator<'h> { /// Whole rows/columns trimmed for ROW()/COLUMN() arrays. diff --git a/crates/calc/src/eval/special.rs b/crates/calc/src/eval/special.rs index f68badbfc..0151ea2f9 100644 --- a/crates/calc/src/eval/special.rs +++ b/crates/calc/src/eval/special.rs @@ -1,4 +1,5 @@ -use super::*; +use super::{Area, BinOp, Ev, Expr, Reference, SheetSel, Value, Workbook}; +use gridcraft_core::{CellRef, MAX_COLS, MAX_ROWS, RangeRef}; /// SUMIF and AVERAGEIF with a sum range: Excel sizes the sum range like the criteria range. pub(super) fn resizes_sum_range(name: &str, args: usize) -> bool { @@ -162,6 +163,3 @@ pub const SPECIAL_FUNCTIONS: &[&str] = &[ pub fn is_known_function(n: &str) -> bool { is_special(&n.to_ascii_uppercase()) || gridcraft_functions::lookup(n).is_some() } - -#[allow(dead_code)] -fn unused(_: RefKind) {} diff --git a/crates/calc/src/recalc.rs b/crates/calc/src/recalc.rs index b7f403131..eb11045db 100644 --- a/crates/calc/src/recalc.rs +++ b/crates/calc/src/recalc.rs @@ -10,14 +10,13 @@ //! and the formulas that read one are evaluated again until the loop settles //! (`PassHost::iterate_loops`). -use std::collections::{BTreeSet, VecDeque}; -use std::hash::{BuildHasherDefault, Hasher}; +use std::collections::VecDeque; use gridcraft_core::{Array, CellError, CellRef, RangeRef, Value}; use gridcraft_formula::Expr; use gridcraft_model::{CalcMode, Workbook}; -use crate::eval::{Area, Evaluator, Host, precedents, stack_position}; +use crate::eval::{Evaluator, Host, precedents, stack_position}; mod graph; mod pass_host; @@ -368,105 +367,13 @@ impl Calc { self.circular.retain(|k| !evaluated.contains(k)); } self.circular.extend(cycles); - // Write back. - let mut spill_changes: Vec = Vec::new(); - let mut freed: Vec<(usize, RangeRef)> = Vec::new(); - for (k, v) in results { - let Some(sheet) = wb.sheet_mut(k.0) else { continue }; - if let Some(cell) = sheet.cells.get_mut(k.1) - && cell.value != v - { - cell.value = v; - } - } - for k in &dirty { - let new = spills.get(k); - let Some(sheet) = wb.sheet_mut(k.0) else { continue }; - let old = sheet.spill_ranges.get(&k.1).copied(); - if old.is_none() && new.is_none() { - continue; - } - // An array that spills exactly as before changes nothing its dependents read, so - // they aren't queued again. Otherwise two arrays that read each other's spill areas - // queue each other in every round, up to the round limit. - if let (Some(r), Some(arr)) = (old, new) - && spills_as_before(sheet, k.1, r, arr) - { - continue; - } - if let Some(r) = old { - for c in r.iter() { - sheet.spill.remove(&c); - } - sheet.spill_ranges.remove(&k.1); - } - let mut changed_range = old; - if let Some(arr) = new { - let end = CellRef::new(k.1.row.saturating_add(arr.rows as u32 - 1), k.1.col.saturating_add(arr.cols as u32 - 1)); - let range = RangeRef::new(k.1, end); - let blocked = end.row >= gridcraft_core::MAX_ROWS - || end.col >= gridcraft_core::MAX_COLS - || range.iter().any(|c| c != k.1 && (sheet.cells.has(c) || sheet.spill.contains_key(&c))) - || sheet.merges.iter().any(|m| m.intersects(&range)) - || sheet.tables.iter().any(|t| t.range.contains(k.1)); - if blocked { - if let Some(cell) = sheet.cells.get_mut(k.1) { - cell.value = Value::Error(CellError::Spill); - } - // Not blocked while the pass ran (it would have been #SPILL! there), so - // blocked by another array of this pass: the formulas that read this one, - // at its anchor or over the area it would have spilled into, read #SPILL! - // or empty cells now. - let mut deps = Vec::new(); - self.graph.dependents_of_range(k.0, range, &mut deps); - spill_changes.extend(deps.into_iter().filter(|d| d != k)); - } else { - for (i, c) in range.iter().enumerate() { - if c != k.1 { - sheet.spill.insert(c, arr.data.get(i).cloned().unwrap_or_default()); - } - } - sheet.spill_ranges.insert(k.1, range); - changed_range = Some(changed_range.map_or(range, |o| o.union(&range))); - } - } - if let Some(r) = changed_range { - let mut deps = Vec::new(); - self.graph.dependents_of_range(k.0, r, &mut deps); - // Not the anchor itself (that would read its own array), unless its old array - // is gone or partly freed: its new formula may have read the old values, still - // laid out while it was evaluated. - let freed = match (old, sheet.spill_ranges.get(&k.1)) { - (Some(o), Some(n)) => !(n.contains(o.start) && n.contains(o.end)), - (_, None) => true, - (None, Some(_)) => false, - }; - spill_changes.extend(deps.into_iter().filter(|d| d != k || freed)); - } - if let Some(o) = old - && !sheet.spill_ranges.get(&k.1).is_some_and(|n| n.contains(o.start) && n.contains(o.end)) - { - freed.push((k.0, o)); - } - } + let mut spill_changes = self.write_back(wb, &dirty, results, &spills); // Formulas with references only known while evaluating (INDIRECT, OFFSET) may have read // a spill area before it was laid out: they are evaluated once more. if dynamic_again && !spills.is_empty() { dynamic_again = false; spill_changes.extend(self.graph.dynamic.iter().copied()); } - // An area a formula no longer spills over may unblock another formula's array. - for (si, o) in freed { - for &(s, c) in self.graph.nodes.keys() { - if s == si - && c.row <= o.end.row - && c.col <= o.end.col - && wb.sheet(s).is_some_and(|sh| sh.value(c) == Value::Error(CellError::Spill)) - { - spill_changes.push((s, c)); - } - } - } if full { // Everything was already evaluated once; spill dependents need another pass. } @@ -488,6 +395,105 @@ impl Calc { self.circular.retain(|k| self.graph.nodes.contains_key(k)); } } + + fn write_back( + &mut self, + wb: &mut Workbook, + dirty: &[Key], + results: HashMap, + spills: &HashMap>, + ) -> Vec { + let mut spill_changes = Vec::new(); + let mut freed = Vec::new(); + for (key, value) in results { + let Some(sheet) = wb.sheet_mut(key.0) else { continue }; + if let Some(cell) = sheet.cells.get_mut(key.1) + && cell.value != value + { + cell.value = value; + } + } + for key in dirty { + let new = spills.get(key); + let Some(sheet) = wb.sheet_mut(key.0) else { continue }; + let old = sheet.spill_ranges.get(&key.1).copied(); + if old.is_none() && new.is_none() { + continue; + } + // An unchanged spill does not alter any values its dependents read. + if let (Some(range), Some(array)) = (old, new) + && spills_as_before(sheet, key.1, range, array) + { + continue; + } + if let Some(range) = old { + for cell in range.iter() { + sheet.spill.remove(&cell); + } + sheet.spill_ranges.remove(&key.1); + } + let mut changed_range = old; + if let Some(array) = new { + let end = CellRef::new(key.1.row.saturating_add(array.rows as u32 - 1), key.1.col.saturating_add(array.cols as u32 - 1)); + let range = RangeRef::new(key.1, end); + let blocked = spill_is_blocked(sheet, key.1, range, end); + if blocked { + if let Some(cell) = sheet.cells.get_mut(key.1) { + cell.value = Value::Error(CellError::Spill); + } + let mut blocked_dependents = Vec::new(); + self.graph.dependents_of_range(key.0, range, &mut blocked_dependents); + spill_changes.extend(blocked_dependents.into_iter().filter(|dependent| dependent != key)); + } else { + fill_spill(sheet, key.1, range, array); + changed_range = Some(changed_range.map_or(range, |previous| previous.union(&range))); + } + } + if let Some(range) = changed_range { + let mut dependents = Vec::new(); + self.graph.dependents_of_range(key.0, range, &mut dependents); + let freed_old_range = match (old, sheet.spill_ranges.get(&key.1)) { + (Some(previous), Some(current)) => !(current.contains(previous.start) && current.contains(previous.end)), + (_, None) => true, + (None, Some(_)) => false, + }; + spill_changes.extend(dependents.into_iter().filter(|dependent| dependent != key || freed_old_range)); + } + if let Some(previous) = old + && !sheet.spill_ranges.get(&key.1).is_some_and(|current| current.contains(previous.start) && current.contains(previous.end)) + { + freed.push((key.0, previous)); + } + } + for (sheet_index, range) in freed { + for &(sheet, cell) in self.graph.nodes.keys() { + let was_blocked_cell = cell.row <= range.end.row + && cell.col <= range.end.col + && wb.sheet(sheet).is_some_and(|sheet| sheet.value(cell) == Value::Error(CellError::Spill)); + if sheet == sheet_index && was_blocked_cell { + spill_changes.push((sheet, cell)); + } + } + } + spill_changes + } +} + +fn spill_is_blocked(sheet: &gridcraft_model::Sheet, anchor: CellRef, range: RangeRef, end: CellRef) -> bool { + end.row >= gridcraft_core::MAX_ROWS + || end.col >= gridcraft_core::MAX_COLS + || range.iter().any(|cell| cell != anchor && (sheet.cells.has(cell) || sheet.spill.contains_key(&cell))) + || sheet.merges.iter().any(|merge| merge.intersects(&range)) + || sheet.tables.iter().any(|table| table.range.contains(anchor)) +} + +fn fill_spill(sheet: &mut gridcraft_model::Sheet, anchor: CellRef, range: RangeRef, array: &Array) { + for (index, cell) in range.iter().enumerate() { + if cell != anchor { + sheet.spill.insert(cell, array.data.get(index).cloned().unwrap_or_default()); + } + } + sheet.spill_ranges.insert(anchor, range); } /// Current date-time as a serial (1900 system), from the system clock (browser clock on wasm). @@ -513,21 +519,13 @@ pub fn evaluate_expr(wb: &Workbook, sheet: usize, at: CellRef, expr: &Expr) -> V ev.value(expr) } -#[allow(clippy::disallowed_methods)] // the clock is read only off wasm fn prof_now() -> f64 { - #[cfg(not(target_arch = "wasm32"))] - { - std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map(|d| d.as_secs_f64() * 1000.0).unwrap_or(0.0) - } - #[cfg(target_arch = "wasm32")] - { - 0.0 - } + gridcraft_core::date::unix_now_ms().unwrap_or(0.0) } #[cfg(test)] pub(crate) mod tests { - use std::time::{Duration, Instant}; + use std::time::Duration; use gridcraft_core::{CellRef, RangeRef, Value}; use gridcraft_model::{Cell, Formula, Workbook}; @@ -640,11 +638,10 @@ pub(crate) mod tests { } /// Opening (full recalc), changing every value of a column, and re-entering every formula. - #[allow(clippy::disallowed_methods)] // a native-only timing test: the clock is read off wasm fn open_and_bulk_edit(rows: u32) -> Duration { let mut wb = sum_rows(rows); let mut calc = Calc::new(); - let t = Instant::now(); + let started = gridcraft_core::date::unix_now_ms().unwrap_or(0.0); calc.recalc_all(&mut wb); let values: Vec = (0..rows).map(|r| (0, CellRef::new(r, 4))).collect(); if let Some(sh) = wb.sheet_mut(0) { @@ -655,7 +652,8 @@ pub(crate) mod tests { calc.cells_changed(&mut wb, &values); let formulas: Vec = (0..rows).map(|r| (0, CellRef::new(r, 10))).collect(); calc.cells_changed(&mut wb, &formulas); - let elapsed = t.elapsed(); + let ended = gridcraft_core::date::unix_now_ms().unwrap_or(started); + let elapsed = Duration::from_secs_f64((ended - started).max(0.0) / 1000.0); let last = wb.sheet(0).map(|s| s.value(CellRef::new(rows - 1, 10))); let r = rows - 1; assert_eq!(last, Some(Value::Number(f64::from(1 + (5..10).map(|c| r % 97 + c).sum::())))); @@ -670,7 +668,6 @@ pub(crate) mod tests { #[test] #[ignore = "timing-sensitive; run with --ignored in release"] - #[allow(clippy::disallowed_methods)] fn recalc_with_range_formulas_scales_linearly() { // Each row's SUM reads a range in the same columns. Recalculating on open used to walk // every row per range read (the used range), and finding a cell's dependents scanned diff --git a/crates/calc/src/recalc/graph.rs b/crates/calc/src/recalc/graph.rs index aee4b46d5..ab034f8cc 100644 --- a/crates/calc/src/recalc/graph.rs +++ b/crates/calc/src/recalc/graph.rs @@ -1,4 +1,8 @@ -use super::*; +use super::RangeRef; +use crate::eval::Area; +use gridcraft_core::CellRef; +use std::collections::BTreeSet; +use std::hash::{BuildHasherDefault, Hasher}; /// A fast, non-cryptographic hasher for cell keys (FxHash-style multiply-rotate). #[derive(Default, Clone, Copy)] @@ -179,6 +183,9 @@ pub struct Graph { unsettled_wide: HashSet, } +/// A formula's precedents split by shape: single cells, column intervals and wide ranges. +type Entries = (Vec, Vec<(usize, u32, u32, u32)>, Vec<(usize, RangeRef)>); + impl Graph { pub fn len(&self) -> usize { self.nodes.len() @@ -189,7 +196,7 @@ impl Graph { /// Index entries of a formula's precedents, without duplicates: single cells, column /// intervals and wide ranges. - fn entries(areas: &[Area]) -> (Vec, Vec<(usize, u32, u32, u32)>, Vec<(usize, RangeRef)>) { + fn entries(areas: &[Area]) -> Entries { let (mut cells, mut cols, mut wide) = (Vec::new(), Vec::new(), Vec::new()); for a in areas { if a.range.is_single() { diff --git a/crates/calc/src/recalc/pass_host.rs b/crates/calc/src/recalc/pass_host.rs index 00e92e82b..6aa8a1204 100644 --- a/crates/calc/src/recalc/pass_host.rs +++ b/crates/calc/src/recalc/pass_host.rs @@ -1,5 +1,7 @@ use super::graph::{HashMap, HashSet, Key}; -use super::*; +use super::{Evaluator, Host, Workbook, stack_position}; +use gridcraft_core::{Array, CellError, CellRef, RangeRef, Value}; +use gridcraft_formula::Expr; /// 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