Diag: Handle CustomAttr in formatDiagnosticArgument

Author: AnthonyLatsis

include/swift/AST/DiagnosticsSema.def

@@ -319,8 +319,8 @@ ERROR(class_cannot_be_addressable_for_dependencies,none,
       "a class cannot be @_addressableForDependencies", ())
 
 ERROR(unsupported_closure_attr,none,
-      "%select{attribute |}0'%1' is not supported on a closure",
-      (bool, StringRef))
+      "%select{attribute |}0%1 is not supported on a closure",
+      (bool, DeclAttribute))
 
 NOTE(suggest_partial_overloads,none,
       "overloads for '%1' exist with these %select{"
@@ -6002,14 +6002,15 @@ NOTE(move_global_actor_attr_to_storage_decl,none,
      "move global actor attribute to %kind0", (const ValueDecl *))
 
 ERROR(actor_isolation_multiple_attr_2,none,
-      "%kind0 has multiple actor-isolation attributes ('%1' and '%2')",
-      (const Decl *, StringRef, StringRef))
+      "%kind0 has multiple actor-isolation attributes (%1 and %2)",
+      (const Decl *, DeclAttribute, DeclAttribute))
 ERROR(actor_isolation_multiple_attr_3,none,
-      "%0 %1 has multiple actor-isolation attributes ('%2', '%3' and '%4')",
-      (const Decl *, StringRef, StringRef, StringRef))
+      "%0 %1 has multiple actor-isolation attributes (%2, %3 and %4)",
+      (const Decl *, DeclAttribute, DeclAttribute, DeclAttribute))
 ERROR(actor_isolation_multiple_attr_4,none,
-      "%0 %1 has multiple actor-isolation attributes ('%2', '%3', '%4', and '%5')",
-      (const Decl *, StringRef, StringRef, StringRef, StringRef))
+      "%0 %1 has multiple actor-isolation attributes (%2, %3, %4, and %5)",
+      (const Decl *, DeclAttribute, DeclAttribute, DeclAttribute,
+       DeclAttribute))
 ERROR(actor_isolation_override_mismatch,none,
       "%0 %kind1 has different actor isolation from %2 overridden declaration",
       (ActorIsolation, const ValueDecl *, ActorIsolation))

lib/AST/DiagnosticEngine.cpp

