Add DiagGroupID to Diagnostic #77928
Conversation
This change addresses the following issue: when an error is being wrapped in a warning, the diagnostic message will use the wrapper's `DiagGroupID` as the warning's name. However, we want to retain the original error's group for use. For example, in Swift 5, async_unavailable_decl is wrapped in error_in_future_swift_version. When we print a diagnostic of this kind, we want to keep the `DiagGroupID` of `async_unavailable_decl`, not that of `error_in_future_swift_version`. To achieve this, we add `DiagGroupID` to the `Diagnostic` class. When an active diagnostic is wrapped in DiagnosticEngine, we retain the original `DiagGroupID`. For illustration purposes, this change also introduces a new group: `DeclarationUnavailableFromAsynchronousContext`. With this change, we produce errors and warnings of this kind with messages like the following: ``` global function 'fNoAsync' is unavailable from asynchronous contexts [DeclarationUnavailableFromAsynchronousContext] global function 'fNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext] ```
DmT021
requested review from
zoecarver,
hyp,
egorzhdan,
Xazax-hun,
ahoppen,
bnbarham,
hamishknight,
rintaro,
hborla,
slavapestov and
xedin
as code owners
|
@DougGregor Can you take a look and run the test, please? |
|
@swift-ci please smoke test |
| @@ -25,6 +25,7 @@ GROUP(no_group, "") | |||
| GROUP(DeprecatedDeclaration, "DeprecatedDeclaration.md") | |||
| GROUP(Unsafe, "Unsafe.md") | |||
| GROUP(UnknownWarningGroup, "UnknownWarningGroup.md") | |||
| GROUP(DeclarationUnavailableFromAsynchronousContext, "DeclarationUnavailableFromAsynchronousContext.md") | |||
There was a problem hiding this comment.
As far as I am concerned, we cannot afford to remove a group once it ships, which calls for very mindful additions. If this group is purely intended to support test coverage, we should establish and document mechanisms for defining and conveying its experimental or internal status. Otherwise, I don’t think it wise to modify the group tree without at least an approving review from the Language Steering Group, if not a proposal.
There was a problem hiding this comment.
I was hoping to gather all the concurrency related groups and start a thread on the forum. Unfortunately, I didn't find enough time to do that. This commit was merged after the 6.1 release was branched. So we can still remove this group for now.
There was a problem hiding this comment.
I have a few ideas about how to make this a unit test. I will tag you in the PR.
This change addresses the following issue: when an error is being wrapped in a warning, the diagnostic message will use the wrapper's
DiagGroupIDas the warning's name. However, we want to retain the original error's group for use. For example, in Swift 5, async_unavailable_decl is wrapped in error_in_future_swift_version. When we print a diagnostic of this kind, we want to keep theDiagGroupIDofasync_unavailable_decl, not that oferror_in_future_swift_version. To achieve this, we addDiagGroupIDto theDiagnosticclass. When an active diagnostic is wrapped in DiagnosticEngine, we retain the originalDiagGroupID.For illustration purposes, this change also introduces a new group:
DeclarationUnavailableFromAsynchronousContext.With this change, we produce errors and warnings of this kind with messages like the following: