Add #pragma clang attribute support for the external_source_symbol attribute

[arphaman]

Summary

Right now the external_source_symbol attribute isn't supported by #pragma clang attribute for the following two reasons:

• The Named attribute subject isn't supported by TableGen.
• There was no way to specify a subject match rule for #pragma clang attribute that could operate on a set of attribute subjects (e.g. the ones that derive from NamedDecl).

This patch fixes the two issues and thus adds external_source_symbol support to #pragma clang attribute,

[arphaman]

Avoid a vector copy by using a reference variable.

[aaron.ballman]

lib/Sema/SemaDeclAttr.cpp

5769

Instead of duplicating this check in the if statement, why not hoist the current check above the if statement?

utils/TableGen/ClangAttrEmitter.cpp

1620

ArrayRef automatically converts to a std::vector, so you should just be able to use Rules(Rules).

1625

Any reason not to return this rule by const ref and make the function const?

1701

Can use const auto * here

[arphaman]

Addressed Aaron's comments.

lib/Sema/SemaDeclAttr.cpp

5781

I hoisted it after diagnoseLangOpts to keep some tests happy.

[aaron.ballman]

LGTM

include/clang/Basic/Attr.td

 def SubjectMatcherForObjCProperty : AttrSubjectMatcherRule<"objc_property",
                                                            [ObjCProperty]> {
   let LangOpts = [ObjC];
 }
 def SubjectMatcherForBlock : AttrSubjectMatcherRule<"block", [Block]> {
   let LangOpts = [BlocksSupported];
 }
 
