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

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

In D91630#2399995 <https://reviews.llvm.org/D91630#2399995>, @aaron.ballman wrote:

> 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?

Sure, I added release notes and language extension docs. I also added a __has_extension for this. That might all be a little overkill, I'm happy to pull out those changes if so.

In D91630#2400731 <https://reviews.llvm.org/D91630#2400731>, @rsmith wrote:

> Do `[[deprecated]]` and `[[maybe_unused]]` now work for //using-declarator//s? If so, a test for that would be useful.

`[[deprecated]]` gets applied, but it doesn't have any effect, since we don't attach an attribute to the UsingShadowDecl, and regardless, we already look past the UsingShadowDecl before we reach DiagnoseUseOfDecl. I added a -Wignored-attributes diagnostic to call this out.



================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:714
 
-  // C++11 attributes are not allowed on a using-declaration, but GNU ones
-  // are.
   ProhibitAttributes(MisplacedAttrs);
+  DiagnoseCXX11AttributeExtension(PrefixAttrs);
----------------
rsmith wrote:
> Should we suggest moving the attributes after the identifier in this case (even though that will leave the program ill-formed), as we do for alias-declarations?
Sure, I added this in the new patch.


================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:737
+    DiagnoseCXX11AttributeExtension(Attrs);
+    Attrs.addAll(PrefixAttrs.begin(), PrefixAttrs.end());
 
----------------
rsmith wrote:
> Should the prefix attributes go before the suffix attributes? (I could imagine there might be attributes for which order matters.) What do we do in the simple-declaration case?
Surprisingly, this does apply attributes to the beginning of the list:

```
void addAll(iterator B, iterator E) {
  AttrList.insert(AttrList.begin(), B.I, E.I);
}
```

This is consistent with simple-declarations.


================
Comment at: clang/test/Parser/cxx0x-attributes.cpp:134
 
-[[]] using ns::i; // expected-error {{an attribute list cannot appear here}}
+[[]] using ns::i;
 [[unknown]] using namespace ns; // expected-warning {{unknown attribute 'unknown' ignored}}
----------------
aaron.ballman wrote:
> Btw, a `-pedantic` test for the parser behavior would be useful so that we can be sure the new diagnostic is firing as expected.
Sure, I added that in a new test file.


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

https://reviews.llvm.org/D91630