Refactoring attribute subject diagnostics

[aaron.ballman]

Summary

Currently, attribute subject lists attempt to automatically determine what diagnostic to emit when the attribute does not appertain to a given declaration. It does so by the attribute tablegen emitter looking at the subject list and attempting to match it up with a viable enumerator in AttributeList.h. This approach is starting to show signs of wear now that we are getting more complex attribute subject lists. It also runs into problems where users don't want to add a one-off enumerator and instead use an existing close-but-not-quite-accurate diagnostic.

This patch attempts to improve the situation by removing the need for the enumeration and complex selection logic in the diagnostics tablegen. Instead, each declaration and statement node tablegen description includes a comma-separated list of subjects for diagnostics that the attribute tablegen emitter uses to create a single string to pass to a new diagnostic. This approach is not ideal -- ideally we would have a "list" notion for the diagnostics engine that could handle properly formatting a list so that it handles multiple elements for us. However, I think my current approach is an incremental improvement that gets us a step closer to being able to use such a list mechanism were it to be implemented in the future.

Before I correct all of the test cases for minor diagnostic differences, I wanted to point out those differences explicitly:

• The following were renamed to reduce confusion:
  • methods -> Objective-C methods
  • interfaces -> Objective-C interfaces
  • protocols -> Objective-C protocols
  • fields -> non-static data members
  • types -> typedefs
• We used to consider CXXRecordDecl to only be "classes" while RecordDecl was "struct, union, or class"; we would drop the "or class" part when not compiling in C++ mode. Now that we've switched to a string literal, dropping (or adding) "classes" is more challenging and so RecordDecl always reports "classes" under the assumption that it won't be overly confusing to C users. I am also banking on there being more C++ users than C users, but an alternative may be to drop "classes" from RecordDecl under the assumption that users understand "struct" is the effectively same thing as a "class" in C++.

Note that the actual declarations the attribute appertains to haven't changed, just the spelling of the diagnostics.

If the general approach taken here is reasonable, then I will update the patch with corrected test cases and other polish-related fixes. But since it affects ~30 test cases (due to the updated terminology, consistent use of 'and' instead of 'or', etc), I didn't want to go through the effort unless the patch is going in a reasonable direction.

[aaron.ballman]

Ping

[rsmith]

I think this is a reasonable direction. I'd like to see some kind of %list form added to our diagnostics system, but I don't mind whether that happens before or after this change.

(I'd also prefer to use singular forms in the various subject names, but that seems to create problems with the presence/absence of "a" / "an". If you see a way to make that work, we should consider it, but otherwise don't worry about it.)

[aaron.ballman]

I had the chance to finalize this long-standing review by fixing up the remaining test cases and removing the unused enumerators from AttributeList.h. I've commit in r319002 (but cannot close the review because the "Accept" didn't happen).

[aaron.ballman]

Closing this via "abandon" so that Richard doesn't have to accept it in order for me to close it. It's already been committed.

include/clang/Basic/Attr.td

 //
 // The code fragment is a boolean expression that will confirm that the subject
 // meets the requirements; the subject will have the name S, and will have the
-// type specified by the base. It should be a simple boolean expression.
-class SubsetSubject<AttrSubject base, code check> : AttrSubject {
+// type specified by the base. It should be a simple boolean expression. The
+// diagnostic string should be a comma-separated list of subject names.
+class SubsetSubject<AttrSubject base, code check, string diag> : AttrSubject {
   AttrSubject Base = base;
   code CheckCode = check;
+  string DiagSpelling = diag;
 }
 
 // This is the type of a variable which C++11 allows alignas(...) to appertain

                               [{S->getStorageClass() != VarDecl::Register &&
                                 S->getKind() != Decl::ImplicitParam &&
                                 S->getKind() != Decl::ParmVar &&
-                                S->getKind() != Decl::NonTypeTemplateParm}]>;
-def NonBitField : SubsetSubject<Field,
-                                [{!S->isBitField()}]>;
+                                S->getKind() != Decl::NonTypeTemplateParm}],
+                              "local variables">;
 
-def ObjCInstanceMethod : SubsetSubject<ObjCMethod,
-                                       [{S->isInstanceMethod()}]>;
+def NonBitField : SubsetSubject<Field, [{!S->isBitField()}],
+                                "non-bit-field non-static data members">;
 
