-
Notifications
You must be signed in to change notification settings - Fork 469
[SwiftWarningControl] Add support for file-scoped 'using @warn' #3211
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
base: main
Are you sure you want to change the base?
Conversation
ahoppen
left a comment
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.
One high-level comment before I dive into a detailed review: Is using @warn only allowed at the top level or also inside declarations?
- If it is only allowed at the top level: We could still have an API that takes a position and just doesn’t walk into declarations
- If it is allowed inside type declarations: Does the
usingdeclaration only apply within that type, similar to applying it as an attribute to the type or does it still apply to the whole file?
77948d9 to
82e9d6c
Compare
This is correct, |
These declarations may appear as top-level statements and affect the entire source file.
82e9d6c to
f7a1d36
Compare
|
@swift-ci test |
ahoppen
left a comment
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.
Could you add a test case that contains using @warn inside a declaration to make sure that it is ignored / diagnosed / handled in a way that makes sense to you?
| let (markerLocations, source) = extractMarkers(markedSource) | ||
|
|
||
| var parser = Parser(source) | ||
| let experimentalFeatures = experimentalFeatures ?? Parser.ExperimentalFeatures(rawValue: 0) |
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.
| let experimentalFeatures = experimentalFeatures ?? Parser.ExperimentalFeatures(rawValue: 0) | |
| let experimentalFeatures = experimentalFeatures ?? [] |
using @warn(DiagGroupID, as: warning)specifies a file-scoped diagnostic group control. This change adds detection of these controls and upon encountering one, adds a corresponding control to the file-scoped root of the warning control region tree.
This change also removes the optimization on
warningGroupControlRegionTreeImplconstruction which only constructs a tree that considers nodes which contain a queried location because we now have a scenario where diagnostic group behavior at a specific location can be affected by anusing @warnnode elsewhere in the file. I expect clients to run queries against a once-built tree for the source file, so I do not think this will have a meaningful impact, in practice.