+// Aggregate attribute subject match rules are abstract match rules that can't
+// be used directly in #pragma clang attribute. Instead, users have to use
+// subject match rules that correspond to attribute subjects that derive from
+// the specified subject.
+class AttrSubjectMatcherAggregateRule<AttrSubject subject> {
+  AttrSubject Subject = subject;
+}
+
+def SubjectMatcherForNamed : AttrSubjectMatcherAggregateRule<Named>;
+
 class Attr {
   // The various ways in which an attribute can be spelled in source
   list<Spelling> Spellings;
   // The things to which an attribute can appertain
   SubjectList Subjects;
   // The arguments allowed on an attribute
   list<Argument> Args = [];
   // Accessors which should be generated for the attribute.

 
 def ExternalSourceSymbol : InheritableAttr {
   let Spellings = [GNU<"external_source_symbol">,
                    CXX11<"clang", "external_source_symbol">];
   let Args = [StringArgument<"language", 1>,
               StringArgument<"definedIn", 1>,
               BoolArgument<"generatedDeclaration", 1>];
   let HasCustomParsing = 1;
-//  let Subjects = SubjectList<[Named]>;
+  let Subjects = SubjectList<[Named]>;
   let Documentation = [ExternalSourceSymbolDocs];
 }
 
 def Blocks : InheritableAttr {
   let Spellings = [GNU<"blocks">];
   let Args = [EnumArgument<"Type", "BlockType", ["byref"], ["ByRef"]>];
   let Documentation = [Undocumented];
 }

lib/Sema/SemaDeclAttr.cpp

 
 static void handleExternalSourceSymbolAttr(Sema &S, Decl *D,
                                            const AttributeList &Attr) {
   if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
     return;
   assert(checkAttributeAtMostNumArgs(S, Attr, 3) &&
          "Invalid number of arguments in an external_source_symbol attribute");
 
-  if (!isa<NamedDecl>(D)) {
-    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-        << Attr.getName() << ExpectedNamedDecl;
-    return;
-  }
-
   StringRef Language;
   if (const auto *SE = dyn_cast_or_null<StringLiteral>(Attr.getArgAsExpr(0)))
     Language = SE->getString();
   StringRef DefinedIn;
   if (const auto *SE = dyn_cast_or_null<StringLiteral>(Attr.getArgAsExpr(1)))
     DefinedIn = SE->getString();
   bool IsGeneratedDeclaration = Attr.getArgAsIdent(2) != nullptr;
 

 }
 
 /// Handles semantic checking for features that are common to all attributes,
 /// such as checking whether a parameter was properly specified, or the correct
 /// number of arguments were passed, etc.
 static bool handleCommonAttributeFeatures(Sema &S, Scope *scope, Decl *D,
                                           const AttributeList &Attr) {
   // Several attributes carry different semantics than the parsing requires, so
-  // those are opted out of the common handling.
+  // those are opted out of the majority of common handling.
   //
   // We also bail on unknown and ignored attributes because those are handled
   // as part of the target-specific handling logic.
-  if (Attr.hasCustomParsing() ||
-      Attr.getKind() == AttributeList::UnknownAttribute)
+  if (Attr.hasCustomParsing()) {
+    // Still check whether the attribute appertains to the given subject as it
+    // might include a subject list even when it has custom parsing.
+    if (!Attr.diagnoseAppertainsTo(S, D))
+      return true;
+    return false;
+  }
+  if (Attr.getKind() == AttributeList::UnknownAttribute)
     return false;
 
   // Check whether the attribute requires specific language extensions to be
   // enabled.
   if (!Attr.diagnoseLangOpts(S))
     return true;
 
   if (Attr.getMinArgs() == Attr.getMaxArgs()) {
     // If there are no optional arguments, then checking for the argument count
     // is trivial.
     if (!checkAttributeNumArgs(S, Attr, Attr.getMinArgs()))
       return true;
   } else {
     // There are optional arguments, so checking is slightly more involved.
     if (Attr.getMinArgs() &&
         !checkAttributeAtLeastNumArgs(S, Attr, Attr.getMinArgs()))

test/Misc/pragma-attribute-supported-attributes-list.test

 // RUN: clang-tblgen -gen-clang-test-pragma-attribute-supported-attributes -I%src_include_dir %src_include_dir/clang/Basic/Attr.td -o - | FileCheck %s
 
 // The number of supported attributes should never go down!
 
-// CHECK: #pragma clang attribute supports 57 attributes:
+// CHECK: #pragma clang attribute supports 58 attributes:
 // CHECK-NEXT: AMDGPUFlatWorkGroupSize (SubjectMatchRule_function)
 // CHECK-NEXT: AMDGPUNumSGPR (SubjectMatchRule_function)
 // CHECK-NEXT: AMDGPUNumVGPR (SubjectMatchRule_function)
 // CHECK-NEXT: AMDGPUWavesPerEU (SubjectMatchRule_function)
 // CHECK-NEXT: AVRSignal (SubjectMatchRule_function)
 // CHECK-NEXT: AbiTag (SubjectMatchRule_record_not_is_union, SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_namespace)
 // CHECK-NEXT: AlignValue (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
 // CHECK-NEXT: AllocSize (SubjectMatchRule_function)
 // CHECK-NEXT: Annotate ()
 // CHECK-NEXT: AssumeAligned (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: CXX11NoReturn (SubjectMatchRule_function)
 // CHECK-NEXT: CallableWhen (SubjectMatchRule_function_is_member)
 // CHECK-NEXT: CarriesDependency (SubjectMatchRule_variable_is_parameter, SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: Consumable (SubjectMatchRule_record)
 // CHECK-NEXT: Convergent (SubjectMatchRule_function)
 // CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
 // CHECK-NEXT: EnumExtensibility (SubjectMatchRule_enum)
+// CHECK-NEXT: ExternalSourceSymbol ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: FlagEnum (SubjectMatchRule_enum)
 // CHECK-NEXT: Flatten (SubjectMatchRule_function)
 // CHECK-NEXT: IFunc (SubjectMatchRule_function)
 // CHECK-NEXT: InternalLinkage (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)
 // CHECK-NEXT: LTOVisibilityPublic (SubjectMatchRule_record)
 // CHECK-NEXT: NoDebug (SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, SubjectMatchRule_variable_not_is_parameter)
 // CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)

utils/TableGen/ClangAttrEmitter.cpp

struct AttributeSubjectMatchRule {
 
   static const char *EnumName;
 };
 
 const char *AttributeSubjectMatchRule::EnumName = "attr::SubjectMatchRule";
 
 struct PragmaClangAttributeSupport {
   std::vector<AttributeSubjectMatchRule> Rules;
-  llvm::DenseMap<const Record *, AttributeSubjectMatchRule> SubjectsToRules;
+
+  class RuleOrAggregateRuleSet {
+    std::vector<AttributeSubjectMatchRule> Rules;
+    bool IsRule;
+    RuleOrAggregateRuleSet(ArrayRef<AttributeSubjectMatchRule> Rules,
+                           bool IsRule)
+        : Rules(Rules.begin(), Rules.end()), IsRule(IsRule) {}
+
+  public:
+    bool isRule() const { return IsRule; }
+
+    AttributeSubjectMatchRule &getRule() {
+      assert(IsRule && "not a rule!");
+      return Rules[0];
+    }
+
+    ArrayRef<AttributeSubjectMatchRule> getAggregateRuleSet() const {
+      return Rules;
+    }
+
+    static RuleOrAggregateRuleSet
+    getRule(const AttributeSubjectMatchRule &Rule) {
+      return RuleOrAggregateRuleSet(Rule, /*IsRule=*/true);
+    }
+    static RuleOrAggregateRuleSet
+    getAggregateRuleSet(ArrayRef<AttributeSubjectMatchRule> Rules) {
+      return RuleOrAggregateRuleSet(Rules, /*IsRule=*/false);
+    }
+  };
+  llvm::DenseMap<const Record *, RuleOrAggregateRuleSet> SubjectsToRules;
 
   PragmaClangAttributeSupport(RecordKeeper &Records);
 
   bool isAttributedSupported(const Record &Attribute);
 
   void emitMatchRuleList(raw_ostream &OS);
 
   std::string generateStrictConformsTo(const Record &Attr, raw_ostream &OS);
 
   void generateParsingHelpers(raw_ostream &OS);
 };
 
 } // end anonymous namespace
 
+static bool doesDeclDeriveFrom(const Record *D, const Record *Base) {
+  const Record *CurrentBase = D->getValueAsDef("Base");
+  if (!CurrentBase)
+    return false;
+  if (CurrentBase == Base)
+    return true;
+  return doesDeclDeriveFrom(CurrentBase, Base);
+}
+
 PragmaClangAttributeSupport::PragmaClangAttributeSupport(
     RecordKeeper &Records) {
   std::vector<Record *> MetaSubjects =
       Records.getAllDerivedDefinitions("AttrSubjectMatcherRule");
   auto MapFromSubjectsToRules = [this](const Record *SubjectContainer,
                                        const Record *MetaSubject,
                                        const Record *Constraint = nullptr) {
     Rules.emplace_back(MetaSubject, Constraint);
     std::vector<Record *> ApplicableSubjects =
         SubjectContainer->getValueAsListOfDefs("Subjects");
     for (const auto *Subject : ApplicableSubjects) {
       bool Inserted =
-          SubjectsToRules.try_emplace(Subject, MetaSubject, Constraint).second;
+          SubjectsToRules
+              .try_emplace(Subject, RuleOrAggregateRuleSet::getRule(
+                                        AttributeSubjectMatchRule(MetaSubject,
+                                                                  Constraint)))
+              .second;
       if (!Inserted) {
         PrintFatalError("Attribute subject match rules should not represent"
                         "same attribute subjects.");
       }
     }
   };
   for (const auto *MetaSubject : MetaSubjects) {
     MapFromSubjectsToRules(MetaSubject, MetaSubject);
     std::vector<Record *> Constraints =
         MetaSubject->getValueAsListOfDefs("Constraints");
     for (const auto *Constraint : Constraints)
       MapFromSubjectsToRules(Constraint, MetaSubject, Constraint);
   }
+
+  std::vector<Record *> Aggregates =
+      Records.getAllDerivedDefinitions("AttrSubjectMatcherAggregateRule");
+  std::vector<Record *> DeclNodes = Records.getAllDerivedDefinitions("DDecl");
+  for (const Record *Aggregate : Aggregates) {
+    Record *SubjectDecl = Aggregate->getValueAsDef("Subject");
+
+    // Gather sub-classes of the aggregate subject that act as attribute
+    // subject rules.
+    std::vector<AttributeSubjectMatchRule> Rules;
+    for (const auto *D : DeclNodes) {
+      if (doesDeclDeriveFrom(D, SubjectDecl)) {
+        auto It = SubjectsToRules.find(D);
+        if (It == SubjectsToRules.end())
+          continue;
+        if (!It->second.isRule() || It->second.getRule().isSubRule())
+          continue; // Assume that the rule will be included as well.
+        Rules.push_back(It->second.getRule());
+      }
+    }
+
+    bool Inserted =
+        SubjectsToRules
+            .try_emplace(SubjectDecl,
+                         RuleOrAggregateRuleSet::getAggregateRuleSet(Rules))
+            .second;
+    if (!Inserted) {
+      PrintFatalError("Attribute subject match rules should not represent"
+                      "same attribute subjects.");
+    }
+  }
 }
 
 static PragmaClangAttributeSupport &
 getPragmaAttributeSupport(RecordKeeper &Records) {
   static PragmaClangAttributeSupport Instance(Records);
   return Instance;
 }
 
if (Attr.isValueUnset("Subjects")) {
     return FnName;
   }
   const Record *SubjectObj = Attr.getValueAsDef("Subjects");
   std::vector<Record *> Subjects = SubjectObj->getValueAsListOfDefs("Subjects");
   for (const auto *Subject : Subjects) {
     auto It = SubjectsToRules.find(Subject);
     assert(It != SubjectsToRules.end() &&
            "This attribute is unsupported by #pragma clang attribute");
-    AttributeSubjectMatchRule Rule = It->getSecond();
+    for (const auto &Rule : It->getSecond().getAggregateRuleSet()) {
       // The rule might be language specific, so only subtract it from the given
       // rules if the specific language options are specified.
       std::vector<Record *> LangOpts = Rule.getLangOpts();
       SS << "  MatchRules.push_back(std::make_pair(" << Rule.getEnumValue()
          << ", /*IsSupported=*/";
       if (!LangOpts.empty()) {
         for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
           std::string Part = (*I)->getValueAsString("Name");
           if ((*I)->getValueAsBit("Negated"))
             SS << "!";
           SS << "LangOpts." + Part;
           if (I + 1 != E)
             SS << " || ";
         }
       } else
         SS << "true";
       SS << "));\n";
     }
+  }
   SS << "}\n\n";
   OS << SS.str();
   return FnName;
 }
 
 void PragmaClangAttributeSupport::generateParsingHelpers(raw_ostream &OS) {
   // Generate routines that check the names of sub-rules.
   OS << "Optional<attr::SubjectMatchRule> "
enum {
     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
+    Enum = 1U << 15,
+    Named = 1U << 16,
   };
   uint32_t SubMask = 0;
 
   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)
