[PATCH] D90188: Add support for attribute 'using_if_exists'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D90188#2394361 <https://reviews.llvm.org/D90188#2394361>, @erik.pilkington wrote:

> Add support for C++11-style attributes on using-declarations.

FWIW, it'd be a bit easier if those changes were split off into their own patch, as they're orthogonal to `using_if_exists`.



================
Comment at: clang/include/clang/Basic/AttrDocs.td:5351
+the availability of those declarations is difficult or impossible to detect at
+compile time with the preprocessor.
+  }];
----------------
Do you think the docs should call out that using the C++ spelling on a `using` declaration is currently a Clang extension? It's a bit of an odd place to put that information, but I don't know of where else to mention that this attribute is nonportable in a few different ways.


================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:689
 def err_attributes_not_allowed : Error<"an attribute list cannot appear here">;
+def ext_cxx11_attr_placement : Extension<
+  "C++ does not allow an attribute list to appear here">,
----------------
I'm not certain what @rsmith thinks, but I think this should be `ExtWarn` and have the `DefaultIgnore` removed -- this is a conforming extension of something that's nonportable even in the absence of any attributes in the attribute list. e.g., `[[]] using foo::bar;``` is not portable to parse.


================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:690
+def ext_cxx11_attr_placement : Extension<
+  "C++ does not allow an attribute list to appear here">,
+  InGroup<DiagGroup<"cxx-attribute-extension">>, DefaultIgnore;
----------------
I think that should say `ISO C++` instead of just `C++` to make it clear that this is a standards thing, not a C++-as-it-really-is thing.


================
Comment at: clang/include/clang/Serialization/ASTBitCodes.h:1364
+      /// An UnresolvedUsingIfExistsDecl record.
+      DECL_UNRESOLVED_USING_IF_EXISTS,
+
----------------
Does this mean you need to bump `VERSION_MAJOR` as well?


================
Comment at: clang/test/Parser/cxx0x-attributes.cpp:160
 template<typename T> using U [[]] = T;
-using ns::i [[]]; // expected-error {{an attribute list cannot appear here}}
+using ns::i [[]];
 using [[]] ns::i; // expected-error {{an attribute list cannot appear here}}
----------------
Can you also add a test for `using foo [[]], bar []]];` syntax?


================
Comment at: clang/test/SemaCXX/using-if-exists-attr.cpp:9-17
+using NS::x [[clang::using_if_exists]]; // expected-warning{{C++ does not allow an attribute list to appear here}}
+
+[[clang::using_if_exists]] // expected-warning{{C++ does not allow an attribute list to appear here}}
+using NS::not_there, NS::not_there2;
+
+using NS::not_there3, // expected-error {{no member named 'not_there3' in namespace 'NS'}}
+      NS::not_there4 [[clang::using_if_exists]]; // expected-warning{{C++ does not allow an attribute list to appear here}}
----------------
These test cases look like they belong in a `Parser` test not a `SemaCXX` test.


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

https://reviews.llvm.org/D90188