Skip to content

Conversation

@artemcm
Copy link
Contributor

@artemcm artemcm commented Dec 15, 2025

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 warningGroupControlRegionTreeImpl construction 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 an using @warn node 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.

Copy link
Member

@ahoppen ahoppen left a 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 using declaration only apply within that type, similar to applying it as an attribute to the type or does it still apply to the whole file?

@artemcm artemcm force-pushed the WarningControlUsing branch from 77948d9 to 82e9d6c Compare December 17, 2025 17:43
@artemcm
Copy link
Contributor Author

artemcm commented Dec 17, 2025

  • 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

This is correct, using @warn is only allowed at the top level. I've restored the position API and changed the logic to specifically skip Declarations which do not contain the specified position and it seems to work well. Thanks.

These declarations may appear as top-level statements and affect the entire source file.
@artemcm artemcm force-pushed the WarningControlUsing branch from 82e9d6c to f7a1d36 Compare December 17, 2025 18:48
@artemcm
Copy link
Contributor Author

artemcm commented Dec 17, 2025

@swift-ci test

Copy link
Member

@ahoppen ahoppen left a 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let experimentalFeatures = experimentalFeatures ?? Parser.ExperimentalFeatures(rawValue: 0)
let experimentalFeatures = experimentalFeatures ?? []

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.

2 participants