[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: lib/Parse/ParseDecl.cpp:1161
+    } else {
+      assert(Keyword == Ident_defined_in);
+      if (HadDefinedIn) {
----------------
Add a string literal to the assert?


================
Comment at: lib/Parse/ParseDeclCXX.cpp:3818-3819
+  if (ScopeName && (ScopeName->getName() == "gnu" ||
+                    (ScopeName->getName() == "clang" &&
+                     AttrName->isStr("external_source_symbol"))))
     // GNU-scoped attributes have some special cases to handle GNU-specific
----------------
aaron.ballman wrote:
> I don't really like hard-coding a list of attributes like this. I think you should handle clang-namespaced attributes with a separate helper function.
This still isn't quite what I was looking for -- the helper function I was talking about was a replacement for `ParseGNUAttributeArgs()`. I'm sorry if I was unclear.

I think we should have a `ParseClangAttributeArgs()` that handles your custom parsing; it can then call through to `ParseAttributeArgsCommon()` for common argument handling in the same way `ParseGNUAttributeArgs()` does. So the predicate would be `else if (ScopeName && ScopeName->getName() == "clang")`.


Repository:
  rL LLVM

https://reviews.llvm.org/D29819