diff --git a/ROADMAP.md b/ROADMAP.md index e8e1c06..5046fc4 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -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-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 | File › Print selects a system printer, copies, duplex and grayscale, submits PDF jobs through craft-print on a worker and shows spooler errors; browser printing and PDF export remain available | | 2026-10-11 | Command-sequence fuzzing: random command sequences checked against undo/redo, XLSX round-trip and full recalculation; fixes for the table, legacy-array, spill, AutoSum, Watch Window and sheet-copy bugs it found | | 2026-10-11 | Password-protected workbooks open: GridCraft asks for the password and decrypts Agile and Standard encryption (#11) | diff --git a/crates/engine/src/display.rs b/crates/engine/src/display.rs index db7e7c8..b90e427 100644 --- a/crates/engine/src/display.rs +++ b/crates/engine/src/display.rs @@ -10,11 +10,13 @@ use gridcraft_numfmt::{Formatted, NumberFormat, format_value}; /// Parsed number formats are cached by code. fn parsed(code: &str) -> NumberFormat { static CACHE: std::sync::LazyLock>> = std::sync::LazyLock::new(|| Mutex::new(HashMap::new())); - let mut m = CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - if let Some(f) = m.get(code) { + let lock = || CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + if let Some(f) = lock().get(code) { return f.clone(); } + // Parse outside the lock, so a slow code never blocks the other threads' lookups. let f = NumberFormat::parse(code); + let mut m = lock(); if m.len() > 4096 { m.clear(); } diff --git a/crates/numfmt/src/parse.rs b/crates/numfmt/src/parse.rs index e54f04c..8d60000 100644 --- a/crates/numfmt/src/parse.rs +++ b/crates/numfmt/src/parse.rs @@ -125,12 +125,20 @@ pub(crate) struct Section { } /// Splits a code into sections at unquoted, unescaped, unbracketed semicolons. +/// +/// A `[` opens a bracket only when a `]` follows it; the bracket then runs to the next `]` (quotes +/// inside it are not special), exactly as `tokenize` reads it. An unclosed `[` is a literal in both +/// passes, so a later `;` still separates sections. Linear: each `[` either jumps over its bracket +/// or, with no `]` left in the code, is a literal at once. fn split_sections(code: &str) -> Vec { + let cs: Vec = code.chars().collect(); + let last_close = cs.iter().rposition(|&c| c == ']'); let mut out = vec![String::new()]; let mut quoted = false; - let mut bracket = false; let mut escape = false; - for c in code.chars() { + let mut i = 0usize; + while let Some(&c) = cs.get(i) { + i += 1; let cur = match out.last_mut() { Some(c) => c, None => break, @@ -145,19 +153,22 @@ fn split_sections(code: &str) -> Vec { quoted = !quoted; cur.push(c); } - '\\' | '_' | '*' if !quoted && !bracket => { + '\\' | '_' | '*' if !quoted => { escape = true; cur.push(c); } - '[' if !quoted => { - bracket = true; + '[' if !quoted && last_close.is_some_and(|l| l >= i) => { + // `i` is already past the `[`; the closing bracket is at or after it. cur.push(c); + while let Some(&d) = cs.get(i) { + i += 1; + cur.push(d); + if d == ']' { + break; + } + } } - ']' if !quoted => { - bracket = false; - cur.push(c); - } - ';' if !quoted && !bracket => { + ';' if !quoted => { if out.len() >= MAX_SECTIONS { break; } @@ -246,6 +257,8 @@ fn tokenize(src: &str) -> Raw { let cs: Vec = src.chars().collect(); let mut raw = Raw { toks: Vec::new(), color: None, cond: None, currency: false }; let toks = &mut raw.toks; + // With no `]` at or after a `[`, that `[` is a literal; checking this once keeps the scan linear. + let last_close = cs.iter().rposition(|&c| c == ']'); let mut i = 0usize; while let Some(&c) = cs.get(i) { match c { @@ -290,7 +303,7 @@ fn tokenize(src: &str) -> Raw { continue; } '[' => { - let close = cs.get(i + 1..).and_then(|r| r.iter().position(|&d| d == ']')); + let close = if last_close.is_some_and(|l| l > i) { cs.get(i + 1..).and_then(|r| r.iter().position(|&d| d == ']')) } else { None }; if let Some(p) = close { let content: String = cs.get(i + 1..i + 1 + p).unwrap_or(&[]).iter().collect(); bracket(&mut raw_parts(toks, &mut raw.color, &mut raw.cond, &mut raw.currency), &content); @@ -513,6 +526,19 @@ fn analyze(raw: Raw) -> Section { let mut seen_exp = false; let mut seen_digit = false; let mut fraction_slash = false; + // Precomputed so each comma and slash is O(1) (a long run of them stays linear): whether the + // first non-comma token after a position is a digit, and where the last exponent marker is. + let mut digit_after_commas = vec![false; toks.len()]; + let mut after = false; + for (j, t) in toks.iter().enumerate().rev() { + if let Some(slot) = digit_after_commas.get_mut(j) { + *slot = after; + } + if !matches!(t, Tok::Comma) { + after = matches!(t, Tok::Digit(_)); + } + } + let last_exp = toks.iter().rposition(|t| matches!(t, Tok::Exp { .. })); for i in 0..toks.len() { let Some(t) = toks.get(i).cloned() else { break }; let replace = match t { @@ -542,8 +568,7 @@ fn analyze(raw: Raw) -> Section { } } Tok::Comma => { - let next = toks.get(i + 1..).and_then(|s| s.iter().find(|t| !matches!(t, Tok::Comma))); - let next_digit = matches!(next, Some(Tok::Digit(_))); + let next_digit = digit_after_commas.get(i).copied().unwrap_or(false); if seen_digit && next_digit && !seen_point && !seen_exp { sec.thousands = true; Some(Tok::Lit(String::new())) @@ -558,7 +583,8 @@ fn analyze(raw: Raw) -> Section { // Literal text may sit between the bar and the denominator (`#/ #`, `# ?=/=?`). let next_ok = matches!(toks.get(i + 1), Some(Tok::FixedDenom(_))) || matches!(toks.get(i + 1..).and_then(|s| s.iter().find(|t| !matches!(t, Tok::Lit(_)))), Some(Tok::Digit(_))); - let has_exp = toks.iter().any(|t| matches!(t, Tok::Exp { .. })); + // An exponent kept before this bar (`seen_exp`) or any marker still ahead of it. + let has_exp = seen_exp || last_exp.is_some_and(|l| l > i); if !fraction_slash && seen_digit && next_ok && !seen_point && !has_exp { fraction_slash = true; sec.fraction = true; diff --git a/crates/numfmt/src/tests.rs b/crates/numfmt/src/tests.rs index 6c2306c..8021a19 100644 --- a/crates/numfmt/src/tests.rs +++ b/crates/numfmt/src/tests.rs @@ -582,3 +582,48 @@ fn decimal_digits_keeps_fifteen_digits_and_truncates_a_tie() { let (d, e) = decimal_digits(999999999999999.6); assert_eq!((d, e), ([1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 15)); } + +/// Parses and renders `code` on a worker thread; panics if that takes more than 20 s, so a quadratic +/// parser fails the test instead of hanging CI. Not a benchmark: at this size an O(n^2) pass takes +/// minutes and an O(n) pass milliseconds, so the margin is decades wide. +fn finishes_in_time(code: String) -> String { + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let _ = tx.send(f(&code, 1234.5)); + }); + rx.recv_timeout(std::time::Duration::from_secs(20)).expect("format code did not parse and render in time") +} + +#[test] +fn hostile_codes_parse_in_linear_time() { + let n = 1_000_000; + // Unclosed brackets, a run of commas after a digit, a run of fraction bars. + assert!(finishes_in_time("[".repeat(n)).starts_with('[')); + finishes_in_time(format!("0{}", ",".repeat(n))); + finishes_in_time(format!("0{}", "/".repeat(n))); + finishes_in_time(format!("0{}0", ",".repeat(n))); + // Unclosed brackets mixed with semicolons: each is a literal and a section separator stays one. + finishes_in_time("[;".repeat(n / 2)); +} + +#[test] +fn unclosed_bracket_is_a_literal_in_both_passes() { + // Sections still split after an unclosed `[`. + assert_eq!(f("[0;0", 5.0), "[5"); + assert_eq!(f("0;[0", -5.0), "[5"); + assert_eq!(f("0;-0;[", 0.0), "["); + assert_eq!(parse::parse_sections("0;[Red-0").len(), 2); + assert_eq!(parse::parse_sections("[0;0").len(), 2); +} + +#[test] +fn closed_brackets_still_hold_their_semicolons() { + assert_eq!(f("[Red]0;[Blue]-0", -5.0), "-5"); + assert_eq!(f("[>=100]0.0;0", 150.0), "150.0"); + assert_eq!(f("[$-409]0.00", 2.5), "2.50"); + assert_eq!(f("[$€-2] #,##0", 1234.0), "€ 1,234"); + assert_eq!(parse::parse_sections("[Red]0;[Blue]0").len(), 2); + assert_eq!(parse::parse_sections("[unknown;x]0;0").len(), 2); + assert_eq!(f("0\\[0", 5.0), "0[5"); + assert_eq!(f("\"[\"0;0", 5.0), "[5"); +}