Repository navigation
Create declaration emit diagnostic selectors on demand - #64723
Draft
Gavin Kline (gwkline) wants to merge 2 commits into
Draft
Gavin Kline (gwkline) wants to merge 2 commits into
Gavin Kline (gwkline) wants to merge 2 commits into
Conversation
Every declaration visited by the declaration transformer installed a closure that selects the diagnostic to report if one of its symbols turns out to be inaccessible, and returned a second closure to restore the previous context. Almost no visited declaration reports such an error, so the closures were pure allocation overhead: about one in nine allocations of a type check with declaration emit enabled. The context now records the declaration node and builds the selector only when an accessibility error is reported. The handful of sites that need a custom selector keep passing a function. The saved context is a plain struct restored by a method, so the deferred restore no longer allocates. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Each exported declaration infers a type that declaration emit cannot name, so every kind of declaration reports its own accessibility error. The baselines were generated before diagnostic selectors were made lazy and are unchanged by it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Gavin Kline (@gwkline) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
This branch has not been deployed
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.
Fixes #64720
Analysis
Every declaration the declaration transformer visits installs a diagnostic context.
setupDiagnosticContextcallscreateGetSymbolAccessibilityDiagnosticForNode(input), which allocates a closure that selects the error message in case a symbol used by the declaration is inaccessible; the wrapped selectors allocate two. It then returns a second closure that restores the previous context. The selector is only invoked fromhandleSymbolAccessibilityError, that is, when an accessibility error is actually reported, which almost never happens.This runs for every emitted declaration file. Under
noEmitwithdeclarationenabled, it runs for every file's declaration diagnostics. On the generator from the issue the closures are about 40% of all allocations. On a private 8.6k-file monorepo package checked with--noEmitthey were 11%.Fix
SymbolTrackerSharedStateholds asymbolAccessibilityDiagnosticContext{node, fn}instead of a function. The common path stores the declaration node, andgetcreates the selector only when an error is reported. The few sites that need a custom selector store a function as before: default export assignments, classextendsexpressions, dynamic names, and the initialthrowDiagnosticsentinel.setupDiagnosticContextreturns the saved state as a struct, andrestoreDiagnosticContextreinstates it. The deferred restore therefore no longer allocates.The selector is created from the same node with the same inputs, only later, so behaviour is unchanged.
New test
declarationEmitInaccessibleNameContextsgives an unnameable inferred type to every kind of declaration the selectors distinguish. It covers variables, destructuring, function return types and parameters, class properties and static properties, constructor parameter properties, methods and static methods, getters and accessor pairs, namespace members, object literal members and default exports, and reports 13 distinct diagnostics. Its baselines were generated with the old code and are unchanged. No existing baseline changes.Results on an M3 Pro:
noEmit+declaration)--noEmit(8.6k files)The profiling, the patch and this description were produced with Claude Code (Claude Fable 5.1 and Claude Opus 5.5). I have read and understand the change and will handle review myself.
Copilot Checklist
I successfully ran the applicable command at the end of my session, and it completed without error:
🤖 Generated with Claude Code