[SwiftWarningControl] Add ability to specify diagnostic sub-group links #3183
Conversation
artemcm
requested review from
ahoppen,
bnbarham,
hamishknight and
rintaro
as code owners
|
@swift-ci test |
ahoppen
left a comment
ahoppen
left a comment
There was a problem hiding this comment.
How large do you envision subGroupLinks to be? Just thinking if it would be more efficient to take the sub group relationships into account when querying for warning controls instead of when populating the tree. Kind of depends on whether you expect to have more queries or more @warn controls.
artemcm
force-pushed
the
WarningControl-GroupLinks
branch
from
a797833 to
5b18fb4
Compare
I considered both options and went with this one to take the relationships into account at tree construction: my main reason for this is that I expect the primary client use-case (such as the compiler in swiftlang/swift#85036) to pay the cost of constructing a tree and then run queries against said tree. I do expect there to be a lot of queries against a constructed tree, and it's tricky to predict how many group relationships we will end up with since that concept is still quite new. Baking these relationships into the constructed tree does also result in simpler query logic on the client side. |
|
@swift-ci test |
ahoppen
left a comment
ahoppen
left a comment
There was a problem hiding this comment.
Sorry, some of the comments below I should have already brought up in the last review.
Sources/SwiftWarningControl/SyntaxProtocol+WarningControl.swift
Outdated
Show resolved
Hide resolved
artemcm
force-pushed
the
WarningControl-GroupLinks
branch
3 times, most recently
from
8feabad to
00891ac
Compare
|
@swift-ci test |
ahoppen
left a comment
ahoppen
left a comment
There was a problem hiding this comment.
A couple nitpicks, otherwise LGTM.
Sources/SwiftWarningControl/DiagnosticGroupInheritanceTree.swift
Outdated
Show resolved
Hide resolved
Sources/SwiftWarningControl/DiagnosticGroupInheritanceTree.swift
Outdated
Show resolved
Hide resolved
Sources/SwiftWarningControl/DiagnosticGroupInheritanceTree.swift
Outdated
Show resolved
Hide resolved
…ance This takes form of a new parameter: `groupInheritanceTree: DiagnosticGroupInheritanceTree? = nil` on both: - `SyntaxProtocol.warningGroupBehavior` - `SyntaxProtocol.warningGroupControlRegionTree` Where `DiagnosticGroupInheritanceTree` is a wrapper for a dictionary where the key is a super-group and values are its sub-groups. Upon encountering a warning group control (`@warn`), its corresponding region is populated with its identifier and behavior, as well as the same corresponding behavior for each of its sub-groups, direct and transitive.
artemcm
force-pushed
the
WarningControl-GroupLinks
branch
from
00891ac to
a0e1efc
Compare
|
@swift-ci test |
|
@swift-ci test Windows platform |
1 similar comment
|
@swift-ci test Windows platform |
2 participants
These links establish group inheritance relationships amongst diagnostic groups. This takes form of a new parameter:
subGroupLinks: [DiagnosticGroupIdentifier: [DiagnosticGroupIdentifier]] = [:]on both:SyntaxProtocol.warningGroupBehaviorSyntaxProtocol.warningGroupControlRegionTreeWhere the dictionary key is a super-group and dictionary values are its sub-groups.Upon encountering a warning group control (
@warn), its corresponding region is populated with its identifier and behavior, as well as the same corresponding behavior for each of its sub-groups, direct and transitive.