Merge pull request #78398 from AnthonyLatsis/micronecta-scholtzi

 DiagnosticEngine: Fix diagnostic groups behavior for wrapped diagnostics

Author: AnthonyLatsis

include/swift/AST/DiagnosticEngine.h

@@ -21,7 +21,6 @@
 #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"
@@ -68,6 +67,8 @@ namespace swift {
   /// this enumeration type that uniquely identifies it.
   enum class DiagID : uint32_t;
 
+  enum class DiagGroupID : uint16_t;
+
   /// Describes a diagnostic along with its argument types.
   ///
   /// The diagnostics header introduces instances of this type for each 
@@ -512,11 +513,17 @@ namespace swift {
     friend DiagnosticEngine;
     friend class InFlightDiagnostic;
 
-    Diagnostic(DiagID ID, DiagGroupID GroupID) : ID(ID), GroupID(GroupID) {}
-
     /// Constructs a Diagnostic with DiagGroupID infered from DiagID.
     Diagnostic(DiagID ID);
 
+  protected:
+    /// Only use this constructor privately in this class or in unit tests by
+    /// subclassing.
+    /// In unit tests, it is used as a means for associating diagnostics with
+    /// groups and, thus, circumventing the need to otherwise define mock
+    /// diagnostics, which is not accounted for in the current design.
+    Diagnostic(DiagID ID, DiagGroupID GroupID) : ID(ID), GroupID(GroupID) {}
+
   public:
     // All constructors are intentionally implicit.
     template<typename ...ArgTypes>
@@ -527,8 +534,9 @@ namespace swift {
       gatherArgs(VArgs...);
     }
 
-    /*implicit*/Diagnostic(DiagID ID, ArrayRef<DiagnosticArgument> Args)
-      : ID(ID), Args(Args.begin(), Args.end()) {}
+    Diagnostic(DiagID ID, ArrayRef<DiagnosticArgument> Args) : Diagnostic(ID) {
+      this->Args.append(Args.begin(), Args.end());
+    }
 
     template <class... ArgTypes>
     static Diagnostic fromTuple(Diag<ArgTypes...> id,
@@ -902,7 +910,9 @@ namespace swift {
     /// Don't emit any remarks
     bool suppressRemarks = false;
 
-    /// Treat these warnings as errors. Indices here correspond to DiagID enum
+    /// A mapping from `DiagGroupID` identifiers to Boolean values indicating
+    /// whether warnings belonging to the respective diagnostic groups should be
+    /// escalated to errors.
     llvm::BitVector warningsAsErrors;
 
     /// Whether a fatal error has occurred
@@ -944,16 +954,23 @@ namespace swift {
     void setSuppressRemarks(bool val) { suppressRemarks = val; }
     bool getSuppressRemarks() const { return suppressRemarks; }
 
-    /// Whether a warning should be upgraded to an error or not
-    void setWarningAsErrorForDiagID(DiagID id, bool value) {
+    /// Sets whether warnings belonging to the diagnostic group identified by
+    /// `id` should be escalated to errors.
+    void setWarningsAsErrorsForDiagGroupID(DiagGroupID id, bool value) {
       warningsAsErrors[(unsigned)id] = value;
     }
-    bool getWarningAsErrorForDiagID(DiagID id) {
+
+    /// Returns a Boolean value indicating whether warnings belonging to the
+    /// diagnostic group identified by `id` should be escalated to errors.
+    bool getWarningsAsErrorsForDiagGroupID(DiagGroupID id) {
       return warningsAsErrors[(unsigned)id];
     }
 
-    /// Whether all warnings should be upgraded to errors or not
+    /// Whether all warnings should be upgraded to errors or not.
     void setAllWarningsAsErrors(bool value) {
+      // This works as intended because every diagnostic belongs to either a
+      // custom group or the top-level `DiagGroupID::no_group`, which is also
+      // a group.
       if (value) {
         warningsAsErrors.set();
       } else {

include/swift/AST/DiagnosticGroups.def

@@ -25,7 +25,6 @@ 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"

include/swift/AST/DiagnosticGroups.h

@@ -33,7 +33,7 @@ enum class DiagGroupID : uint16_t {
 
 constexpr const auto DiagGroupsCount = [] {
   size_t count = 0;
-#define GROUP(Name, Version) count++;
+#define GROUP(Name, Version) ++count;
 #include "DiagnosticGroups.def"
   return count;
 }();

include/swift/AST/DiagnosticsSema.def

@@ -5963,7 +5963,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 *))
-GROUPED_ERROR(async_unavailable_decl,DeclarationUnavailableFromAsynchronousContext,none,
+ERROR(async_unavailable_decl,none,
       "%kindbase0 is unavailable from asynchronous contexts%select{|; %1}1",
       (const ValueDecl *, StringRef))
 

lib/AST/DiagnosticEngine.cpp

@@ -176,7 +176,7 @@ DiagnosticState::DiagnosticState() {
   // Initialize our ignored diagnostics to default
   ignoredDiagnostics.resize(LocalDiagID::NumDiags);
   // Initialize warningsAsErrors to default
-  warningsAsErrors.resize(LocalDiagID::NumDiags);
+  warningsAsErrors.resize(DiagGroupsCount);
 }
 
 Diagnostic::Diagnostic(DiagID ID)
@@ -551,9 +551,7 @@ void DiagnosticEngine::setWarningsAsErrorsRules(
       if (auto groupID = getDiagGroupIDByName(name);
           groupID && *groupID != DiagGroupID::no_group) {
         getDiagGroupInfoByID(*groupID).traverseDepthFirst([&](auto group) {
-          for (DiagID diagID : group.diagnostics) {
-            state.setWarningAsErrorForDiagID(diagID, isEnabled);
-          }
+          state.setWarningsAsErrorsForDiagGroupID(*groupID, isEnabled);
         });
       } else {
         unknownGroups.push_back(std::string(name));
@@ -1232,7 +1230,7 @@ DiagnosticBehavior DiagnosticState::determineBehavior(const Diagnostic &diag) {
   //   4) If the user substituted a different behavior for this behavior, apply
   //      that change
   if (lvl == DiagnosticBehavior::Warning) {
-    if (getWarningAsErrorForDiagID(diag.getID()))
+    if (getWarningsAsErrorsForDiagGroupID(diag.getGroupID()))
       lvl = DiagnosticBehavior::Error;
     if (suppressWarnings)
       lvl = DiagnosticBehavior::Ignore;

test/attr/attr_availability_noasync.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -print-diagnostic-groups
+// RUN: %target-typecheck-verify-swift
 
 // 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 [DeclarationUnavailableFromAsynchronousContext]}}
+    // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
     basicNoAsync()
 
-    // 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]}}
+    // 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}}
     messageNoAsync()
 
-    // 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}}
+    // expected-warning@+1{{global function 'renamedNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}{{5-19=asyncReplacement}}
     renamedNoAsync() { _ in }
 
-    // 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}}
+    // expected-warning@+1{{global function 'readStringFromIO' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}{{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 [DeclarationUnavailableFromAsynchronousContext]}}
+        // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
         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 [DeclarationUnavailableFromAsynchronousContext]}}
+        // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
         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 [DeclarationUnavailableFromAsynchronousContext]}}
+            // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
             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 [DeclarationUnavailableFromAsynchronousContext]}}
+        // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
         defer { basicNoAsync() }
-        // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
+        // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
         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 [DeclarationUnavailableFromAsynchronousContext]}}
+        // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
         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 [DeclarationUnavailableFromAsynchronousContext]}}
+        // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
         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 [DeclarationUnavailableFromAsynchronousContext]}}
+            // expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
             defer { basicNoAsync() }
             return ()
         }

unittests/AST/CMakeLists.txt

@@ -4,6 +4,7 @@ add_swift_unittest(SwiftASTTests
   ASTWalkerTests.cpp
   IndexSubsetTests.cpp
   DiagnosticConsumerTests.cpp
+  DiagnosticGroupsTests.cpp
   SourceLocTests.cpp
   TestContext.cpp
   TypeMatchTests.cpp