-
Notifications
You must be signed in to change notification settings - Fork 747
Make Program diagnostic API clearer #2197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the Program diagnostic API to simplify diagnostic collection and improve performance. The main changes eliminate redundant binding operations that occurred at multiple levels, now that the system uses a single checker pool instead of multiple checkers.
Key Changes:
- Introduced a new
collectDiagnosticshelper function that centralizes the pattern of collecting diagnostics from one or all files - Removed the complex
getDiagnosticsHelperfunction in favor of simpler, more direct implementations - Added pointer equality fast paths to diagnostic comparison functions to optimize deduplication when bind and semantic diagnostics share the same objects
- Renamed
GetSemanticDiagnosticsNoFiltertoGetSemanticDiagnosticsForFilesfor clarity
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
internal/ast/diagnostic.go |
Added pointer equality fast paths in EqualDiagnostics, EqualDiagnosticsNoRelatedInfo, equalMessageChain, and CompareDiagnostics to optimize deduplication performance |
internal/compiler/program.go |
Major refactoring: introduced collectDiagnostics helper, simplified all diagnostic collection methods, removed redundant binding calls, renamed GetSemanticDiagnosticsNoFilter to GetSemanticDiagnosticsForFiles, and replaced slices.Concat with core.Concatenate |
internal/execute/incremental/program.go |
Updated call from GetSemanticDiagnosticsNoFilter to GetSemanticDiagnosticsForFiles to match the renamed function |
Now that we do not need to ask multiple checkers for their errors anymore, we can restructure the funcs to make a bit more sense.
I've tried to simplify things. Notably:
GetBindDiagnosticswhich is is "special" and is used solely for timingtsc.ForEachCheckerParallelis now limited to non-ProjectPrograms, used for stats/testing.