@@ -1083,17 +1083,28 @@ static void formatDiagnosticArgument(StringRef Modifier,
     Out << Stmt::getDescriptiveKindName(Arg.getAsDescriptiveStmtKind());
     break;
 
-  case DiagnosticArgumentKind::DeclAttribute:
+  case DiagnosticArgumentKind::DeclAttribute: {
+    auto *const attr = Arg.getAsDeclAttribute();
+    const auto printAttrName = [&] {
+      if (auto *custom = dyn_cast<CustomAttr>(attr)) {
+        custom->getTypeRepr()->print(Out);
+      } else {
+        Out << attr->getAttrName();
+      }
+    };
+
     assert(Modifier.empty() &&
            "Improper modifier for DeclAttribute argument");
-    if (Arg.getAsDeclAttribute()->isDeclModifier())
-      Out << FormatOpts.OpeningQuotationMark
-          << Arg.getAsDeclAttribute()->getAttrName()
-          << FormatOpts.ClosingQuotationMark;
-    else
-      Out << '@' << Arg.getAsDeclAttribute()->getAttrName();
+    if (Arg.getAsDeclAttribute()->isDeclModifier()) {
+      Out << FormatOpts.OpeningQuotationMark;
+      printAttrName();
+      Out << FormatOpts.ClosingQuotationMark;
+    } else {
+      Out << '@';
+      printAttrName();
+    }
     break;
-
+  }
   case DiagnosticArgumentKind::AvailabilityDomain:
     assert(Modifier.empty() &&
            "Improper modifier for AvailabilityDomain argument");

lib/Sema/TypeCheckAttr.cpp

@@ -4309,69 +4309,57 @@ static void checkGlobalActorAttr(
   auto isolatedAttr = decl->getAttrs().getAttribute<IsolatedAttr>();
   auto nonisolatedAttr = decl->getAttrs().getAttribute<NonisolatedAttr>();
   auto executionAttr = decl->getAttrs().getAttribute<ExecutionAttr>();
-  struct NameAndRange {
-    StringRef name;
-    SourceRange range;
 
-    NameAndRange(StringRef _name, SourceRange _range)
-        : name(_name), range(_range) {}
-  };
-
-  llvm::SmallVector<NameAndRange, 4> attributes;
+  llvm::SmallVector<const DeclAttribute *, 2> attributes;
 
-  attributes.push_back(NameAndRange(globalActorAttr.second->getName().str(),
-                                    globalActorAttr.first->getRangeWithAt()));
+  attributes.push_back(globalActorAttr.first);
 
   if (isolatedAttr) {
-    attributes.push_back(NameAndRange(isolatedAttr->getAttrName(),
-                                      isolatedAttr->getRangeWithAt()));
+    attributes.push_back(isolatedAttr);
   }
   if (nonisolatedAttr) {
-    attributes.push_back(NameAndRange(nonisolatedAttr->getAttrName(),
-                                      nonisolatedAttr->getRangeWithAt()));
+    attributes.push_back(nonisolatedAttr);
   }
   if (executionAttr) {
-    attributes.push_back(NameAndRange(executionAttr->getAttrName(),
-                                      executionAttr->getRangeWithAt()));
+    attributes.push_back(executionAttr);
   }
 
   if (attributes.size() == 1)
     return;
 
   if (attributes.size() == 2) {
-    decl->diagnose(diag::actor_isolation_multiple_attr_2, decl,
-                   attributes[0].name, attributes[1].name)
-        .highlight(attributes[0].range)
-        .highlight(attributes[1].range)
+    decl->diagnose(diag::actor_isolation_multiple_attr_2, decl, attributes[0],
+                   attributes[1])
+        .highlight(attributes[0]->getRangeWithAt())
+        .highlight(attributes[1]->getRangeWithAt())
         .warnUntilSwiftVersion(6)
-        .fixItRemove(attributes[1].range);
+        .fixItRemove(attributes[1]->getRangeWithAt());
     return;
   }
 
   if (attributes.size() == 3) {
-    decl->diagnose(diag::actor_isolation_multiple_attr_3, decl,
-                   attributes[0].name, attributes[1].name, attributes[2].name)
-        .highlight(attributes[0].range)
-        .highlight(attributes[1].range)
-        .highlight(attributes[2].range)
+    decl->diagnose(diag::actor_isolation_multiple_attr_3, decl, attributes[0],
+                   attributes[1], attributes[2])
+        .highlight(attributes[0]->getRangeWithAt())
+        .highlight(attributes[1]->getRangeWithAt())
+        .highlight(attributes[2]->getRangeWithAt())
         .warnUntilSwiftVersion(6)
-        .fixItRemove(attributes[1].range)
-        .fixItRemove(attributes[2].range);
+        .fixItRemove(attributes[1]->getRangeWithAt())
+        .fixItRemove(attributes[2]->getRangeWithAt());
     return;
   }
 
   assert(attributes.size() == 4);
-  decl->diagnose(diag::actor_isolation_multiple_attr_4, decl,
-                 attributes[0].name, attributes[1].name, attributes[2].name,
-                 attributes[3].name)
-      .highlight(attributes[0].range)
-      .highlight(attributes[1].range)
-      .highlight(attributes[2].range)
-      .highlight(attributes[3].range)
+  decl->diagnose(diag::actor_isolation_multiple_attr_4, decl, attributes[0],
+                 attributes[1], attributes[2], attributes[3])
+      .highlight(attributes[0]->getRangeWithAt())
+      .highlight(attributes[1]->getRangeWithAt())
+      .highlight(attributes[2]->getRangeWithAt())
+      .highlight(attributes[3]->getRangeWithAt())
       .warnUntilSwiftVersion(6)
-      .fixItRemove(attributes[1].range)
-      .fixItRemove(attributes[2].range)
-      .fixItRemove(attributes[3].range);
+      .fixItRemove(attributes[1]->getRangeWithAt())
+      .fixItRemove(attributes[2]->getRangeWithAt())
+      .fixItRemove(attributes[3]->getRangeWithAt());
 }
 
 void AttributeChecker::visitCustomAttr(CustomAttr *attr) {
@@ -8137,10 +8125,10 @@ class ClosureAttributeChecker
     : ctx(closure->getASTContext()), closure(closure) { }
 
   void visitDeclAttribute(DeclAttribute *attr) {
-    ctx.Diags.diagnose(
-        attr->getLocation(), diag::unsupported_closure_attr,
-        attr->isDeclModifier(), attr->getAttrName())
-      .fixItRemove(attr->getRangeWithAt());
+    ctx.Diags
+        .diagnose(attr->getLocation(), diag::unsupported_closure_attr,
+                  attr->isDeclModifier(), attr)
+        .fixItRemove(attr->getRangeWithAt());
     attr->setInvalid();
   }
 
@@ -8236,18 +8224,10 @@ class ClosureAttributeChecker
     }
 
     // Otherwise, it's an error.
-    std::string typeName;
-    if (auto typeRepr = attr->getTypeRepr()) {
-      llvm::raw_string_ostream out(typeName);
-      typeRepr->print(out);
-    } else {
-      typeName = attr->getType().getString();
-    }
-
-    ctx.Diags.diagnose(
-        attr->getLocation(), diag::unsupported_closure_attr,
-        attr->isDeclModifier(), typeName)
-      .fixItRemove(attr->getRangeWithAt());
+    ctx.Diags
+        .diagnose(attr->getLocation(), diag::unsupported_closure_attr,
+                  attr->isDeclModifier(), attr)
+        .fixItRemove(attr->getRangeWithAt());
     attr->setInvalid();
   }
 };