+def ObjCInstanceMethod : SubsetSubject<ObjCMethod, [{S->isInstanceMethod()}],
+                                       "Objective-C instance methods">;
+
 def ObjCInterfaceDeclInitMethod : SubsetSubject<ObjCMethod,
                                [{S->getMethodFamily() == OMF_init &&
                                  (isa<ObjCInterfaceDecl>(S->getDeclContext()) ||
                                   (isa<ObjCCategoryDecl>(S->getDeclContext()) &&
-            cast<ObjCCategoryDecl>(S->getDeclContext())->IsClassExtension()))}]>;
+            cast<ObjCCategoryDecl>(S->getDeclContext())->IsClassExtension()))}],
+            "init methods of interface or class extension declarations">;
 
 def Struct : SubsetSubject<Record,
-                           [{!S->isUnion()}]>;
+                           [{!S->isUnion()}], "structs">;
 
 def TLSVar : SubsetSubject<Var,
-                           [{S->getTLSKind() != 0}]>;
+                           [{S->getTLSKind() != 0}], "thread-local variables">;
 
 def SharedVar : SubsetSubject<Var,
-                              [{S->hasGlobalStorage() && !S->getTLSKind()}]>;
+                              [{S->hasGlobalStorage() && !S->getTLSKind()}],
+                              "global variables">;
 
-def GlobalVar : SubsetSubject<Var,
-                             [{S->hasGlobalStorage()}]>;
+def GlobalVar : SubsetSubject<Var, [{S->hasGlobalStorage()}],
+                              "global variables">;
 
 // FIXME: this hack is needed because DeclNodes.td defines the base Decl node
 // type to be a class, not a definition. This makes it impossible to create an

 // the case of a SubsetSubject, there's no way to express it without this hack.
 def DeclBase : AttrSubject;
 def FunctionLike : SubsetSubject<DeclBase,
-                                  [{S->getFunctionType(false) != nullptr}]>;
+                                 [{S->getFunctionType(false) != nullptr}],
+                                 "functions, function pointers">;
 
-def OpenCLKernelFunction : SubsetSubject<Function, [{
-  S->hasAttr<OpenCLKernelAttr>()
-}]>;
+def OpenCLKernelFunction
+    : SubsetSubject<Function, [{S->hasAttr<OpenCLKernelAttr>()}],
+                    "kernel functions">;
 
 // HasFunctionProto is a more strict version of FunctionLike, so it should
 // never be specified in a Subjects list along with FunctionLike (due to the

                                      [{(S->getFunctionType(true) != nullptr &&
                               isa<FunctionProtoType>(S->getFunctionType())) ||
                                        isa<ObjCMethodDecl>(S) ||
-                                       isa<BlockDecl>(S)}]>;
+                                       isa<BlockDecl>(S)}],
+                                     "non-K&R-style functions">;
 
 // A single argument to an attribute
 class Argument<string name, bit optional, bit fake = 0> {

 def AbiTag : Attr {
   let Spellings = [GCC<"abi_tag">];
   let Args = [VariadicStringArgument<"Tags">];
-  let Subjects = SubjectList<[Struct, Var, Function, Namespace], ErrorDiag,
-      "ExpectedStructClassVariableFunctionOrInlineNamespace">;
+  let Subjects = SubjectList<[Struct, Var, Function, Namespace], ErrorDiag>;
   let Documentation = [AbiTagsDocs];
 }
 

 def Alias : Attr {
   let Spellings = [GCC<"alias">];
   let Args = [StringArgument<"Aliasee">];
-  let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag,
-                             "ExpectedFunctionGlobalVarMethodOrProperty">;
+  let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag>;
   let Documentation = [Undocumented];
 }
 

     // , Declspec<"align_value">
   ];
   let Args = [ExprArgument<"Alignment">];
-  let Subjects = SubjectList<[Var, TypedefName], WarnDiag,
-                             "ExpectedVariableOrTypedef">;
+  let Subjects = SubjectList<[Var, TypedefName]>;
   let Documentation = [AlignValueDocs];
 }
 

 
 def TLSModel : InheritableAttr {
   let Spellings = [GCC<"tls_model">];
-  let Subjects = SubjectList<[TLSVar], ErrorDiag, "ExpectedTLSVar">;
+  let Subjects = SubjectList<[TLSVar], ErrorDiag>;
   let Args = [StringArgument<"Model">];
   let Documentation = [TLSModelDocs];
 }

   let Spellings = [GNU<"launch_bounds">];
   let Args = [ExprArgument<"MaxThreads">, ExprArgument<"MinBlocks", 1>];
   let LangOpts = [CUDA];
-  let Subjects = SubjectList<[ObjCMethod, FunctionLike], WarnDiag,
-                             "ExpectedFunctionOrMethod">;
+  let Subjects = SubjectList<[ObjCMethod, FunctionLike]>;
   // An AST node is created for this attribute, but is not used by other parts
   // of the compiler. However, this node needs to exist in the AST because
   // non-LLVM backends may be relying on the attribute's presence.

   let Spellings = [GCC<"format">];
   let Args = [IdentifierArgument<"Type">, IntArgument<"FormatIdx">,
               IntArgument<"FirstArg">];
-  let Subjects = SubjectList<[ObjCMethod, Block, HasFunctionProto], WarnDiag,
-                             "ExpectedFunctionWithProtoType">;
+  let Subjects = SubjectList<[ObjCMethod, Block, HasFunctionProto]>;
   let Documentation = [FormatDocs];
 }
 

 def FormatArg : InheritableAttr {
   let Spellings = [GCC<"format_arg">];
   let Args = [IntArgument<"FormatIdx">];
-  let Subjects = SubjectList<[ObjCMethod, HasFunctionProto], WarnDiag,
-                             "ExpectedFunctionWithProtoType">;
+  let Subjects = SubjectList<[ObjCMethod, HasFunctionProto]>;
   let Documentation = [Undocumented];
 }
 

 
 def IBAction : InheritableAttr {
   let Spellings = [GNU<"ibaction">];
-  let Subjects = SubjectList<[ObjCInstanceMethod], WarnDiag,
-                             "ExpectedObjCInstanceMethod">;
+  let Subjects = SubjectList<[ObjCInstanceMethod]>;
   // An AST node is created for this attribute, but is not used by other parts
   // of the compiler. However, this node needs to exist in the AST because
   // external tools rely on it.

 
 def Mode : Attr {
   let Spellings = [GCC<"mode">];
-  let Subjects = SubjectList<[Var, Enum, TypedefName, Field], ErrorDiag,
-                             "ExpectedVariableEnumFieldOrTypedef">;
+  let Subjects = SubjectList<[Var, Enum, TypedefName, Field], ErrorDiag>;
   let Args = [IdentifierArgument<"Mode">];
   let Documentation = [Undocumented];
 }

 // FIXME: This should be for OpenCLKernelFunction, but is not to
 // workaround needing to see kernel attribute before others to know if
 // this should be rejected on non-kernels.
-  let Subjects = SubjectList<[Function], ErrorDiag,
-                             "ExpectedKernelFunction">;
+  let Subjects = SubjectList<[Function], ErrorDiag, "kernel functions">;
 }
 
 def AMDGPUNumSGPR : InheritableAttr {

   let Spellings = [GNU<"amdgpu_num_sgpr">];
   let Args = [UnsignedArgument<"NumSGPR">];
   let Documentation = [AMDGPUNumSGPRDocs];
-  let Subjects = SubjectList<[Function], ErrorDiag,
-                              "ExpectedKernelFunction">;
+  let Subjects = SubjectList<[Function], ErrorDiag, "kernel functions">;
 }
 
 def NoSplitStack : InheritableAttr {

 def NonNull : InheritableAttr {
   let Spellings = [GCC<"nonnull">];
   let Subjects = SubjectList<[ObjCMethod, HasFunctionProto, ParmVar], WarnDiag,
-                             "ExpectedFunctionMethodOrParameter">;
+                             "functions, methods, and parameters">;
   let Args = [VariadicUnsignedArgument<"Args">];
   let AdditionalMembers =
 [{bool isNonNull(unsigned idx) const {

 
 def ReturnsNonNull : InheritableAttr {
   let Spellings = [GCC<"returns_nonnull">];
-  let Subjects = SubjectList<[ObjCMethod, Function], WarnDiag,
-                             "ExpectedFunctionOrMethod">;
+  let Subjects = SubjectList<[ObjCMethod, Function]>;
   let Documentation = [ReturnsNonNullDocs];
 }
 

 
 def ObjCBridge : InheritableAttr {
   let Spellings = [GNU<"objc_bridge">];
-  let Subjects = SubjectList<[Record, TypedefName], ErrorDiag,
-                             "ExpectedStructOrUnionOrTypedef">;
+  let Subjects = SubjectList<[Record, TypedefName], ErrorDiag>;
   let Args = [IdentifierArgument<"BridgedType">];
   let Documentation = [Undocumented];
 }

 
 def ObjCDesignatedInitializer : Attr {
   let Spellings = [GNU<"objc_designated_initializer">];
-  let Subjects = SubjectList<[ObjCInterfaceDeclInitMethod], ErrorDiag,
-                             "ExpectedObjCInterfaceDeclInitMethod">;
+  let Subjects = SubjectList<[ObjCInterfaceDeclInitMethod], ErrorDiag>;
   let Documentation = [Undocumented];
 }
 

 
 def ObjCBoxable : Attr {
   let Spellings = [GNU<"objc_boxable">];
-  let Subjects = SubjectList<[Record], ErrorDiag, "ExpectedStructOrUnion">;
+  let Subjects = SubjectList<[Record], ErrorDiag>;
   let Documentation = [ObjCBoxableDocs];
 }
 

     }
   }];
   let Args = [IdentifierArgument<"Module">, VariadicUnsignedArgument<"Args">];
-  let Subjects = SubjectList<[HasFunctionProto], WarnDiag,
-                             "ExpectedFunctionWithProtoType">;
+  let Subjects = SubjectList<[HasFunctionProto]>;
   let Documentation = [Undocumented];
 }
 

 def Section : InheritableAttr {
   let Spellings = [GCC<"section">, Declspec<"allocate">];
   let Args = [StringArgument<"Name">];
-  let Subjects = SubjectList<[Function, GlobalVar,
-                              ObjCMethod, ObjCProperty], ErrorDiag,
-                             "ExpectedFunctionGlobalVarMethodOrProperty">;
+  let Subjects =
+      SubjectList<[ Function, GlobalVar, ObjCMethod, ObjCProperty ], ErrorDiag>;
   let Documentation = [SectionDocs];
 }
 

 def Unused : InheritableAttr {
   let Spellings = [CXX11<"", "maybe_unused", 201603>, GCC<"unused">];
   let Subjects = SubjectList<[Var, ObjCIvar, Type, Enum, EnumConstant, Label,
-                              Field, ObjCMethod, FunctionLike], WarnDiag,
-                             "ExpectedForMaybeUnused">;
+                              Field, ObjCMethod, FunctionLike]>;
   let Documentation = [WarnMaybeUnusedDocs];
 }
 

   let Spellings = [CXX11<"", "nodiscard", 201603>,
                    CXX11<"clang", "warn_unused_result">,
                    GCC<"warn_unused_result">];
