[Clang] include attribute scope in diagnostics

Author: a-tarasyuk

clang/include/clang/Basic/AttributeCommonInfo.h

@@ -15,6 +15,7 @@
 #define LLVM_CLANG_BASIC_ATTRIBUTECOMMONINFO_H
 
 #include "clang/Basic/AttributeScopeInfo.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TokenKinds.h"
 
@@ -175,6 +176,10 @@ class AttributeCommonInfo {
       : AttributeCommonInfo(nullptr, AttributeScopeInfo(), AttrRange, K,
                             FormUsed) {}
 
+  AttributeCommonInfo(SourceRange AttrRange, AttributeScopeInfo AttrScope,
+                      Kind K, Form FormUsed)
+      : AttributeCommonInfo(nullptr, AttrScope, AttrRange, K, FormUsed) {}
+
   AttributeCommonInfo(AttributeCommonInfo &&) = default;
   AttributeCommonInfo(const AttributeCommonInfo &) = default;
 
@@ -292,6 +297,45 @@ inline bool doesKeywordAttributeTakeArgs(tok::TokenKind Kind) {
   }
 }
 
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+                                             const AttributeCommonInfo &CI) {
+  DB.AddTaggedVal(reinterpret_cast<uint64_t>(&CI),
+                  DiagnosticsEngine::ak_attr_info);
+  return DB;
+}
+
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+                                             const AttributeCommonInfo *CI) {
+  DB.AddTaggedVal(reinterpret_cast<uint64_t>(CI),
+                  DiagnosticsEngine::ak_attr_info);
+  return DB;
+}
+
+/// AttributeCommonInfo has a non-explicit constructor which takes an
+/// SourceRange as its only argument, this constructor has many uses so making
+/// it explicit is hard. This constructor causes ambiguity with
+/// DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB, SourceRange R).
+/// We use SFINAE to disable any conversion and remove any ambiguity.
+template <
+    typename ACI,
+    std::enable_if_t<std::is_same<ACI, AttributeCommonInfo>::value, int> = 0>
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+                                             const ACI &CI) {
+  DB.AddTaggedVal(reinterpret_cast<uint64_t>(&CI),
+                  DiagnosticsEngine::ak_attr_info);
+  return DB;
+}
+
+template <
+    typename ACI,
+    std::enable_if_t<std::is_same<ACI, AttributeCommonInfo>::value, int> = 0>
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+                                             const ACI *CI) {
+  DB.AddTaggedVal(reinterpret_cast<uint64_t>(CI),
+                  DiagnosticsEngine::ak_attr_info);
+  return DB;
+}
+
 } // namespace clang
 
 #endif // LLVM_CLANG_BASIC_ATTRIBUTECOMMONINFO_H

clang/include/clang/Basic/Diagnostic.h

@@ -289,6 +289,9 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
 
     /// Expr *
     ak_expr,
+
+    /// AttributeCommonInfo *
+    ak_attr_info,
   };
 
   /// Represents on argument value, which is a union discriminated

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -3275,10 +3275,11 @@ def err_attribute_wrong_number_arguments : Error<
 def err_attribute_wrong_number_arguments_for : Error <
   "%0 attribute references function %1, which %plural{0:takes no arguments|1:takes one argument|"
   ":takes exactly %2 arguments}2">;
-def err_callback_attribute_wrong_arg_count : Error<
-  "'callback' attribute references function of type %0 which expects %1 "
-  "%plural{1:argument|:arguments}1 but attribute specifies %2 parameter index "
-  "%plural{1:argument|:arguments}2">;
+def err_callback_attribute_wrong_arg_count
+    : Error<"%0 attribute references function of type %1 which expects %2 "
+            "%plural{1:argument|:arguments}2 but attribute specifies %3 "
+            "parameter index "
+            "%plural{1:argument|:arguments}3">;
 def err_attribute_bounds_for_function : Error<
   "%0 attribute references parameter %1, but the function %2 has only %3 parameters">;
 def err_attribute_no_member_function : Error<
@@ -4694,9 +4695,9 @@ def note_protocol_decl : Note<
   "protocol is declared here">;
 def note_protocol_decl_undefined : Note<
   "protocol %0 has no definition">;
-def err_attribute_preferred_name_arg_invalid : Error<
-  "argument %0 to 'preferred_name' attribute is not a typedef for "
-  "a specialization of %1">;
+def err_attribute_preferred_name_arg_invalid
+    : Error<"argument %0 to %1 attribute is not a typedef for "
+            "a specialization of %2">;
 def err_attribute_builtin_alias : Error<
   "%0 attribute can only be applied to a ARM, HLSL, SPIR-V or RISC-V builtin">;
 

clang/include/clang/Sema/ParsedAttr.h

@@ -1094,45 +1094,6 @@ enum AttributeDeclKind {
   ExpectedTypedef,
 };
 
-inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
-                                             const ParsedAttr &At) {
-  DB.AddTaggedVal(reinterpret_cast<uint64_t>(At.getAttrName()),
-                  DiagnosticsEngine::ak_identifierinfo);
-  return DB;
-}
-
-inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
-                                             const ParsedAttr *At) {
-  DB.AddTaggedVal(reinterpret_cast<uint64_t>(At->getAttrName()),
-                  DiagnosticsEngine::ak_identifierinfo);
-  return DB;
-}
-
-/// AttributeCommonInfo has a non-explicit constructor which takes an
-/// SourceRange as its only argument, this constructor has many uses so making
-/// it explicit is hard. This constructor causes ambiguity with
-/// DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB, SourceRange R).
-/// We use SFINAE to disable any conversion and remove any ambiguity.
-template <
-    typename ACI,
-    std::enable_if_t<std::is_same<ACI, AttributeCommonInfo>::value, int> = 0>
-inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
-                                             const ACI &CI) {
-  DB.AddTaggedVal(reinterpret_cast<uint64_t>(CI.getAttrName()),
-                  DiagnosticsEngine::ak_identifierinfo);
-  return DB;
-}
-
-template <
-    typename ACI,
-    std::enable_if_t<std::is_same<ACI, AttributeCommonInfo>::value, int> = 0>
-inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
-                                             const ACI *CI) {
-  DB.AddTaggedVal(reinterpret_cast<uint64_t>(CI->getAttrName()),
-                  DiagnosticsEngine::ak_identifierinfo);
-  return DB;
-}
-
 } // namespace clang
 
 #endif // LLVM_CLANG_SEMA_PARSEDATTR_H

clang/lib/AST/ASTDiagnostic.cpp

@@ -506,7 +506,15 @@ void clang::FormatASTNodeDiagnosticArgument(
     case DiagnosticsEngine::ak_attr: {
       const Attr *At = reinterpret_cast<Attr *>(Val);
       assert(At && "Received null Attr object!");
-      OS << '\'' << At->getSpelling() << '\'';
+
+      OS << '\'';
+      if (At->hasScope()) {
+        OS << At->getNormalizedFullName(At->getScopeName()->getName(),
+                                        At->getSpelling());
+      } else {
+        OS << At->getSpelling();
+      }
+      OS << '\'';
       NeedQuotes = false;
       break;
     }
@@ -516,6 +524,20 @@ void clang::FormatASTNodeDiagnosticArgument(
       E->printPretty(OS, /*Helper=*/nullptr, Context.getPrintingPolicy());
       break;
     }
+    case DiagnosticsEngine::ak_attr_info: {
+      AttributeCommonInfo *AT = reinterpret_cast<AttributeCommonInfo *>(Val);
+      assert(AT && "Received null AttributeCommonInfo object!");
+
+      OS << '\'';
+      if (AT->isStandardAttributeSyntax()) {
+        OS << AT->getNormalizedFullName();
+      } else {
+        OS << AT->getAttrName()->getName();
+      }
+      OS << '\'';
+      NeedQuotes = false;
+      break;
+    }
   }
 
   if (NeedQuotes) {

clang/lib/Basic/Diagnostic.cpp

@@ -1349,6 +1349,7 @@ void Diagnostic::FormatDiagnostic(const char *DiagStr, const char *DiagEnd,
     case DiagnosticsEngine::ak_declcontext:
     case DiagnosticsEngine::ak_attr:
     case DiagnosticsEngine::ak_expr:
+    case DiagnosticsEngine::ak_attr_info:
       getDiags()->ConvertArgToString(Kind, getRawArg(ArgNo),
                                      StringRef(Modifier, ModifierLen),
                                      StringRef(Argument, ArgumentLen),

clang/lib/Sema/SemaDeclAttr.cpp

@@ -1226,7 +1226,7 @@ static void handlePreferredName(Sema &S, Decl *D, const ParsedAttr &AL) {
   }
 
   S.Diag(AL.getLoc(), diag::err_attribute_preferred_name_arg_invalid)
-      << T << CTD;
+      << T << AL << CTD;
   if (const auto *TT = T->getAs<TypedefType>())
     S.Diag(TT->getDecl()->getLocation(), diag::note_entity_declared_at)
         << TT->getDecl();
@@ -4147,7 +4147,8 @@ static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   if (CalleeFnProtoType->getNumParams() != EncodingIndices.size() - 1) {
     S.Diag(AL.getLoc(), diag::err_callback_attribute_wrong_arg_count)
-        << QualType{CalleeFnProtoType, 0} << CalleeFnProtoType->getNumParams()
+        << AL << QualType{CalleeFnProtoType, 0}
+        << CalleeFnProtoType->getNumParams()
         << (unsigned)(EncodingIndices.size() - 1);
     return;
   }
@@ -7969,9 +7970,7 @@ void Sema::checkUnusedDeclAttributes(Declarator &D) {
 }
 
 void Sema::DiagnoseUnknownAttribute(const ParsedAttr &AL) {
-  std::string NormalizedFullName = '\'' + AL.getNormalizedFullName() + '\'';
   SourceRange NR = AL.getNormalizedRange();
-
   StringRef ScopeName = AL.getNormalizedScopeName();
   std::optional<StringRef> CorrectedScopeName =
       AL.tryGetCorrectedScopeName(ScopeName);
@@ -7993,7 +7992,7 @@ void Sema::DiagnoseUnknownAttribute(const ParsedAttr &AL) {
         Diag(CorrectedScopeName ? NR.getBegin() : AL.getRange().getBegin(),
              diag::warn_unknown_attribute_ignored_suggestion);
 
-    D << NormalizedFullName << CorrectedFullName;
+    D << AL << CorrectedFullName;
 
     if (AL.isExplicitScope()) {
       D << FixItHint::CreateReplacement(NR, CorrectedFullName) << NR;
@@ -8007,8 +8006,7 @@ void Sema::DiagnoseUnknownAttribute(const ParsedAttr &AL) {
       }
     }
   } else {
-    Diag(NR.getBegin(), diag::warn_unknown_attribute_ignored)
-        << NormalizedFullName << NR;
+    Diag(NR.getBegin(), diag::warn_unknown_attribute_ignored) << AL << NR;
   }
 }
 

clang/lib/Sema/SemaHLSL.cpp

@@ -1382,6 +1382,15 @@ bool SemaHLSL::handleResourceTypeAttr(QualType T, const ParsedAttr &AL) {
     return false;
 
   Attr *A = nullptr;
+
+  AttributeCommonInfo ACI(
+      AL.getLoc(), AttributeScopeInfo(AL.getScopeName(), AL.getScopeLoc()),
+      AttributeCommonInfo::NoSemaHandlerAttribute,
+      {
+          AttributeCommonInfo::AS_CXX11, 0, false /*IsAlignas*/,
+          false /*IsRegularKeywordAttribute*/
+      });
+
   switch (AL.getKind()) {
   case ParsedAttr::AT_HLSLResourceClass: {
     if (!AL.isArgIdent(0)) {
@@ -1401,16 +1410,16 @@ bool SemaHLSL::handleResourceTypeAttr(QualType T, const ParsedAttr &AL) {
           << "ResourceClass" << Identifier;
       return false;
     }
-    A = HLSLResourceClassAttr::Create(getASTContext(), RC, AL.getLoc());
+    A = HLSLResourceClassAttr::Create(getASTContext(), RC, ACI);
     break;
   }
 
   case ParsedAttr::AT_HLSLROV:
-    A = HLSLROVAttr::Create(getASTContext(), AL.getLoc());
+    A = HLSLROVAttr::Create(getASTContext(), ACI);
     break;
 
   case ParsedAttr::AT_HLSLRawBuffer:
-    A = HLSLRawBufferAttr::Create(getASTContext(), AL.getLoc());
+    A = HLSLRawBufferAttr::Create(getASTContext(), ACI);
     break;
 
   case ParsedAttr::AT_HLSLContainedType: {
@@ -1425,7 +1434,7 @@ bool SemaHLSL::handleResourceTypeAttr(QualType T, const ParsedAttr &AL) {
     if (SemaRef.RequireCompleteType(TSI->getTypeLoc().getBeginLoc(), QT,
                                     diag::err_incomplete_type))
       return false;
-    A = HLSLContainedTypeAttr::Create(getASTContext(), TSI, AL.getLoc());
+    A = HLSLContainedTypeAttr::Create(getASTContext(), TSI, ACI);
     break;
   }
 

clang/test/AST/ByteCode/functions.cpp

@@ -637,7 +637,7 @@ namespace {
 
 namespace {
   /// The InitListExpr here is of void type.
-  void bir [[clang::annotate("B", {1, 2, 3, 4})]] (); // both-error {{'annotate' attribute requires parameter 1 to be a constant expression}} \
+  void bir [[clang::annotate("B", {1, 2, 3, 4})]] (); // both-error {{'clang::annotate' attribute requires parameter 1 to be a constant expression}} \
                                                       // both-note {{subexpression not valid in a constant expression}}
 }
 

clang/test/C/C23/n3037.c

@@ -60,7 +60,7 @@ union purr { float y; int x; }; // c23-error {{type 'union purr' has incompatibl
 
 // The presence of an attribute makes two types not compatible.
 struct [[gnu::packed]] attr_test { // c17-note {{previous definition is here}} \
-                                      c23-note {{attribute 'packed' here}}
+                                      c23-note {{attribute 'gnu::packed' here}}
   int x;
 };
 
@@ -75,26 +75,26 @@ struct attr_test_2 { // c17-note {{previous definition is here}}
 
 struct [[gnu::packed]] attr_test_2 { // c17-error {{redefinition of 'attr_test_2'}} \
                                         c23-error {{type 'struct attr_test_2' has an attribute which currently causes the types to be treated as though they are incompatible}} \
-                                        c23-note {{attribute 'packed' here}}
+                                        c23-note {{attribute 'gnu::packed' here}}
   int x;
 };
 
 // This includes the same attribute on both types.
 struct [[gnu::packed]] attr_test_3 { // c17-note {{previous definition is here}} \
-                                       c23-note {{attribute 'packed' here}}
+                                       c23-note {{attribute 'gnu::packed' here}}
   int x;
 };
 
 struct [[gnu::packed]] attr_test_3 { // c17-error {{redefinition of 'attr_test_3'}} \
                                         c23-error {{type 'struct attr_test_3' has an attribute which currently causes the types to be treated as though they are incompatible}} \
-                                        c23-note {{attribute 'packed' here}}
+                                        c23-note {{attribute 'gnu::packed' here}}
   int x;
 };
 
 // Everything which applies to the tag itself also applies to fields.
 struct field_attr_test_1 { // c17-note {{previous definition is here}}
   int x;
-  [[gnu::packed]] int y; // c23-note {{attribute 'packed' here}}
+  [[gnu::packed]] int y; // c23-note {{attribute 'gnu::packed' here}}
 };
 
 struct field_attr_test_1 { // c17-error {{redefinition of 'field_attr_test_1'}} \
@@ -104,7 +104,7 @@ struct field_attr_test_1 { // c17-error {{redefinition of 'field_attr_test_1'}}
 };
 
 struct field_attr_test_2 { // c17-note {{previous definition is here}}
-  [[gnu::packed]] int x; // c23-note {{attribute 'packed' here}}
+  [[gnu::packed]] int x; // c23-note {{attribute 'gnu::packed' here}}
   int y;
 };
 
@@ -115,13 +115,13 @@ struct field_attr_test_2 { // c17-error {{redefinition of 'field_attr_test_2'}}
 };
 
 struct field_attr_test_3 { // c17-note {{previous definition is here}}
-  [[gnu::packed]] int x;   // c23-note {{attribute 'packed' here}}
+  [[gnu::packed]] int x;   // c23-note {{attribute 'gnu::packed' here}}
   int y;
 };
 
 struct field_attr_test_3 { // c17-error {{redefinition of 'field_attr_test_3'}} \
                               c23-error {{type 'struct field_attr_test_3' has a member with an attribute which currently causes the types to be treated as though they are incompatible}}
-  int x [[gnu::packed]];   // c23-note {{attribute 'packed' here}}
+  int x [[gnu::packed]];   // c23-note {{attribute 'gnu::packed' here}}
   int y;
 };