Skip to content

Split MLIRGen.cpp: extract MLIRGenImpl into private header, move cast family to MLIRGenCast.cpp (review doc 1)#217

Merged
ASDAlexander77 merged 1 commit into
mainfrom
refactor/mlirgen-tu-split-cast
Jul 12, 2026
Merged

Split MLIRGen.cpp: extract MLIRGenImpl into private header, move cast family to MLIRGenCast.cpp (review doc 1)#217
ASDAlexander77 merged 1 commit into
mainfrom
refactor/mlirgen-tu-split-cast

Conversation

@ASDAlexander77

Copy link
Copy Markdown
Owner

First mechanical slice of the §1 TU split from docs/MLIRGen-refactoring-review.md.

What

  • MLIRGenImpl.h (new, private to lib/TypeScript): the whole MLIRGenImpl class moved out of MLIRGen.cpp's anonymous namespace into typescript::mlirgen. Bodies remain inline for now; each following slice replaces one family's inline bodies with declarations and moves the definitions into a new TU.
  • MLIRGenCast.cpp (new): first satellite TU — the contiguous cast family (selectFieldsValuescreateBoundMethodFromExtensionMethod, 29 methods, ~1,600 lines) defined out-of-line. Default arguments stay on the declarations only.
  • MLIRGen.cpp shrinks to 101 lines: compileOptionsPtr static + getCompileOptions()/new setCompileOptions(), SourceMgrDiagnosticHandlerEx methods, and the two public entry points.

Enabling fixes (single-TU assumptions the split exposed)

  • GenContext/PassResult/ValueOrLogicalResult were defined in an anonymous namespace inside a header (MLIRGenContext.h), so every TU got its own distinct types — fine with one TU, an ODR/link break with two. They now live in namespace typescript (the header already had using namespace ::typescript; above, so all unqualified uses keep working).
  • ts::print(Node) in ts-new-parser/dump.h was a non-inline function defined in a header → multiply defined; now inline.
  • The constructor no longer touches the TU-static compileOptionsPtr directly (inline code referencing a TU-static would be an ODR trap); it calls setCompileOptions() defined in MLIRGen.cpp.

Verification

  • Debug build clean (the earlier C5046 internal-linkage warning is gone).
  • MLIRGenTests 12/12.
  • Full ctest suite: JIT 339/339, AOT 342/342.

🤖 Generated with Claude Code

… family to MLIRGenCast.cpp (review doc 1)

Mechanical first slice of the TU split: MLIRGenImpl moves from
MLIRGen.cpp's anonymous namespace into typescript::mlirgen in the
private header MLIRGenImpl.h (bodies still inline; later slices move
them out). MLIRGenCast.cpp is the first satellite TU with the 29
cast-family methods defined out-of-line.

Enabling fixes: GenContext/PassResult/ValueOrLogicalResult had internal
linkage (anonymous namespace in MLIRGenContext.h) and now live in
namespace typescript; ts::print(Node) in dump.h is now inline;
compileOptionsPtr stays TU-local behind setCompileOptions().

Verified: MLIRGenTests 12/12, ctest JIT 339/339, AOT 342/342.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant