check: find volatile defined names with a worklist instead of repeated passes (#3187)

apps/gridcraft-cli/src/check/calc.rs

@@ -31,20 +31,41 @@ fn is_volatile(e: &Expr, volatile_names: &HashSet<String>) -> bool {
 }
 
 /// Upper-cased defined names whose formula is volatile, directly or through other names.
+///
+/// Volatility flows backwards along name references: each name is parsed and walked once, and a
+/// worklist carries volatility from a name to the names that use it. Rescanning every name until
+/// nothing changes was quadratic: a file listing its names so that each pass marked only one more
+/// did one full pass per name.
 pub(super) fn volatile_names(wb: &Workbook) -> HashSet<String> {
-    let parsed: Vec<(String, Option<Expr>)> =
-        wb.names.iter().map(|n| (n.name.to_ascii_uppercase(), gridcraft_engine::formula::parse(&n.formula).ok())).collect();
     let mut set = HashSet::new();
-    // Each round adds at least one name or stops, so `len + 1` rounds reach the fixed point.
-    for _ in 0..=parsed.len() {
-        let before = set.len();
-        for (name, e) in &parsed {
-            if !set.contains(name) && e.as_ref().is_some_and(|e| is_volatile(e, &set)) {
-                set.insert(name.clone());
+    // Upper-cased name -> the names whose formulas refer to it.
+    let mut users: HashMap<String, Vec<String>> = HashMap::new();
+    let mut queue = Vec::new();
+    let none = HashSet::new();
+    for n in &wb.names {
+        let name = n.name.to_ascii_uppercase();
+        let Ok(e) = gridcraft_engine::formula::parse(&n.formula) else { continue };
+        if is_volatile(&e, &none) {
+            if set.insert(name.clone()) {
+                queue.push(name.clone());
             }
+            continue;
         }
-        if set.len() == before {
-            break;
+        let mut used = BTreeSet::new();
+        e.walk(&mut |x| {
+            if let Expr::Name(r) = x {
+                used.insert(r.to_ascii_uppercase());
+            }
+        });
+        for r in used {
+            users.entry(r).or_default().push(name.clone());
+        }
+    }
+    while let Some(v) = queue.pop() {
+        for user in users.remove(&v).unwrap_or_default() {
+            if set.insert(user.clone()) {
+                queue.push(user);
+            }
         }
     }
     set

apps/gridcraft-cli/src/check/tests.rs

@@ -178,6 +178,28 @@ fn volatile_names_propagate() {
     assert_eq!(check(&wb, &Options::default()).calc.volatile, 1);
 }
 
+#[test]
+fn volatile_names_propagate_through_a_long_chain_in_one_walk() {
+    // Chain0 uses Chain1, Chain1 uses Chain2, ... and only the last is volatile: listed in this order, a rescan
+    // until nothing changes marked one name per pass (quadratic). Every name in the chain is
+    // volatile; a name off the chain is not.
+    const LEN: usize = 5_000;
+    let mut wb = book(&[("A1", "=1", n(1.0))]);
+    let name =
+        |name: String, formula: String| gridcraft_engine::model::DefinedName { name, scope: None, formula, comment: String::new(), hidden: false };
+    for i in 0..LEN {
+        let formula = if i + 1 == LEN { "NOW()".to_string() } else { format!("Chain{}+1", i + 1) };
+        wb.names.push(name(format!("Chain{i}"), formula));
+    }
+    wb.names.push(name("Plain".into(), "Chain0*0+Other".into()));
+    wb.names.push(name("Other".into(), "1".into()));
+    let set = volatile_names(&wb);
+    assert!((0..LEN).all(|i| set.contains(&format!("CHAIN{i}"))));
+    assert!(set.contains("PLAIN"));
+    assert!(!set.contains("OTHER"));
+    assert_eq!(set.len(), LEN + 1);
+}
+
 #[test]
 fn round_trip_of_a_simple_book_is_clean() {
     let mut wb = book(&[("A1", "2", n(2.0)), ("B1", "=A1*3", n(6.0)), ("A2", "x", Value::text("x"))]);