[PATCH] D97362: [clang][parser] Allow attributes in explicit template instantiations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Parse/Parser.h:1586-1589
+      for (const ParsedAttr &A : *this) {
+        if (A.getSyntax() != ParsedAttr::Syntax::AS_GNU)
+          return false;
+      }
----------------



================
Comment at: clang/include/clang/Parse/Parser.h:1591-1593
+      // If the attr list is empty, but the range is still set, we did parse
+      // some attributes. Return false in this case, even though we might've
+      // parsed [[]].
----------------
I would argue that this function should return false in this case because it has no attributes and since it has no attributes, it does not contain *only* GNU attributes. There shouldn't be a need to check `Range.isInvalid()` at all but could instead `return !empty();` and this functionality could be moved to `ParsedAttributes` instead of requiring there to be a range.

However, I don't know that this function needs to exist at all (see below).


================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1828-1830
+      // Allow only GNU attributes here.
+      if (!attrs.hasOnlyGNUAttributes())
+        ProhibitAttributes(attrs);
----------------
We really should be using `ProhibitCXX11Attributes()` here, but that doesn't currently handle the case where it's an empty attribute list. However, we could use the valid source range information in that case to look at the tokens used to decide whether to diagnose or not.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97362/new/

https://reviews.llvm.org/D97362