-  let Subjects = SubjectList<[ObjCMethod, Enum, CXXRecord, FunctionLike],
-                             WarnDiag, "ExpectedFunctionMethodEnumOrClass">;
+  let Subjects = SubjectList<[ObjCMethod, Enum, CXXRecord, FunctionLike]>;
   let Documentation = [WarnUnusedResultsDocs];
 }
 

 
 def GuardedVar : InheritableAttr {
   let Spellings = [GNU<"guarded_var">];
-  let Subjects = SubjectList<[Field, SharedVar], WarnDiag,
-                             "ExpectedFieldOrGlobalVar">;
+  let Subjects = SubjectList<[Field, SharedVar]>;
   let Documentation = [Undocumented];
 }
 
 def PtGuardedVar : InheritableAttr {
   let Spellings = [GNU<"pt_guarded_var">];
-  let Subjects = SubjectList<[Field, SharedVar], WarnDiag,
-                             "ExpectedFieldOrGlobalVar">;
+  let Subjects = SubjectList<[Field, SharedVar]>;
   let Documentation = [Undocumented];
 }
 

   let Spellings = [GNU<"capability">, CXX11<"clang", "capability">,
                    GNU<"shared_capability">,
                    CXX11<"clang", "shared_capability">];
-  let Subjects = SubjectList<[Record, TypedefName], ErrorDiag,
-                             "ExpectedStructOrUnionOrTypedef">;
+  let Subjects = SubjectList<[Record, TypedefName], ErrorDiag>;
   let Args = [StringArgument<"Name">];
   let Accessors = [Accessor<"isShared",
                     [GNU<"shared_capability">,

   let TemplateDependent = 1;
   let ParseArgumentsAsUnevaluated = 1;
   let DuplicatesAllowedWhileMerging = 1;
-  let Subjects = SubjectList<[Field, SharedVar], WarnDiag,
-                             "ExpectedFieldOrGlobalVar">;
+  let Subjects = SubjectList<[Field, SharedVar]>;
   let Documentation = [Undocumented];
 }
 

   let TemplateDependent = 1;
   let ParseArgumentsAsUnevaluated = 1;
   let DuplicatesAllowedWhileMerging = 1;
-  let Subjects = SubjectList<[Field, SharedVar], WarnDiag,
-                             "ExpectedFieldOrGlobalVar">;
+  let Subjects = SubjectList<[Field, SharedVar]>;
   let Documentation = [Undocumented];
 }
 

   let TemplateDependent = 1;
   let ParseArgumentsAsUnevaluated = 1;
   let DuplicatesAllowedWhileMerging = 1;
