[ZedThree]

The uses of Regex in formatter.rs were not being constructed lazily or with OnceLock, meaning that the cost of constructing them was being paid every single time the containing function was called.

Constructing them lazily gives a significant performance enhancement (~15x in some cases).

Using the lazy-regex crate simplifies the construction of lazy+static regexes, and has some other benefits (compile time checking, for instance).

Improvement is most noticeable on large files (>1k lines), where running ffmt --check can go from ~800ms to ~50ms.

[sbryngelson]

Verified this as behavior-preserving:

• cargo build, cargo test (336 tests), cargo fmt --check, and cargo clippy --all-targets -- -D warnings all clean.
• Mechanically extracted every raw-string regex literal from classifier.rs, formatter.rs, keyword_norm.rs, and whitespace.rs on master vs. this branch and diffed them: identical, no pattern altered by the rewrap.
• Confirmed lazy_regex::regex_replace! is replacen(..., 1) (leftmost-only), matching the old Regex::replace in remove_fypp_macro_paren_space — a regex_replace_all! there would have been a behavior change; it isn't.
• Differential test: release binaries from both branches run over 900 real Fortran files plus all repo fixtures — byte-identical output on every one.
• Builds and passes against the minimum declared lazy-regex 3.3.0 and with the lite feature forced on.

Worth noting the perf angle: this removes several Regex::new(...).unwrap() calls that were recompiling their regex on every call (is_end_procedure, is_procedure_start, is_major_end_block, is_procedure_opener, separate_declarations_from_code), which is a real hot-path win beyond the boilerplate cleanup.

Two non-blocking comments inline, both in keyword_norm.rs; the second one subsumes the first.

[ZedThree]

Thanks @sbryngelson, that's much cleaner! I can also confirm that there's no performance hit from doing it that way either

[Copilot]

🟢 Approval recommended

The migration preserves existing regex behavior while eliminating repeated compilation costs.

Pull request overview

Replaces manual regex construction/caching with compile-time-checked, lazily initialized lazy-regex macros to improve formatter performance.

Changes:

• Migrates regex usage across formatting, classification, keyword, and whitespace logic.
• Replaces the regex dependency with lazy-regex.

File summaries

File	Description
Cargo.toml	Updates the regex dependency.
src/classifier.rs	Migrates classification patterns to lazy macros.
src/formatter.rs	Lazily initializes formatting regexes.
src/keyword_norm.rs	Migrates keyword-normalization patterns.
src/whitespace.rs	Migrates whitespace-normalization patterns.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 0
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sbryngelson]

Nice — verified this locally and it does what it says. Building both branches in release on an M-series Mac:

Behavioral equivalence. I ran both binaries over MFC's src/ (99 .f90/.fpp files). Against the already-formatted tree the output is byte-identical, but that's a weak test since it's a no-op, so I deliberately mangled the corpus first — stripped indentation, ENDDO/endif/ELSEIF, if(, intent (, doubleprecision, goto, collapsed :: spacing — so 98 of 99 files actually get reformatted. Output is still byte-identical between master and this branch. That exercises the classifier, keyword-norm, and whitespace regex paths the PR touches.

Performance. ffmt --check on MFC src/, three runs each, no variance:

	real
master	4.07s
lazy-regex	0.25s

~16x, consistent with what you measured.

Other checks. 47 tests pass; cargo clippy --all-targets -- -D warnings is clean. No regex::Regex or use regex remains in src/ or tests/, so dropping the direct dep is correct (it still arrives transitively via ignore/globset). No MSRV concern — there's no rust-version in Cargo.toml and CI is on dtolnay/rust-toolchain@stable.

The macro swaps read as faithful to me: regex_replace! is Regex::replace (first match only), so remove_fypp_macro_paren_space keeps its (?m)^ first-match behavior, and regex_is_match! is is_match. Replacing the OnceLock<Vec<(Regex, &str)>> in keyword_norm.rs with a plain array of regex! references is a nice simplification — the array holds &'static Regex, so it's pointer copies per call rather than recompilation, and it drops a heap allocation.

Three non-blocking nits:

1. src/whitespace.rs — use lazy_regex::regex; lands mid-file after the OpKind enum rather than with the other imports at the top.
2. Cargo.toml — lazy-regex is appended at the bottom. The dep list wasn't sorted before either, so purely cosmetic.
3. This adds a proc-macro dependency (syn), so clean debug builds get modestly slower. Worth it for the runtime win plus compile-time pattern validation.

None of those block. LGTM.

[sbryngelson]

rebase against master please

[Copilot]

🟡 Changes recommended

A regex_replace! result has the wrong return type, preventing compilation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced