aaron.ballman added inline comments.
================
Comment at: clang/include/clang/AST/DeclCXX.h:3801
+/// error.
+class UnresolvedUsingIfExistsDecl final : public NamedDecl {
+ UnresolvedUsingIfExistsDecl(DeclContext *DC, SourceLocation Loc,
----------------
erik.pilkington wrote:
> aaron.ballman wrote:
> > Why is this inheriting from a `NamedDecl` rather than a `UsingDecl`? Given
> > that this is a type of using declaration, I guess I would have expected it
> > to appear as such in the AST hierarchy. For instance, should people using
> > AST matchers be able to match one of these as a using declaration or are
> > they so different semantically that they need to be sibling AST nodes?
> This node isn't a kind of using declaration, it is a declaration that gets
> inserted into the scope via a usual UsingDecl & UsingShadowDecl mechanism
> that Sema knows to error out on if it is ever used. So presumably existing
> AST users would still recognize that this is a UsingDecl that adds a single
> declaration into the current context, but wouldn't really know anything about
> that declaration. I updated the doc comment above to make that more clear.
So given code like this:
```
using empty_namespace::does_not_exist __attribute__((using_if_exists));
```
would you expect this AST matcher to match or not?
```
usingDecl()
```
(I hope the answer is "yes" because otherwise the behavior is rather
inexplicable to me.)
================
Comment at: clang/include/clang/Basic/AttrDocs.td:5273
+ namespace empty_namespace {};
+ using empty_namespace::does_not_exist __attribute__((using_if_exists)); //
no error!
+
----------------
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.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:1177
+ if (auto *EmptyD = dyn_cast<UnresolvedUsingIfExistsDecl>(FirstDecl)) {
+ Diag(NameLoc, diag::err_use_of_empty_using_if_exists);
----------------
erik.pilkington wrote:
> aaron.ballman wrote:
> > `const auto *`
> This would lead to a bit of a `const`-goosechase in DiagnoseUseOfDecl. I
> thought we generally weren't too interested in `const` on AST nodes since
> they're assumed to be immutable anyways, so it doesn't really show much
> intent.
Ah, then don't bother with the goose chase. We don't require `const`
correctness because of these kinds of goose chases, but we usually strive for
`const` correctness when it's cheap to do so.
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
