DiagnosticEngine: Support TypeAttribute diagnostic arguments

Author: AnthonyLatsis

Diff for: docs/Diagnostics.md

@@ -41,7 +41,8 @@ Clang also has a kind of diagnostic called a "remark", which represents informat
   - Normal: "cannot call 'super.init' outside of an initializer"
   - Better: "'super.init' cannot be called outside of an initializer"
 
-- When referring to attributes by name, use *either* "the 'foo' attribute" or "'@foo'", rather than "the '@foo' attribute".
+- When referring to attributes by name, use *either* `attribute 'foo'` or 
+  `'@foo'`, rather than `attribute '@foo'`.
 
 - Match the tone and phrasing of other diagnostics. Some common phrases:
 
@@ -130,7 +131,10 @@ If you run into any issues or have questions while following the steps above, fe
 
 - `%%` - Emits a literal percent sign.
 
-There are several format specifiers that are specific to `Decl` parameters:
+The following subsections describe format specifiers that are specific to 
+diagnostic arguments of a given type.
+
+#### `Decl`
 
 - `%kind0` - Prefixes the declaration's name with its descriptive decl kind (e.g. `instance method 'foo(x:)'`).
 
@@ -140,6 +144,11 @@ There are several format specifiers that are specific to `Decl` parameters:
 
 - `%kindbase0` - Combines `kind` and `base` (e.g. `instance method 'foo'`).
 
+#### `TypeAttribute`
+
+- `%kind0` - Replaced with `attribute 'foo'`, whereas `%0` is replaced with
+  `'@foo'`.
+
 Note: If your diagnostic could apply to accessors, be careful how you format the declaration's name; accessors have an empty name, so you need to display their accessor kind and the name of their storage decl instead. Inserting the name with a `Decl *` parameter will handle these complications automatically; if you want to use `DeclName` or `Identifier` instead, you'll probably need a separate version of the diagnostic for accessors. 
 
 ### Diagnostic Verifier ###

Diff for: include/swift/AST/Attr.h

@@ -3781,6 +3781,10 @@ class alignas(1 << AttrAlignInBits) TypeAttribute
   TypeAttrKind getKind() const {
     return TypeAttrKind(Bits.TypeAttribute.Kind);
   }
+
+  /// - Note: Do not call this directly when emitting a diagnostic. Instead,
+  /// define the diagnostic to accept a `const TypeAttribute *` and use the
+  /// appropriate format specifier.
   const char *getAttrName() const {
     return getAttrName(getKind());
   }

Diff for: include/swift/AST/DiagnosticArgument.h

@@ -31,6 +31,7 @@ namespace swift {
 
 class Decl;
 class DeclAttribute;
+class TypeAttribute;
 class TypeRepr;
 
 enum class DescriptivePatternKind : uint8_t;
@@ -85,6 +86,7 @@ enum class DiagnosticArgumentKind {
   DescriptiveDeclKind,
   DescriptiveStmtKind,
   DeclAttribute,
+  TypeAttribute,
   AvailabilityDomain,
   AvailabilityRange,
   VersionTuple,
@@ -120,6 +122,7 @@ class DiagnosticArgument {
     DescriptiveDeclKind DescriptiveDeclKindVal;
     StmtKind DescriptiveStmtKindVal;
     const DeclAttribute *DeclAttributeVal;
+    const TypeAttribute *TypeAttributeVal;
     AvailabilityDomain AvailabilityDomainVal;
     AvailabilityRange AvailabilityRangeVal;
     llvm::VersionTuple VersionVal;
@@ -153,6 +156,7 @@ class DiagnosticArgument {
   DiagnosticArgument(DescriptiveDeclKind DDK);
   DiagnosticArgument(StmtKind SK);
   DiagnosticArgument(const DeclAttribute *attr);
+  DiagnosticArgument(const TypeAttribute *attr);
   DiagnosticArgument(const AvailabilityDomain domain);
   DiagnosticArgument(const AvailabilityRange &range);
   DiagnosticArgument(llvm::VersionTuple version);
@@ -192,6 +196,7 @@ class DiagnosticArgument {
   DescriptiveDeclKind getAsDescriptiveDeclKind() const;
   StmtKind getAsDescriptiveStmtKind() const;
   const DeclAttribute *getAsDeclAttribute() const;
+  const TypeAttribute *getAsTypeAttribute() const;
   const AvailabilityDomain getAsAvailabilityDomain() const;
   const AvailabilityRange getAsAvailabilityRange() const;
   llvm::VersionTuple getAsVersionTuple() const;

Diff for: include/swift/AST/DiagnosticsCommon.def

@@ -251,8 +251,10 @@ REMARK(remark_scanner_invalidate_missing_cas, none,
 // MARK: custom attribute diagnostics
 //------------------------------------------------------------------------------
 
-ERROR(unknown_attribute,none,
+ERROR(unknown_attr_name,none,
       "unknown attribute '%0'", (StringRef))
+ERROR(unknown_type_attr,none,
+      "unknown %kind0", (const TypeAttribute *))
 
 //------------------------------------------------------------------------------
 // MARK: macro diagnostics

Diff for: include/swift/AST/DiagnosticsParse.def

@@ -1484,9 +1484,12 @@ ERROR(multiple_access_level_modifiers,none,
       "multiple incompatible access-level modifiers specified", ())
 NOTE(previous_access_level_modifier,none,
      "previous modifier specified here", ())
-ERROR(mutually_exclusive_attrs,none,
+ERROR(mutually_exclusive_decl_attrs,none,
       "%0 contradicts previous %select{attribute|modifier}2 %1",
       (DeclAttribute, DeclAttribute, bool))
+ERROR(mutually_exclusive_type_attrs,none,
+      "%0 contradicts previous %1",
+      (const TypeAttribute *, const TypeAttribute *))
 ERROR(mutually_exclusive_attr_names,none,
       "'%0' contradicts previous %select{attribute|modifier}2 '%1'",
       (StringRef, StringRef, bool))

Diff for: include/swift/AST/DiagnosticsSema.def

@@ -2686,10 +2686,6 @@ NOTE(invalid_extension_rewrite,none,
 ERROR(cannot_extend_nominal,none,
       "cannot extend %kind0", (const NominalTypeDecl *))
 
-ERROR(retroactive_not_in_extension_inheritance_clause,none,
-      "'retroactive' attribute only applies in inheritance clauses in "
-      "extensions", ())
-
 ERROR(retroactive_attr_does_not_apply,none,
       "'retroactive' attribute does not apply; %0 is declared in "
       "%select{the same package|this module}1",
@@ -5862,9 +5858,14 @@ ERROR(sendable_raw_storage,none,
       "struct %0 with @_rawLayout does not conform to the 'Sendable' protocol", (DeclName))
 
 ERROR(typeattr_not_inheritance_clause,none,
-      "'%0' attribute only applies in inheritance clauses", (StringRef))
+      "%0 only applies in inheritance clauses",
+      (const TypeAttribute *))
+ERROR(typeattr_not_extension_inheritance_clause,none,
+      "%0 only applies in inheritance clauses in extensions",
+      (const TypeAttribute *))
 ERROR(typeattr_not_existential,none,
-      "'%0' attribute cannot apply to non-protocol type %1", (StringRef, Type))
+      "%0 cannot apply to non-protocol type %1",
+      (const TypeAttribute *, Type))
 
 WARNING(unchecked_conformance_not_special,none,
       "@unchecked conformance to %0 has no meaning", (Type))
@@ -6262,8 +6263,8 @@ NOTE(overridden_required_initializer_here,none,
      "overridden required initializer is here", ())
 
 // Functions
-ERROR(attribute_requires_function_type,none,
-      "@%0 attribute only applies to function types", (StringRef))
+ERROR(type_attr_requires_function_type,none,
+      "%0 only applies to function types", (const TypeAttribute *))
 ERROR(generic_function_type,none,
       "function values cannot be generic", ())
 ERROR(unsupported_convention,none,
@@ -7603,8 +7604,8 @@ ERROR(marker_protocol_conditional_conformance,none,
 //------------------------------------------------------------------------------
 
 ERROR(differentiable_programming_attr_used_without_required_module, none,
-      "'@%0' attribute used without importing module %1",
-      (StringRef, Identifier))
+      "%0 used without importing module %1",
+      (const TypeAttribute *, Identifier))
 
 //------------------------------------------------------------------------------
 // MARK: OSLog

Diff for: lib/AST/DiagnosticArgument.cpp

@@ -91,6 +91,9 @@ DiagnosticArgument::DiagnosticArgument(StmtKind SK)
 DiagnosticArgument::DiagnosticArgument(const DeclAttribute *attr)
     : Kind(DiagnosticArgumentKind::DeclAttribute), DeclAttributeVal(attr) {}
 
+DiagnosticArgument::DiagnosticArgument(const TypeAttribute *attr)
+    : Kind(DiagnosticArgumentKind::TypeAttribute), TypeAttributeVal(attr) {}
+
 DiagnosticArgument::DiagnosticArgument(const AvailabilityDomain domain)
     : Kind(DiagnosticArgumentKind::AvailabilityDomain),
       AvailabilityDomainVal(domain) {}
@@ -207,6 +210,11 @@ const DeclAttribute *DiagnosticArgument::getAsDeclAttribute() const {
   return DeclAttributeVal;
 }
 
+const TypeAttribute *DiagnosticArgument::getAsTypeAttribute() const {
+  ASSERT(Kind == DiagnosticArgumentKind::TypeAttribute);
+  return TypeAttributeVal;
+}
+
 const AvailabilityDomain DiagnosticArgument::getAsAvailabilityDomain() const {
   ASSERT(Kind == DiagnosticArgumentKind::AvailabilityDomain);
   return AvailabilityDomainVal;

Diff for: lib/AST/DiagnosticEngine.cpp

@@ -1079,6 +1079,26 @@ static void formatDiagnosticArgument(StringRef Modifier,
     }
     break;
   }
+  case DiagnosticArgumentKind::TypeAttribute: {
+    bool useAtStyle = true;
+    if (Modifier == "kind") {
+      useAtStyle = false;
+    } else {
+      ASSERT(Modifier.empty() &&
+             "Improper modifier for TypeAttribute argument");
+    }
+
+    if (!useAtStyle) {
+      Out << "attribute ";
+    }
+    Out << FormatOpts.OpeningQuotationMark;
+    if (useAtStyle) {
+      Out << '@';
+    }
+    Out << Arg.getAsTypeAttribute()->getAttrName();
+    Out << FormatOpts.ClosingQuotationMark;
+    break;
+  }
   case DiagnosticArgumentKind::AvailabilityDomain:
     assert(Modifier.empty() &&
            "Improper modifier for AvailabilityDomain argument");

Diff for: lib/Parse/ParseDecl.cpp

@@ -4343,7 +4343,7 @@ ParserStatus Parser::parseDeclAttribute(DeclAttributes &Attributes,
   if (TypeAttribute::getAttrKindFromString(Tok.getText()).has_value())
     diagnose(Tok, diag::type_attribute_applied_to_decl);
   else if (Tok.isContextualKeyword("unknown")) {
-    diagnose(Tok, diag::unknown_attribute, "unknown");
+    diagnose(Tok, diag::unknown_attr_name, "unknown");
   } else {
     // Change the context to create a custom attribute syntax.
     auto customAttr = parseCustomAttribute(AtLoc);
@@ -4713,7 +4713,7 @@ ParserStatus Parser::parseTypeAttribute(TypeOrCustomAttr &result,
         } else {
           // We could use diag::only_allowed_in_sil here, but it's better
           // not to mention SIL in general diagnostics.
-          diagnose(AtLoc, diag::unknown_attribute, Text);
+          diagnose(AtLoc, diag::unknown_attr_name, Text);
         }
       }
       return makeParserSuccess();
@@ -5457,7 +5457,7 @@ static void diagnoseOperatorFixityAttributes(Parser &P,
   for (auto it = fixityAttrs.begin(); it != fixityAttrs.end(); ++it) {
     if (it != fixityAttrs.begin()) {
       auto *attr = *it;
-      P.diagnose(attr->getLocation(), diag::mutually_exclusive_attrs, attr,
+      P.diagnose(attr->getLocation(), diag::mutually_exclusive_decl_attrs, attr,
                  fixityAttrs.front(), attr->isDeclModifier())
           .fixItRemove(attr->getRange());
       attr->setInvalid();

Diff for: lib/Parse/ParseStmt.cpp

@@ -2720,7 +2720,7 @@ ParserResult<CaseStmt> Parser::parseStmtCase(bool IsActive) {
       assert(peekToken().is(tok::identifier) && "isAtStartOfSwitchCase() lied");
 
       consumeToken(tok::at_sign);
-      diagnose(Tok, diag::unknown_attribute, Tok.getText());
+      diagnose(Tok, diag::unknown_attr_name, Tok.getText());
       consumeToken(tok::identifier);
 
       if (Tok.is(tok::l_paren))

Diff for: lib/SIL/Parser/ParseSIL.cpp

@@ -4843,7 +4843,7 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
       } else if (attr == "builtin") {
         setFromBuiltin(true);
       } else {
-        P.diagnose(identLoc, diag::unknown_attribute, attr);
+        P.diagnose(identLoc, diag::unknown_attr_name, attr);
       }
 
       if (!P.consumeIf(tok::r_square))
@@ -7045,14 +7045,14 @@ bool SILParser::parseCallInstruction(SILLocation InstLoc,
         llvm::SmallString<64> name;
         llvm::raw_svector_ostream os(name);
         os << isolationCrossing->getCalleeIsolation();
-        P.diagnose(InstLoc.getSourceLoc(), diag::unknown_attribute, name);
+        P.diagnose(InstLoc.getSourceLoc(), diag::unknown_attr_name, name);
       }
       if (isolationCrossing->getCallerIsolation() !=
           ActorIsolation::Unspecified) {
         llvm::SmallString<64> name;
         llvm::raw_svector_ostream os(name);
         os << isolationCrossing->getCalleeIsolation();
-        P.diagnose(InstLoc.getSourceLoc(), diag::unknown_attribute, name);
+        P.diagnose(InstLoc.getSourceLoc(), diag::unknown_attr_name, name);
       }
       return true;
     }

Diff for: lib/SIL/Parser/SILParser.h

@@ -411,7 +411,7 @@ class SILParser {
     if (allowed)
       setEnum(existing, value, name, loc);
     else
-      P.diagnose(loc, diag::unknown_attribute, name);
+      P.diagnose(loc, diag::unknown_attr_name, name);
   }
 };
 

Diff for: lib/Sema/TypeCheckAttr.cpp

@@ -4409,7 +4409,8 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) {
       llvm::raw_string_ostream out(typeName);
       typeRepr->print(out);
 
-      Ctx.Diags.diagnose(attr->getLocation(), diag::unknown_attribute, typeName);
+      Ctx.Diags.diagnose(attr->getLocation(), diag::unknown_attr_name,
+                         typeName);
     }
 
     attr->setInvalid();