Skip to content

Commit 9ca5e5a

Browse files
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 <noreply@anthropic.com>
1 parent d981042 commit 9ca5e5a

1 file changed

Lines changed: 26 additions & 0 deletions

File tree

docs/MLIRGen-refactoring-review.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,3 +219,29 @@ Proposed redesign (one PR per channel, needs review before implementation):
219219
5. **`usingVars`** → same pointer-channel treatment as (1), or fold into the disposal walk's own state.
220220
221221
Until a channel lands, its casts stay — documented and greppable via the `NOTE: upward mailbox` markers.
222+
223+
## Series retrospective — 2026-07-13, §1 complete
224+
225+
Twenty-seven PRs later (#201–#227), every section of this review except A7 is done. The 26,702-line `MLIRGen.cpp` monolith is now:
226+
227+
| File | Contents |
228+
|---|---|
229+
| `MLIRGen.cpp` | thin driver: options, diagnostics handler, AST dump entry |
230+
| `MLIRGenImpl.h` | private class definition, ~10.7k lines: small helpers, templates (`mlirGenClassType<T>`, `mlirGenElementAccessTuple<T>`, `mlirGenCallFunction<T>`, `tryInferTupleFields<T>`), RAII scopes (`SourceFileScope`, `TempModuleScope`, `DiscoveryModuleScope`), evaluate machinery, object/array-literal helpers, save/binary-op logic, exports, accessors |
231+
| `MLIRGenModule.cpp` | module/discovery/import driver |
232+
| `MLIRGenStatements.cpp` | statement dispatch + all statement generators |
233+
| `MLIRGenExpressions.cpp` | all expression `mlirGen` overloads incl. literals |
234+
| `MLIRGenFunctions.cpp` | prototypes, bodies, params, captures, generator |
235+
| `MLIRGenClasses.cpp` | class info/members/constructors/vtables/RTTI statics |
236+
| `MLIRGenInterfaces.cpp` | interface/enum declarations, object interface vtables |
237+
| `MLIRGenAccessCall.cpp` | property/element access, call machinery, `new` |
238+
| `MLIRGenVariables.cpp` | variable registration, binding patterns, identifier resolution |
239+
| `MLIRGenGenerics.cpp` | type inference, specialization/instantiation |
240+
| `MLIRGenTypes.cpp` | `getType` family, union/intersection/conditional/mapped |
241+
| `MLIRGenCast.cpp` | the `cast()` pipeline stages and conversions |
242+
243+
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).
244+
245+
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`).
246+
247+
Open items: **A7 channel redesign** (the 10 documented upward-mailbox `const_cast`s) awaits review; the header's remaining inline tail is optional polish.

0 commit comments

Comments
 (0)