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

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.


================
Comment at: include/clang/Basic/AttrDocs.td:1007
+
+defined_in=\ *string-literal*
+  The name of the source container in which the declaration was defined. The
----------------
aaron.ballman wrote:
> Would this hold something like a file name? If so, I worry about conflicts between the comma separator and a file name -- you might want to pick a separator that can't be used in a file name (like | or :).
It could potentially include a filename, yes.
I don't quite follow your concerns though.. If a comma is in a string literal then it's wrapped in quotes. Wouldn't that be enough to distinguish the comma separator token from the comma in a filename?


================
Comment at: lib/Sema/SemaDeclAttr.cpp:2414
+                                           const AttributeList &Attr) {
+  if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
+    return;
----------------
aaron.ballman wrote:
> You should also diagnose if there's more than 3 arguments, no?
I think an assert would be more appropriate since I only use up to 3 arguments when creating the attribute, so I wouldn't be able to test the diagnostic.


Repository:
  rL LLVM

https://reviews.llvm.org/D29819