Numfmt: format-code parsing is linear, and an unclosed [ is treated the same in both passes (#810)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
vangel76 2026-10-12 22:10:35 +02:00
parent fc45b7f6d8
commit a60b49fad0
4 changed files with 90 additions and 16 deletions

View File

@ -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) |

View File

@ -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<Mutex<HashMap<String, NumberFormat>>> = 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();
}

View File

@ -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<String> {
let cs: Vec<char> = 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<String> {
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<char> = 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;

View File

@ -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");
}