-  let Subjects = SubjectList<[Field, SharedVar], WarnDiag,
-                             "ExpectedFieldOrGlobalVar">;
+  let Subjects = SubjectList<[Field, SharedVar]>;
   let Documentation = [Undocumented];
 }
 

   let TemplateDependent = 1;
   let ParseArgumentsAsUnevaluated = 1;
   let DuplicatesAllowedWhileMerging = 1;
-  let Subjects = SubjectList<[Field, SharedVar], WarnDiag,
-                             "ExpectedFieldOrGlobalVar">;
+  let Subjects = SubjectList<[Field, SharedVar]>;
   let Documentation = [Undocumented];
 }
 

include/clang/Basic/DeclNodes.td

 class AttrSubject;
 
-class Decl<bit abstract = 0> : AttrSubject {
+class Decl<string diagSpelling = "", bit abstract = 0> : AttrSubject {
   bit Abstract = abstract;
+  string DiagSpelling = diagSpelling;
 }
 
-class DDecl<Decl base, bit abstract = 0> : Decl<abstract> {
+class DDecl<Decl base, string diagSpelling = "", bit abstract = 0>
+    : Decl<diagSpelling, abstract> {
   Decl Base = base;
 }
 
-class DeclContext { }
+class DeclContext {}
 
-def TranslationUnit : Decl, DeclContext;
+def TranslationUnit : Decl,
+                      DeclContext;
 def PragmaComment : Decl;
 def PragmaDetectMismatch : Decl;
 def ExternCContext : Decl, DeclContext;
-def Named : Decl<1>;
-  def Namespace : DDecl<Named>, DeclContext;
+def Named : Decl<"named declarations", 1>;
+  def Namespace : DDecl<Named, "namespaces">, DeclContext;
   def UsingDirective : DDecl<Named>;
   def NamespaceAlias : DDecl<Named>;
-  def Label : DDecl<Named>;
-  def Type : DDecl<Named, 1>;
-    def TypedefName : DDecl<Type, 1>;
+  def Label : DDecl<Named, "labels">;
+  def Type : DDecl<Named, "types", 1>;
+    def TypedefName : DDecl<Type, "typedefs", 1>;
       def Typedef : DDecl<TypedefName>;
       def TypeAlias : DDecl<TypedefName>;
       def ObjCTypeParam : DDecl<TypedefName>;
     def UnresolvedUsingTypename : DDecl<Type>;
-    def Tag : DDecl<Type, 1>, DeclContext;
-      def Enum : DDecl<Tag>;
-      def Record : DDecl<Tag>;
-        def CXXRecord : DDecl<Record>;
+    def Tag : DDecl<Type, "tag types", 1>, DeclContext;
+      def Enum : DDecl<Tag, "enums">;
+      def Record : DDecl<Tag, "structs, unions, classes">;
+        def CXXRecord : DDecl<Record, "classes">;
           def ClassTemplateSpecialization : DDecl<CXXRecord>;
             def ClassTemplatePartialSpecialization
               : DDecl<ClassTemplateSpecialization>;
     def TemplateTypeParm : DDecl<Type>;
-  def Value : DDecl<Named, 1>;
-    def EnumConstant : DDecl<Value>;
+  def Value : DDecl<Named, "value declarations", 1>;
+    def EnumConstant : DDecl<Value, "enumerators">;
     def UnresolvedUsingValue : DDecl<Value>;
     def IndirectField : DDecl<Value>;
     def OMPDeclareReduction : DDecl<Value>, DeclContext;
-    def Declarator : DDecl<Value, 1>;
-      def Field : DDecl<Declarator>;
+    def Declarator : DDecl<Value, "declarators", 1>;
+      def Field : DDecl<Declarator, "non-static data members">;
         def ObjCIvar : DDecl<Field>;
         def ObjCAtDefsField : DDecl<Field>;
       def MSProperty : DDecl<Declarator>;
-      def Function : DDecl<Declarator>, DeclContext;
+      def Function : DDecl<Declarator, "functions">, DeclContext;
         def CXXMethod : DDecl<Function>;
           def CXXConstructor : DDecl<CXXMethod>;
           def CXXDestructor : DDecl<CXXMethod>;
           def CXXConversion : DDecl<CXXMethod>;
-      def Var : DDecl<Declarator>;
+      def Var : DDecl<Declarator, "variables">;
         def VarTemplateSpecialization : DDecl<Var>;
           def VarTemplatePartialSpecialization
             : DDecl<VarTemplateSpecialization>;
         def ImplicitParam : DDecl<Var>;
-        def ParmVar : DDecl<Var>;
+        def ParmVar : DDecl<Var, "parameters">;
         def OMPCapturedExpr : DDecl<Var>;
       def NonTypeTemplateParm : DDecl<Declarator>;
-  def Template : DDecl<Named, 1>;
-    def RedeclarableTemplate : DDecl<Template, 1>;
+  def Template : DDecl<Named, "templates", 1>;
+    def RedeclarableTemplate : DDecl<Template, "redeclarable templates", 1>;
       def FunctionTemplate : DDecl<RedeclarableTemplate>;
       def ClassTemplate : DDecl<RedeclarableTemplate>;
       def VarTemplate : DDecl<RedeclarableTemplate>;

     def BuiltinTemplate : DDecl<Template>;
   def Using : DDecl<Named>;
   def UsingShadow : DDecl<Named>;
-  def ObjCMethod : DDecl<Named>, DeclContext;
-  def ObjCContainer : DDecl<Named, 1>, DeclContext;
+  def ObjCMethod : DDecl<Named, "Objective-C methods">, DeclContext;
+  def ObjCContainer : DDecl<Named, "Objective-C containers", 1>, DeclContext;
     def ObjCCategory : DDecl<ObjCContainer>;
-    def ObjCProtocol : DDecl<ObjCContainer>;
-    def ObjCInterface : DDecl<ObjCContainer>;
-    def ObjCImpl : DDecl<ObjCContainer, 1>;
-      def ObjCCategoryImpl : DDecl<ObjCImpl>;
+    def ObjCProtocol : DDecl<ObjCContainer, "Objective-C protocols">;
+    def ObjCInterface : DDecl<ObjCContainer, "Objective-C interfaces">;
+    def ObjCImpl
+        : DDecl<ObjCContainer, "Objective-C implementation declarations", 1>;
+    def ObjCCategoryImpl : DDecl<ObjCImpl>;
       def ObjCImplementation : DDecl<ObjCImpl>;
-  def ObjCProperty : DDecl<Named>;
+  def ObjCProperty : DDecl<Named, "Objective-C properties">;
   def ObjCCompatibleAlias : DDecl<Named>;
 def LinkageSpec : Decl, DeclContext;
 def ObjCPropertyImpl : Decl;

 def Friend : Decl;
 def FriendTemplate : Decl;
 def StaticAssert : Decl;
-def Block : Decl, DeclContext;
+def Block : Decl<"blocks">, DeclContext;
 def Captured : Decl, DeclContext;
 def ClassScopeFunctionSpecialization : Decl;
 def Import : Decl;

include/clang/Basic/DiagnosticSemaKinds.td

   "definition with same mangled name as another definition">;
 def err_cyclic_alias : Error<
   "alias definition is part of a cycle">;
+def warn_attribute_wrong_decl_type_str
+    : Warning<"%0 attribute only applies to %1">,
+      InGroup<IgnoredAttributes>;
+def err_attribute_wrong_decl_type_str
+    : Error<warn_attribute_wrong_decl_type_str.Text>;
 def warn_attribute_wrong_decl_type : Warning<
   "%0 attribute only applies to %select{functions|unions|"
   "variables and functions|functions and methods|parameters|"

utils/TableGen/ClangAttrEmitter.cpp

   OS << "}\n\n";
 }
 
+static std::string GetDiagnosticSpelling(const Record &R) {
+  std::string Ret = R.getValueAsString("DiagSpelling");
+  if (!Ret.empty())
+    return Ret;
+
+  // If we couldn't find the DiagSpelling in this object, we can check to see
+  // if the object is one that has a base, and if it is, loop up to the Base
+  // member recursively.
+  std::string Super = R.getSuperClasses().back().first->getName();
+  if (Super == "DDecl" || Super == "DStmt")
+    return GetDiagnosticSpelling(*R.getValueAsDef("Base"));
+
+  return "";
+}
+
 static std::string CalculateDiagnostic(const Record &S) {
   // If the SubjectList object has a custom diagnostic associated with it,
   // return that directly.
   std::string CustomDiag = S.getValueAsString("CustomDiag");
   if (!CustomDiag.empty())
-    return CustomDiag;
+    return '"' + CustomDiag + '"';
 
-  // Given the list of subjects, determine what diagnostic best fits.
-  enum {
-    Func = 1U << 0,
-    Var = 1U << 1,
-    ObjCMethod = 1U << 2,
-    Param = 1U << 3,
-    Class = 1U << 4,
-    GenericRecord = 1U << 5,
-    Type = 1U << 6,
-    ObjCIVar = 1U << 7,
-    ObjCProp = 1U << 8,
-    ObjCInterface = 1U << 9,
-    Block = 1U << 10,
-    Namespace = 1U << 11,
-    Field = 1U << 12,
-    CXXMethod = 1U << 13,
-    ObjCProtocol = 1U << 14,
-    Enum = 1U << 15
-  };
-  uint32_t SubMask = 0;
-
+  std::vector<std::string> DiagList;
   std::vector<Record *> Subjects = S.getValueAsListOfDefs("Subjects");
   for (const auto *Subject : Subjects) {
     const Record &R = *Subject;
-    std::string Name;
-
-    if (R.isSubClassOf("SubsetSubject")) {
-      PrintError(R.getLoc(), "SubsetSubjects should use a custom diagnostic");
-      // As a fallback, look through the SubsetSubject to see what its base
-      // type is, and use that. This needs to be updated if SubsetSubjects
-      // are allowed within other SubsetSubjects.
-      Name = R.getValueAsDef("Base")->getName();
-    } else
-      Name = R.getName();
-
-    uint32_t V = StringSwitch<uint32_t>(Name)
-                   .Case("Function", Func)
-                   .Case("Var", Var)
-                   .Case("ObjCMethod", ObjCMethod)
-                   .Case("ParmVar", Param)
-                   .Case("TypedefName", Type)
-                   .Case("ObjCIvar", ObjCIVar)
-                   .Case("ObjCProperty", ObjCProp)
-                   .Case("Record", GenericRecord)
-                   .Case("ObjCInterface", ObjCInterface)
-                   .Case("ObjCProtocol", ObjCProtocol)
-                   .Case("Block", Block)
-                   .Case("CXXRecord", Class)
-                   .Case("Namespace", Namespace)
-                   .Case("Field", Field)
-                   .Case("CXXMethod", CXXMethod)
-                   .Case("Enum", Enum)
-                   .Default(0);
-    if (!V) {
-      // Something wasn't in our mapping, so be helpful and let the developer
-      // know about it.
-      PrintFatalError(R.getLoc(), "Unknown subject type: " + R.getName());
-      return "";
+    // Get the diagnostic text from the Decl or Stmt node given.
+    std::string V = GetDiagnosticSpelling(R);
+    if (V.empty()) {
+      PrintError(R.getLoc(),
+                 "Could not determine diagnostic spelling for the node: " +
+                     R.getName() + "; please add one to DeclNodes.td");
+    } else {
+      // The node may contain a list of elements itself, so split the elements
+      // by a comma, and trim any whitespace.
+      SmallVector<StringRef, 2> Frags;
+      llvm::SplitString(V, Frags, ",");
+      for (auto Str : Frags) {
+        DiagList.push_back(Str.trim());
+      }
     }
+  }
 
-    SubMask |= V;
+  if (DiagList.empty()) {
+    PrintFatalError(S.getLoc(),
+                    "Could not deduce diagnostic argument for Attr subjects");
+    return "";
   }
 
-  switch (SubMask) {
-    // For the simple cases where there's only a single entry in the mask, we
-    // don't have to resort to bit fiddling.
-    case Func:  return "ExpectedFunction";
-    case Var:   return "ExpectedVariable";
-    case Param: return "ExpectedParameter";
-    case Class: return "ExpectedClass";
-    case Enum:  return "ExpectedEnum";
-    case CXXMethod:
-      // FIXME: Currently, this maps to ExpectedMethod based on existing code,
-      // but should map to something a bit more accurate at some point.
-    case ObjCMethod:  return "ExpectedMethod";
-    case Type:  return "ExpectedType";
-    case ObjCInterface: return "ExpectedObjectiveCInterface";
-    case ObjCProtocol: return "ExpectedObjectiveCProtocol";
-    
-    // "GenericRecord" means struct, union or class; check the language options
-    // and if not compiling for C++, strip off the class part. Note that this
-    // relies on the fact that the context for this declares "Sema &S".
-    case GenericRecord:
-      return "(S.getLangOpts().CPlusPlus ? ExpectedStructOrUnionOrClass : "
-                                           "ExpectedStructOrUnion)";
-    case Func | ObjCMethod | Block: return "ExpectedFunctionMethodOrBlock";
-    case Func | ObjCMethod | Class: return "ExpectedFunctionMethodOrClass";
-    case Func | Param:
-    case Func | ObjCMethod | Param: return "ExpectedFunctionMethodOrParameter";
-    case Func | ObjCMethod: return "ExpectedFunctionOrMethod";
-    case Func | Var: return "ExpectedVariableOrFunction";
+  // FIXME: this is not particularly good for localization purposes and ideally
+  // should be part of the diagnostics engine itself with some sort of list
+  // specifier.
 
-    // If not compiling for C++, the class portion does not apply.
-    case Func | Var | Class:
-      return "(S.getLangOpts().CPlusPlus ? ExpectedFunctionVariableOrClass : "
-                                           "ExpectedVariableOrFunction)";
+  // A single member of the list can be returned directly.
+  if (DiagList.size() == 1)
+    return '"' + DiagList.front() + '"';
 
-    case ObjCMethod | ObjCProp: return "ExpectedMethodOrProperty";
-    case ObjCProtocol | ObjCInterface:
-      return "ExpectedObjectiveCInterfaceOrProtocol";
-    case Field | Var: return "ExpectedFieldOrGlobalVar";
-  }
+  if (DiagList.size() == 2)
+    return '"' + DiagList[0] + " and " + DiagList[1] + '"';
 
-  PrintFatalError(S.getLoc(),
-                  "Could not deduce diagnostic argument for Attr subjects");
-
-  return "";
+  // If there are more than two in the list, we serialize the first N - 1
+  // elements with a comma. This leaves the string in the state: foo, bar,
+  // baz (but misses quux). We can then add ", and " for the last element
+  // manually.
+  std::string Diag = llvm::join(DiagList.begin(), DiagList.end() - 1, ", ");
+  return '"' + Diag + ", and " + *(DiagList.end() - 1) + '"';
 }
 
 static std::string GetSubjectWithSuffix(const Record *R) {

   }
   SS << ") {\n";
   SS << "    S.Diag(Attr.getLoc(), diag::";
-  SS << (Warn ? "warn_attribute_wrong_decl_type" :
-               "err_attribute_wrong_decl_type");
+  SS << (Warn ? "warn_attribute_wrong_decl_type_str" :
+               "err_attribute_wrong_decl_type_str");
   SS << ")\n";
   SS << "      << Attr.getName() << ";
   SS << CalculateDiagnostic(*SubjectObj) << ";\n";