[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 17 07:47:36 PST 2020


aaron.ballman added a comment.

Can you mention this change in the release notes? Also, should we document it explicitly in the Language Extensions documentation, or do you think this is too tiny of a feature to warrant that?



================
Comment at: clang/include/clang/Parse/Parser.h:2648
 
+  /// Emit warnings for C++11 attributes that are in a position that clang
+  /// accepts as an extension.
----------------
In general, things named with `CXX11Attributes` really mean `double square bracket attributes` more than `C++`, so they also impact `[[]]` behavior in C. I think the function name here is fine (it's just as bad as the other names) but the comment should probably also include C2x attributes.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:1606
+  for (const ParsedAttr &PA : Attrs) {
+    if (PA.isCXX11Attribute())
+      Diag(PA.getLoc(), diag::ext_cxx11_attr_placement) << PA;
----------------
And the implementation side of things should probably be checking `isC2xAttribute()` as well.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:1607
+    if (PA.isCXX11Attribute())
+      Diag(PA.getLoc(), diag::ext_cxx11_attr_placement) << PA;
+  }
----------------
Can you also pass in `<< PA.getRange()`? (This ensures that the whole attribute is underlined for the diagnostic instead of just the first token of the attribute, which may be just the vendor namespace.)


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

https://reviews.llvm.org/D91630



More information about the cfe-commits mailing list