From d56b7df8a9e8c30180c6d6f25c65bcb6dad89527 Mon Sep 17 00:00:00 2001 From: Dmitrii Galimzianov Date: Tue, 3 Dec 2024 20:12:11 +0100 Subject: [PATCH] Add DiagGroupID to Diagnostic 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] ``` --- include/swift/AST/DiagnosticEngine.h | 24 +++++++-- include/swift/AST/DiagnosticGroups.def | 1 + include/swift/AST/DiagnosticsCommon.def | 4 ++ include/swift/AST/DiagnosticsSema.def | 2 +- lib/AST/DiagnosticEngine.cpp | 49 +++++++++++++------ lib/IDE/CodeCompletionDiagnostics.cpp | 2 +- lib/PrintAsClang/ModuleContentsWriter.cpp | 4 +- test/attr/attr_availability_noasync.swift | 26 +++++----- ...ationUnavailableFromAsynchronousContext.md | 18 +++++++ 9 files changed, 94 insertions(+), 36 deletions(-) create mode 100644 userdocs/diagnostic_groups/DeclarationUnavailableFromAsynchronousContext.md diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h index 3319614217df7..1112a67bab45c 100644 --- a/include/swift/AST/DiagnosticEngine.h +++ b/include/swift/AST/DiagnosticEngine.h @@ -21,6 +21,7 @@ #include "swift/AST/ActorIsolation.h" #include "swift/AST/DeclNameLoc.h" #include "swift/AST/DiagnosticConsumer.h" +#include "swift/AST/DiagnosticGroups.h" #include "swift/AST/TypeLoc.h" #include "swift/Basic/PrintDiagnosticNamesMode.h" #include "swift/Basic/Statistic.h" @@ -498,6 +499,7 @@ namespace swift { private: DiagID ID; + DiagGroupID GroupID; SmallVector Args; SmallVector Ranges; SmallVector FixIts; @@ -510,7 +512,10 @@ namespace swift { friend DiagnosticEngine; friend class InFlightDiagnostic; - Diagnostic(DiagID ID) : ID(ID) {} + Diagnostic(DiagID ID, DiagGroupID GroupID) : ID(ID), GroupID(GroupID) {} + + /// Constructs a Diagnostic with DiagGroupID infered from DiagID. + Diagnostic(DiagID ID); public: // All constructors are intentionally implicit. @@ -535,6 +540,7 @@ namespace swift { // Accessors. DiagID getID() const { return ID; } + DiagGroupID getGroupID() const { return GroupID; } ArrayRef getArgs() const { return Args; } ArrayRef getRanges() const { return Ranges; } ArrayRef getFixIts() const { return FixIts; } @@ -1434,7 +1440,8 @@ namespace swift { /// Generate DiagnosticInfo for a Diagnostic to be passed to consumers. std::optional - diagnosticInfoForDiagnostic(const Diagnostic &diagnostic); + diagnosticInfoForDiagnostic(const Diagnostic &diagnostic, + bool includeDiagnosticName); /// Send \c diag to all diagnostic consumers. void emitDiagnostic(const Diagnostic &diag); @@ -1460,9 +1467,16 @@ namespace swift { public: DiagnosticKind declaredDiagnosticKindFor(const DiagID id); - llvm::StringRef - diagnosticStringFor(const DiagID id, - PrintDiagnosticNamesMode printDiagnosticNamesMode); + /// Get a localized format string for a given `DiagID`. If no localization + /// available returns the default string for that `DiagID`. + llvm::StringRef diagnosticStringFor(DiagID id); + + /// Get a localized format string with an optional diagnostic name appended + /// to it. The diagnostic name type is defined by + /// `PrintDiagnosticNamesMode`. + llvm::StringRef diagnosticStringWithNameFor( + DiagID id, DiagGroupID groupID, + PrintDiagnosticNamesMode printDiagnosticNamesMode); static llvm::StringRef diagnosticIDStringFor(const DiagID id); diff --git a/include/swift/AST/DiagnosticGroups.def b/include/swift/AST/DiagnosticGroups.def index 1854905d6e6bf..f74d4e659ea48 100644 --- a/include/swift/AST/DiagnosticGroups.def +++ b/include/swift/AST/DiagnosticGroups.def @@ -25,6 +25,7 @@ GROUP(no_group, "") GROUP(DeprecatedDeclaration, "DeprecatedDeclaration.md") GROUP(Unsafe, "Unsafe.md") GROUP(UnknownWarningGroup, "UnknownWarningGroup.md") +GROUP(DeclarationUnavailableFromAsynchronousContext, "DeclarationUnavailableFromAsynchronousContext.md") #define UNDEFINE_DIAGNOSTIC_GROUPS_MACROS #include "swift/AST/DefineDiagnosticGroupsMacros.h" diff --git a/include/swift/AST/DiagnosticsCommon.def b/include/swift/AST/DiagnosticsCommon.def index 380608bd6c46a..f1ea5d7ca27fa 100644 --- a/include/swift/AST/DiagnosticsCommon.def +++ b/include/swift/AST/DiagnosticsCommon.def @@ -47,10 +47,14 @@ ERROR(error_no_group_info,none, NOTE(brace_stmt_suggest_do,none, "did you mean to use a 'do' statement?", ()) +// `error_in_future_swift_version` does not have a group because warnings of this kind +// inherit the group from the wrapped error. WARNING(error_in_future_swift_version,none, "%0; this is an error in the Swift %1 language mode", (DiagnosticInfo *, unsigned)) +// `error_in_a_future_swift_version` does not have a group because warnings of this kind +// inherit the group from the wrapped error. WARNING(error_in_a_future_swift_version,none, "%0; this will be an error in a future Swift language mode", (DiagnosticInfo *)) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 7980e7e38e640..61bae7dc448da 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -5950,7 +5950,7 @@ ERROR(async_decl_must_be_available_from_async,none, ERROR(async_named_decl_must_be_available_from_async,none, "asynchronous %kind0 must be available from asynchronous contexts", (const ValueDecl *)) -ERROR(async_unavailable_decl,none, +GROUPED_ERROR(async_unavailable_decl,DeclarationUnavailableFromAsynchronousContext,none, "%kindbase0 is unavailable from asynchronous contexts%select{|; %1}1", (const ValueDecl *, StringRef)) diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp index f5bca58bd0c70..80d476856e8bf 100644 --- a/lib/AST/DiagnosticEngine.cpp +++ b/lib/AST/DiagnosticEngine.cpp @@ -179,6 +179,9 @@ DiagnosticState::DiagnosticState() { warningsAsErrors.resize(LocalDiagID::NumDiags); } +Diagnostic::Diagnostic(DiagID ID) + : Diagnostic(ID, storedDiagnosticInfos[(unsigned)ID].groupID) {} + static CharSourceRange toCharSourceRange(SourceManager &SM, SourceRange SR) { return CharSourceRange(SM, SR.Start, Lexer::getLocForEndOfToken(SM, SR.End)); } @@ -464,8 +467,8 @@ InFlightDiagnostic::wrapIn(const Diagnostic &wrapper) { limit(Engine->getActiveDiagnostic().BehaviorLimit, DiagnosticBehavior::Unspecified); - Engine->WrappedDiagnostics.push_back( - *Engine->diagnosticInfoForDiagnostic(Engine->getActiveDiagnostic())); + Engine->WrappedDiagnostics.push_back(*Engine->diagnosticInfoForDiagnostic( + Engine->getActiveDiagnostic(), /* includeDiagnosticName= */ false)); Engine->state.swap(tempState); @@ -478,6 +481,7 @@ InFlightDiagnostic::wrapIn(const Diagnostic &wrapper) { // Overwrite the ID and argument with those from the wrapper. Engine->getActiveDiagnostic().ID = wrapper.ID; Engine->getActiveDiagnostic().Args = wrapper.Args; + // Intentionally keeping the original GroupID here // Set the argument to the diagnostic being wrapped. assert(wrapper.getArgs().front().getKind() == DiagnosticArgumentKind::Diagnostic); @@ -1294,7 +1298,8 @@ void DiagnosticEngine::forwardTentativeDiagnosticsTo( } std::optional -DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic) { +DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic, + bool includeDiagnosticName) { auto behavior = state.determineBehavior(diagnostic); if (behavior == DiagnosticBehavior::Ignore) return std::nullopt; @@ -1347,12 +1352,19 @@ DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic) { } } - return DiagnosticInfo( - diagnostic.getID(), loc, toDiagnosticKind(behavior), - diagnosticStringFor(diagnostic.getID(), getPrintDiagnosticNamesMode()), - diagnostic.getArgs(), Category, getDefaultDiagnosticLoc(), - /*child note info*/ {}, diagnostic.getRanges(), fixIts, - diagnostic.isChildNote()); + llvm::StringRef format; + if (includeDiagnosticName) + format = + diagnosticStringWithNameFor(diagnostic.getID(), diagnostic.getGroupID(), + getPrintDiagnosticNamesMode()); + else + format = diagnosticStringFor(diagnostic.getID()); + + return DiagnosticInfo(diagnostic.getID(), loc, toDiagnosticKind(behavior), + format, diagnostic.getArgs(), Category, + getDefaultDiagnosticLoc(), + /*child note info*/ {}, diagnostic.getRanges(), fixIts, + diagnostic.isChildNote()); } static DeclName @@ -1462,7 +1474,9 @@ void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) { ArrayRef childNotes = diagnostic.getChildNotes(); std::vector extendedChildNotes; - if (auto info = diagnosticInfoForDiagnostic(diagnostic)) { + if (auto info = + diagnosticInfoForDiagnostic(diagnostic, + /* includeDiagnosticName= */ true)) { // If the diagnostic location is within a buffer containing generated // source code, add child notes showing where the generation occurred. // We need to avoid doing this if this is itself a child note, as otherwise @@ -1478,7 +1492,9 @@ void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) { SmallVector childInfo; for (unsigned i : indices(childNotes)) { - auto child = diagnosticInfoForDiagnostic(childNotes[i]); + auto child = + diagnosticInfoForDiagnostic(childNotes[i], + /* includeDiagnosticName= */ true); assert(child); assert(child->Kind == DiagnosticKind::Note && "Expected child diagnostics to all be notes?!"); @@ -1516,12 +1532,18 @@ DiagnosticKind DiagnosticEngine::declaredDiagnosticKindFor(const DiagID id) { return storedDiagnosticInfos[(unsigned)id].kind; } -llvm::StringRef DiagnosticEngine::diagnosticStringFor( - const DiagID id, PrintDiagnosticNamesMode printDiagnosticNamesMode) { +llvm::StringRef DiagnosticEngine::diagnosticStringFor(DiagID id) { llvm::StringRef message = diagnosticStrings[(unsigned)id]; if (auto localizationProducer = localization.get()) { message = localizationProducer->getMessageOr(id, message); } + return message; +} + +llvm::StringRef DiagnosticEngine::diagnosticStringWithNameFor( + DiagID id, DiagGroupID groupID, + PrintDiagnosticNamesMode printDiagnosticNamesMode) { + auto message = diagnosticStringFor(id); auto formatMessageWithName = [&](StringRef message, StringRef name) { const int additionalCharsLength = 3; // ' ', '[', ']' std::string messageWithName; @@ -1540,7 +1562,6 @@ llvm::StringRef DiagnosticEngine::diagnosticStringFor( message = formatMessageWithName(message, diagnosticIDStringFor(id)); break; case PrintDiagnosticNamesMode::Group: - auto groupID = storedDiagnosticInfos[(unsigned)id].groupID; if (groupID != DiagGroupID::no_group) { message = formatMessageWithName(message, getDiagGroupInfoByID(groupID).name); diff --git a/lib/IDE/CodeCompletionDiagnostics.cpp b/lib/IDE/CodeCompletionDiagnostics.cpp index a399109dc35aa..1928245d963c6 100644 --- a/lib/IDE/CodeCompletionDiagnostics.cpp +++ b/lib/IDE/CodeCompletionDiagnostics.cpp @@ -67,7 +67,7 @@ bool CodeCompletionDiagnostics::getDiagnostics( typename swift::detail::PassArgument::type... VArgs) { DiagID id = ID.ID; std::vector DiagArgs{std::move(VArgs)...}; - auto format = Engine.diagnosticStringFor(id, PrintDiagnosticNamesMode::None); + auto format = Engine.diagnosticStringFor(id); DiagnosticEngine::formatDiagnosticText(Out, format, DiagArgs); severity = getSeverity(Engine.declaredDiagnosticKindFor(id)); diff --git a/lib/PrintAsClang/ModuleContentsWriter.cpp b/lib/PrintAsClang/ModuleContentsWriter.cpp index ec09f2ca97bd5..5deec8caf3346 100644 --- a/lib/PrintAsClang/ModuleContentsWriter.cpp +++ b/lib/PrintAsClang/ModuleContentsWriter.cpp @@ -977,8 +977,8 @@ class ModuleWriter { const_cast(vd)); // Emit a specific unavailable message when we know why a decl can't be // exposed, or a generic message otherwise. - auto diagString = M.getASTContext().Diags.diagnosticStringFor( - diag.getID(), PrintDiagnosticNamesMode::None); + auto diagString = + M.getASTContext().Diags.diagnosticStringFor(diag.getID()); DiagnosticEngine::formatDiagnosticText(os, diagString, diag.getArgs(), DiagnosticFormatOptions()); os << "\");\n"; diff --git a/test/attr/attr_availability_noasync.swift b/test/attr/attr_availability_noasync.swift index 99ade6524791f..c88d1f9247dd7 100644 --- a/test/attr/attr_availability_noasync.swift +++ b/test/attr/attr_availability_noasync.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift +// RUN: %target-typecheck-verify-swift -print-diagnostic-groups // REQUIRES: concurrency @@ -27,16 +27,16 @@ actor IOActor { @available(SwiftStdlib 5.5, *) func asyncFunc() async { - // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}} + // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}} basicNoAsync() - // expected-warning@+1{{global function 'messageNoAsync' is unavailable from asynchronous contexts; a message from the author}} + // expected-warning@+1{{global function 'messageNoAsync' is unavailable from asynchronous contexts; a message from the author; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}} messageNoAsync() - // expected-warning@+1{{global function 'renamedNoAsync' is unavailable from asynchronous contexts}}{{5-19=asyncReplacement}} + // expected-warning@+1{{global function 'renamedNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}{{5-19=asyncReplacement}} renamedNoAsync() { _ in } - // expected-warning@+1{{global function 'readStringFromIO' is unavailable from asynchronous contexts}}{{13-29=IOActor.readString}} + // expected-warning@+1{{global function 'readStringFromIO' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}{{13-29=IOActor.readString}} let _ = readStringFromIO() } @@ -76,7 +76,7 @@ func test_defers_sync() { } func local_async_func() async { - // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}} + // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}} defer { basicNoAsync() } _ = () } @@ -89,7 +89,7 @@ func test_defers_sync() { // local async closure let local_async_closure = { () async -> Void in - // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}} + // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}} defer { basicNoAsync() } _ = () } @@ -102,7 +102,7 @@ func test_defers_sync() { var local_async_var: Void { get async { - // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}} + // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}} defer { basicNoAsync() } return () } @@ -112,9 +112,9 @@ func test_defers_sync() { @available(SwiftStdlib 5.5, *) func test_defer_async() async { defer { - // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}} + // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}} defer { basicNoAsync() } - // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}} + // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}} basicNoAsync() } @@ -124,7 +124,7 @@ func test_defer_async() async { } func local_async_func() async { - // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}} + // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}} defer { basicNoAsync() } _ = () } @@ -136,7 +136,7 @@ func test_defer_async() async { _ = local_sync_closure let local_async_closure = { () async -> Void in - // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}} + // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}} defer { basicNoAsync() } _ = () } @@ -149,7 +149,7 @@ func test_defer_async() async { var local_async_var: Void { get async { - // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}} + // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}} defer { basicNoAsync() } return () } diff --git a/userdocs/diagnostic_groups/DeclarationUnavailableFromAsynchronousContext.md b/userdocs/diagnostic_groups/DeclarationUnavailableFromAsynchronousContext.md new file mode 100644 index 0000000000000..ae3332e562ec4 --- /dev/null +++ b/userdocs/diagnostic_groups/DeclarationUnavailableFromAsynchronousContext.md @@ -0,0 +1,18 @@ +# `DeclarationUnavailableFromAsynchronousContext` + +This diagnostic group includes errors and warnings produced by the compiler when attempting to call a function from an asynchronous context that is marked as unavailable in an asynchronous context. + +```swift +@available(*, noasync) +func fNoAsync() {} + +// Swift 6 language mode +func test() async { + fNoAsync() // error: global function 'fNoAsync' is unavailable from asynchronous contexts +} + +// Swift 5 language mode +func test() async { + fNoAsync() // warning: global function 'fNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode +} +```