[PATCH] D32176: Add #pragma clang attribute support for the external_source_symbol attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5769
+    // might include a subject list even when it has custom parsing.
+    if (!Attr.diagnoseAppertainsTo(S, D))
+      return true;
----------------
Instead of duplicating this check in the if statement, why not hoist the current check above the if statement?


================
Comment at: utils/TableGen/ClangAttrEmitter.cpp:1620
+                           bool IsRule)
+        : Rules(Rules.begin(), Rules.end()), IsRule(IsRule) {}
+
----------------
ArrayRef automatically converts to a std::vector, so you should just be able to use `Rules(Rules)`.


================
Comment at: utils/TableGen/ClangAttrEmitter.cpp:1625
+
+    AttributeSubjectMatchRule &getRule() {
+      assert(IsRule && "not a rule!");
----------------
Any reason not to return this rule by const ref and make the function const?


================
Comment at: utils/TableGen/ClangAttrEmitter.cpp:1701
+  std::vector<Record *> DeclNodes = Records.getAllDerivedDefinitions("DDecl");
+  for (const Record *Aggregate : Aggregates) {
+    Record *SubjectDecl = Aggregate->getValueAsDef("Subject");
----------------
Can use `const auto *` here


Repository:
  rL LLVM

https://reviews.llvm.org/D32176