From 9ca5e5a632917fc4d2ba5a568e5de13aec34785e Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Mon, 13 Jul 2026 12:55:08 +0100 Subject: [PATCH] Add series retrospective to MLIRGen refactoring review - section 1 complete Final architecture map (13 TUs), the latent bugs the series fixed, the extraction recipe with its edge cases, and the remaining open item (A7 channel redesign). Co-Authored-By: Claude Fable 5 --- docs/MLIRGen-refactoring-review.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/docs/MLIRGen-refactoring-review.md b/docs/MLIRGen-refactoring-review.md index 8b4d0315..c2116594 100644 --- a/docs/MLIRGen-refactoring-review.md +++ b/docs/MLIRGen-refactoring-review.md @@ -219,3 +219,29 @@ Proposed redesign (one PR per channel, needs review before implementation): 5. **`usingVars`** → same pointer-channel treatment as (1), or fold into the disposal walk's own state. Until a channel lands, its casts stay — documented and greppable via the `NOTE: upward mailbox` markers. + +## Series retrospective — 2026-07-13, §1 complete + +Twenty-seven PRs later (#201–#227), every section of this review except A7 is done. The 26,702-line `MLIRGen.cpp` monolith is now: + +| File | Contents | +|---|---| +| `MLIRGen.cpp` | thin driver: options, diagnostics handler, AST dump entry | +| `MLIRGenImpl.h` | private class definition, ~10.7k lines: small helpers, templates (`mlirGenClassType`, `mlirGenElementAccessTuple`, `mlirGenCallFunction`, `tryInferTupleFields`), RAII scopes (`SourceFileScope`, `TempModuleScope`, `DiscoveryModuleScope`), evaluate machinery, object/array-literal helpers, save/binary-op logic, exports, accessors | +| `MLIRGenModule.cpp` | module/discovery/import driver | +| `MLIRGenStatements.cpp` | statement dispatch + all statement generators | +| `MLIRGenExpressions.cpp` | all expression `mlirGen` overloads incl. literals | +| `MLIRGenFunctions.cpp` | prototypes, bodies, params, captures, generator | +| `MLIRGenClasses.cpp` | class info/members/constructors/vtables/RTTI statics | +| `MLIRGenInterfaces.cpp` | interface/enum declarations, object interface vtables | +| `MLIRGenAccessCall.cpp` | property/element access, call machinery, `new` | +| `MLIRGenVariables.cpp` | variable registration, binding patterns, identifier resolution | +| `MLIRGenGenerics.cpp` | type inference, specialization/instantiation | +| `MLIRGenTypes.cpp` | `getType` family, union/intersection/conditional/mapped | +| `MLIRGenCast.cpp` | the `cast()` pipeline stages and conversions | + +Latent bugs found and fixed by the series (each was invisible in the monolith): the nested-discovery module clobbering (§4a, structural fix), the caller-context `isLoop`/`thisType` leaks (#211), the mapped-type key leak (#210), the inference-loop non-convergence with its load-bearing double-count (#216), dangling `FuncOp` handles after discovery erase (#214–#215), a leaked detached `FuncOp` (#215), and the `GenContext` uninitialized-field/raw-ownership hazards (#202/#204). + +Extraction recipe (for any future family move): select 4-indent signature start lines; consume signatures until a line ends `)` or `) {` (K&R); bodies end at the first `^ \}[ \t]*$` — **whitespace-tolerant, or the state machine overruns**; strip default args from definitions, keep them on declarations; templates stay in the header (check the line *above* the signature for `template`); qualify with `MLIRGenImpl::`; verify with a **multiset reconciliation** (old header vs new header + new TU — git-diff removed-lines gives false losses from hunk realignment); gate with the full ctest suite (~150 s, 683 tests; one known flaky test: `test-compile-00-for-await` under `-j8`). + +Open items: **A7 channel redesign** (the 10 documented upward-mailbox `const_cast`s) awaits review; the header's remaining inline tail is optional polish.