+                     .Case("Named", Named)
                      .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 "";
     }
 
     SubMask |= V;
switch (SubMask) {
     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 | Var | Class | ObjCInterface:
              "     : ((S.getLangOpts().ObjC1 || S.getLangOpts().ObjC2)"
              "            ? ExpectedFunctionVariableOrObjCInterface"
              "            : ExpectedVariableOrFunction))";
 
     case ObjCMethod | ObjCProp: return "ExpectedMethodOrProperty";
     case ObjCProtocol | ObjCInterface:
       return "ExpectedObjectiveCInterfaceOrProtocol";
     case Field | Var: return "ExpectedFieldOrGlobalVar";
+
+    case Named:
+      return "ExpectedNamedDecl";
   }
 
   PrintFatalError(S.getLoc(),
                   "Could not deduce diagnostic argument for Attr subjects");
 
   return "";
 }
 
for (const auto &I : Attrs) {
     }
     const Record *SubjectObj = I.second->getValueAsDef("Subjects");
     std::vector<Record *> Subjects =
         SubjectObj->getValueAsListOfDefs("Subjects");
     OS << " (";
     for (const auto &Subject : llvm::enumerate(Subjects)) {
       if (Subject.index())
         OS << ", ";
-      OS << Support.SubjectsToRules.find(Subject.value())
-                ->getSecond()
-                .getEnumValueName();
+      auto RuleSet = Support.SubjectsToRules.find(Subject.value())->getSecond();
+      if (RuleSet.isRule()) {
+        OS << RuleSet.getRule().getEnumValueName();
+        continue;
+      }
+      OS << "(";
+      for (const auto &Rule : llvm::enumerate(RuleSet.getAggregateRuleSet())) {
+        if (Rule.index())
+          OS << ", ";
+        OS << Rule.value().getEnumValueName();
+      }
+      OS << ")";
     }
     OS << ")\n";
   }
 }
 
 } // end namespace clang