test/Concurrency/isolated_parameters.swift

@@ -341,7 +341,7 @@ func isolatedClosures() {
   }
 }
 
-// expected-warning@+3 {{global function 'allOfEm' has multiple actor-isolation attributes ('MainActor' and 'nonisolated')}}
+// expected-warning@+3 {{global function 'allOfEm' has multiple actor-isolation attributes (@MainActor and 'nonisolated')}}
 // expected-warning@+2 {{global function with 'isolated' parameter cannot be 'nonisolated'; this is an error in the Swift 6 language mode}}{{12-24=}}
 // expected-warning@+1 {{global function with 'isolated' parameter cannot have a global actor; this is an error in the Swift 6 language mode}}{{1-12=}}
 @MainActor nonisolated func allOfEm(_ a: isolated A) {

test/Concurrency/nonisolated_rules.swift

@@ -154,7 +154,7 @@ class KlassA {
 
 @MainActor
 nonisolated struct Conflict {}
-// expected-error@-1 {{struct 'Conflict' has multiple actor-isolation attributes ('MainActor' and 'nonisolated')}}
+// expected-error@-1 {{struct 'Conflict' has multiple actor-isolation attributes (@MainActor and 'nonisolated')}}
 
 struct B: Sendable {
   // expected-error@+1 {{'nonisolated' can not be applied to variable with non-'Sendable' type 'NonSendable}}

test/attr/attr_execution.swift

@@ -96,11 +96,11 @@ struct TestAttributeCollisions {
   }
 
   @MainActor @execution(concurrent) func testGlobalActor() async {}
-  // expected-warning @-1 {{instance method 'testGlobalActor()' has multiple actor-isolation attributes ('MainActor' and 'execution(concurrent)')}}
+  // expected-warning @-1 {{instance method 'testGlobalActor()' has multiple actor-isolation attributes (@MainActor and @execution(concurrent))}}
 
   @execution(caller) nonisolated func testNonIsolatedCaller() async {} // Ok
   @MainActor @execution(caller) func testGlobalActorCaller() async {}
-  // expected-warning@-1 {{instance method 'testGlobalActorCaller()' has multiple actor-isolation attributes ('MainActor' and 'execution(caller)')}}
+  // expected-warning@-1 {{instance method 'testGlobalActorCaller()' has multiple actor-isolation attributes (@MainActor and @execution(caller))}}
   @execution(caller) func testCaller(arg: isolated MainActor) async {}
   // expected-error@-1 {{cannot use '@execution' on instance method 'testCaller(arg:)' because it has an isolated parameter: 'arg'}}
 

test/attr/closures.swift

@@ -1,11 +1,11 @@
 // RUN: %target-typecheck-verify-swift -swift-version 5
 
 func testNonacceptedClosures() {
-  let fn = { @usableFromInline in // expected-error{{'usableFromInline' is not supported on a closure}}
+  let fn = { @usableFromInline in // expected-error{{attribute @usableFromInline is not supported on a closure}}
     "hello"
   }
 
-  let fn2: (Int) -> Int = { @usableFromInline x in  // expected-error{{'usableFromInline' is not supported on a closure}}
+  let fn2: (Int) -> Int = { @usableFromInline x in  // expected-error{{attribute @usableFromInline is not supported on a closure}}
     print("hello")
     return x
   }

test/attr/global_actor.swift

@@ -98,7 +98,7 @@ struct Container {
 // Redundant attributes
 // -----------------------------------------------------------------------
 extension SomeActor {
-  @GA1 nonisolated func conflict1() { } // expected-warning {{instance method 'conflict1()' has multiple actor-isolation attributes ('GA1' and 'nonisolated')}}
+  @GA1 nonisolated func conflict1() { } // expected-warning {{instance method 'conflict1()' has multiple actor-isolation attributes (@GA1 and 'nonisolated')}}
 }