compiler refactor #515

Merged
navicore merged 2 commits from may-refactor into main 2026-05-24 23:15:26 +00:00
Owner

Session recap: all non-test HIGH items in crates/compiler are now audited.
This session covered 20 files:

Refactored (13): error_flag_lint/analyzer.rs, codegen_ops.rs, resolver.rs,
typechecker/control_flow.rs, stack_utils.rs, parser/type_parse.rs,
parser/statements.rs, unification.rs, typechecker/driver.rs, quotations.rs,
typechecker/words.rs, main/lint.rs, ffi/manifest.rs, call_graph.rs,
parser/token.rs, codegen_calls.rs, builtins/list.rs, codegen/types.rs,
virtual_stack.rs, builtins/map.rs — recurring wins were context-structs
(TarjanState), single-source-of-truth helpers (span_line, or_exit,
emit_specialized_call, flush_token, quot promoted to macros.rs), fixing
several misattributed/stale doc comments, and dispatch-table consolidation.

Reviewed clean, no changes (7): types.rs, codegen/state.rs,
builtins/text.rs, and the 6 runtime/*.rs declarative tables
(batch-cleared).

Everything verified green at each step (just lint clean, 388 compiler-lib /
1140 workspace tests passing). The checklist at
tmp/rust-audit-crates-compiler.md reflects all of it, including the
deferred cross-file notes (e.g. Pattern::variant_name(), Parser::expect(),
FlagStack::clear_flags(), the runtime DECLS/SYMBOLS unification).

Session recap: all non-test HIGH items in crates/compiler are now audited. This session covered 20 files: Refactored (13): error_flag_lint/analyzer.rs, codegen_ops.rs, resolver.rs, typechecker/control_flow.rs, stack_utils.rs, parser/type_parse.rs, parser/statements.rs, unification.rs, typechecker/driver.rs, quotations.rs, typechecker/words.rs, main/lint.rs, ffi/manifest.rs, call_graph.rs, parser/token.rs, codegen_calls.rs, builtins/list.rs, codegen/types.rs, virtual_stack.rs, builtins/map.rs — recurring wins were context-structs (TarjanState), single-source-of-truth helpers (span_line, or_exit, emit_specialized_call, flush_token, quot promoted to macros.rs), fixing several misattributed/stale doc comments, and dispatch-table consolidation. Reviewed clean, no changes (7): types.rs, codegen/state.rs, builtins/text.rs, and the 6 runtime/*.rs declarative tables (batch-cleared). Everything verified green at each step (just lint clean, 388 compiler-lib / 1140 workspace tests passing). The checklist at tmp/rust-audit-crates-compiler.md reflects all of it, including the deferred cross-file notes (e.g. Pattern::variant_name(), Parser::expect(), FlagStack::clear_flags(), the runtime DECLS/SYMBOLS unification).
compiler refactor
All checks were successful
CI - Linux / CI - Linux x86_64 (pull_request) Successful in 4m10s
f5ce4291a8
Session recap: all non-test HIGH items in crates/compiler are now audited.
  This session covered 20 files:

  Refactored (13): error_flag_lint/analyzer.rs, codegen_ops.rs, resolver.rs,
  typechecker/control_flow.rs, stack_utils.rs, parser/type_parse.rs,
  parser/statements.rs, unification.rs, typechecker/driver.rs, quotations.rs,
   typechecker/words.rs, main/lint.rs, ffi/manifest.rs, call_graph.rs,
  parser/token.rs, codegen_calls.rs, builtins/list.rs, codegen/types.rs,
  virtual_stack.rs, builtins/map.rs — recurring wins were context-structs
  (TarjanState), single-source-of-truth helpers (span_line, or_exit,
  emit_specialized_call, flush_token, quot promoted to macros.rs), fixing
  several misattributed/stale doc comments, and dispatch-table consolidation.

  Reviewed clean, no changes (7): types.rs, codegen/state.rs,
  builtins/text.rs, and the 6 runtime/*.rs declarative tables
  (batch-cleared).

  Everything verified green at each step (just lint clean, 388 compiler-lib /
   1140 workspace tests passing). The checklist at
  tmp/rust-audit-crates-compiler.md reflects all of it, including the
  deferred cross-file notes (e.g. Pattern::variant_name(), Parser::expect(),
  FlagStack::clear_flags(), the runtime DECLS/SYMBOLS unification).
fix broken benchmark workflows
All checks were successful
CI - Linux / CI - Linux x86_64 (pull_request) Successful in 3m59s
53463002ab
Author
Owner

Review — approve, no blockers

What I verified

  • just ci green on branch HEAD: 547 integration tests pass (same as main — no regression), all 9 CI checks ✓.
  • crates/compiler/src/typechecker/combinators.rs — clean refactor. The 5 quotation-effect dispatch arms (Quotation / Closure / Var fallback / error) consolidated into one extract_quotation_effect helper. Critically, the issue #471 polymorphic_pop calls are preserved at all 5 sites (dip, keep, bi, if-cond, if-combinator).
  • crates/compiler/src/ast/program.rs — wholesale restructure (+448/−453, net −5). Verified the hoisted const BUILTINS: &[&str] is bit-identical to the original embedded list: 273 unique literals on both sides, zero differences.
  • 30 remaining files surveyed: 28 CLEAN / 2 NOTABLE / 0 FLAG.

Two NOTABLE items (neither a blocker, both deliberate)

  1. builtins/macros.rs ty! catch-all. Enumerated arms for T, U, V, …, Acc replaced with a single $v:ident => Type::Var(stringify!($v)). Current call sites unaffected; future typos like ty!(Bol) now silently become a type variable instead of a macro error. Surface area is small (internal, only called inside builtin!), but the compile-time guard is gone — worth a deliberate yes.

  2. codegen/specialization/codegen_word.rs emit_branch consolidation. Then/else duplication folded into one helper; else_branch.unwrap_or(&[]) collapses the old is_none() || is_empty() guard into a single is_empty() check on the slice. Semantically equivalent on inspection.

Deferred follow-ups noted in the PR description

Confirmed not addressed in this PR (and shouldn't be — out of scope):

  • Pattern::variant_name()
  • Parser::expect()
  • FlagStack::clear_flags()
  • runtime DECLS/SYMBOLS unification

The ArmInfo struct introduced in codegen/control_flow.rs is the natural home for a future Pattern::variant_name() extraction.

Other notable wins worth calling out

  • TarjanState context-struct in call_graph.rs with accurate .expect("Tarjan invariant: …") messages — better failure mode than the prior .unwrap()s.
  • span_line helper deduping 5 copies in typechecker/control_flow.rs.
  • flush_token in parser/token.rs (3 sites folded).
  • or_exit in main/lint.rs (5 match-and-die blocks deduped).
  • pop_actual_capture_types extraction in typechecker/quotations.rs with the load-bearing "don't unify, use actual types" invariant preserved verbatim.

Net diff: +1342 / −1599 — a refactor that removes code. Exactly the right shape.

## Review — approve, no blockers ### What I verified - `just ci` green on branch HEAD: 547 integration tests pass (same as `main` — no regression), all 9 CI checks ✓. - `crates/compiler/src/typechecker/combinators.rs` — clean refactor. The 5 quotation-effect dispatch arms (`Quotation` / `Closure` / `Var` fallback / error) consolidated into one `extract_quotation_effect` helper. Critically, the issue #471 `polymorphic_pop` calls are preserved at all 5 sites (dip, keep, bi, if-cond, if-combinator). - `crates/compiler/src/ast/program.rs` — wholesale restructure (+448/−453, net −5). Verified the hoisted `const BUILTINS: &[&str]` is **bit-identical to the original embedded list**: 273 unique literals on both sides, zero differences. - 30 remaining files surveyed: **28 CLEAN / 2 NOTABLE / 0 FLAG**. ### Two NOTABLE items (neither a blocker, both deliberate) 1. **`builtins/macros.rs` `ty!` catch-all.** Enumerated arms for `T, U, V, …, Acc` replaced with a single `$v:ident => Type::Var(stringify!($v))`. Current call sites unaffected; future typos like `ty!(Bol)` now silently become a type variable instead of a macro error. Surface area is small (internal, only called inside `builtin!`), but the compile-time guard is gone — worth a deliberate yes. 2. **`codegen/specialization/codegen_word.rs` `emit_branch` consolidation.** Then/else duplication folded into one helper; `else_branch.unwrap_or(&[])` collapses the old `is_none() || is_empty()` guard into a single `is_empty()` check on the slice. Semantically equivalent on inspection. ### Deferred follow-ups noted in the PR description Confirmed *not* addressed in this PR (and shouldn't be — out of scope): - `Pattern::variant_name()` - `Parser::expect()` - `FlagStack::clear_flags()` - runtime `DECLS/SYMBOLS` unification The `ArmInfo` struct introduced in `codegen/control_flow.rs` is the natural home for a future `Pattern::variant_name()` extraction. ### Other notable wins worth calling out - `TarjanState` context-struct in `call_graph.rs` with accurate `.expect("Tarjan invariant: …")` messages — better failure mode than the prior `.unwrap()`s. - `span_line` helper deduping 5 copies in `typechecker/control_flow.rs`. - `flush_token` in `parser/token.rs` (3 sites folded). - `or_exit` in `main/lint.rs` (5 match-and-die blocks deduped). - `pop_actual_capture_types` extraction in `typechecker/quotations.rs` with the load-bearing "don't unify, use actual types" invariant preserved verbatim. Net diff: +1342 / −1599 — a refactor that removes code. Exactly the right shape.
navicore deleted branch may-refactor 2026-05-24 23:15:26 +00:00
Sign in to join this conversation.
No description provided.