rsmith added inline comments.
================ Comment at: clang/include/clang/Basic/AttrDocs.td:5273 + namespace empty_namespace {}; + using empty_namespace::does_not_exist __attribute__((using_if_exists)); // no error! + ---------------- aaron.ballman wrote: > Mordante wrote: > > erik.pilkington wrote: > > > aaron.ballman wrote: > > > > erik.pilkington wrote: > > > > > Mordante wrote: > > > > > > Mordante wrote: > > > > > > > Can you add an example using `[[clang::using_if_exists]]` or use > > > > > > > that instead of this attribute? > > > > > > Why is the attribute placed here? I would expect the attribute > > > > > > before the declaration `[[clang::using_if_exists]] using > > > > > > empty_namespace::does_not_exist;` > > > > > The attribute is written like that because clang rejects C++ style > > > > > attributes on using declarations, and only accepts attributes in that > > > > > position. I think this is the first attribute we actually support on > > > > > using-declarations, so maybe we should consider supporting it. > > > > > I think this is the first attribute we actually support on > > > > > using-declarations, so maybe we should consider supporting it. > > > > > > > > This isn't the first attribute we *should* be supporting on using > > > > declarations. Any attribute that appertains to a `NamedDecl` should > > > > apply as should the annotate attribute. > > > > > > > > However: > > > > ``` > > > > [[clang::whatever]] using foo::bar; // Correct to reject, #1 > > > > using foo::bar [[clang::whatever]]; // Correct to reject, #2 > > > > ``` > > > > #1 is rejected because it's a declaration-statement and those cannot > > > > have a leading attribute-specifier-seq > > > > (http://eel.is/c++draft/stmt.stmt#stmt.pre-1). #2 is rejected because > > > > the using-declaration cannot have a trailing attribute-specifier-seq > > > > (http://eel.is/c++draft/namespace.udecl#nt:using-declaration). > > > > > > > > This seems like a case where we may want to explore an extension to C++ > > > > that we propose to WG21. > > > > This isn't the first attribute we *should* be supporting on using > > > > declarations. Any attribute that appertains to a NamedDecl should apply > > > > as should the annotate attribute. > > > > > > Yeah, agreed. Its just that Sema was failing to call > > > ProcessDeclAttributeList for UsingDecls, so no attributes were actually > > > getting applied in practice. > > > > > > Okay, if our policy is to only parse C++-style attributes in places that > > > the C++ grammar allows them then I agree that this would be an issue for > > > the standards committee. > > I would be in favour to implement this as an extension and propose it to > > the committee. I wonder whether it's not allowed because the committee > > objected or nobody wrote a paper. (Attributes are already allowed on using > > namespace directives http://eel.is/c++draft/namespace.udir.) > I wouldn't be opposed to adding an extension for parsing C++-style attributes > on parts of a using declaration so long as we diagnosed them as an extension > to alert people to portability issues. I would say that the attribute should > be written as a prefix attribute (similar to the using-directive grammar) and > after the using-declarator. This would allow you to do: > ``` > [[]] using foo::bar; > using foo::bar [[]]; > using foo::bar [[]], baz::quux; > ``` FWIW, this matches my intuition for attribute placement. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90188/new/ https://reviews.llvm.org/D90188 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits