Repository navigation
Define the parser's diagnostic messages once, not in every translation unit - #664
Merged
Merged
Conversation
…n unit diagnostics.h defined its 1967 DiagnosticMessageStore objects `static` at namespace scope. Each holds two std::wstring, so every TU that includes the parser headers (through scanner.h: all of MLIRGen and the passes reached via MLIRTypeHelper.h) got its own copy of the table plus its dynamic initializers, about 0.6 MB of tslang.exe per TU and ~3900 string allocations per TU at startup. The header now declares them `extern` through a TS_DIAGNOSTIC(name, ...) macro, and the new diagnostics.cpp defines TS_DIAGNOSTICS_DEFINE and includes it to emit the single definition. The table stays one source of truth. diagnostics.cpp is added to ts-new-parser-lib, ts-new-scanner and ts-new-parser. The header also gains an include guard. Nothing depends on per-TU identity: messages are compared by `.code`, and none is read by a namespace-scope initializer in another TU, so cross-TU initialization order does not matter. Release tslang.exe (Windows, MSVC): 104,951,808 -> 86,646,272 bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tslang/ts-new-parser/diagnostics.hdefined its 1967DiagnosticMessageStoreobjectsstaticat namespace scope. Each object holds twostd::wstrings. As a result, every translation unit that includes the parser headers got its own copy of the table, plus dynamic initializers for it. Those headers are reached throughscanner.h, which covers all of MLIRGen and everything that includesMLIRTypeHelper.h. The cost was about 0.6 MB oftslang.exeper TU, and every new file split paid it again. The own-passes split, for example, grewtslang.exeby 12 MB for this reason alone.externthrough a variadicTS_DIAGNOSTIC(name, ...)macro. A new file,diagnostics.cpp, definesTS_DIAGNOSTICS_DEFINEand includes the header, which emits the single definition. The table is still the single source of truth.diagnostics.cppis added tots-new-parser-lib,ts-new-scannerandts-new-parser.externwas chosen overinlinebecause the initializers are dynamic. Withinline, every TU would still compile all 1967 initializers, and the result would depend on the linker folding them, which GCC does not do: it keeps a guarded init in every TU.Nothing depends on per-TU identity. Messages are compared by
.code, never by address. They are used only inside function bodies inparser.cppandscanner.cpp. No namespace-scope initializer reads one, so cross-TU init order does not matter.Results
tslang.exe(Windows, MSVC 2026): 104,951,808 → 86,646,272 bytes (−18.3 MB, −17.4%)Test plan
ctest -j 12 -C Release: 100% passed, 5369/5369 (1 disabled, 1 skipped, as before)ts-new-scanner,ts-new-parserandtslangbuild and linklet x = ;→Expression expected.)🤖 Generated with Claude Code