riccibruno marked an inline comment as done.
riccibruno added inline comments.


================
Comment at: lib/Sema/SemaDecl.cpp:16607
+  // Check for other kinds of shadowing not already handled.
+  if (PrevDecl && isa<ValueDecl>(PrevDecl->getUnderlyingDecl()) &&
+      !TheEnumDecl->isScoped())
----------------
aaron.ballman wrote:
> riccibruno wrote:
> > aaron.ballman wrote:
> > > Is the change to `PrevDecl->getUnderlyingDecl()` intended here?
> > Yes it is. Previously it was done inside `LookupResult::getAsSingle`. 
> > However with this patch `PrevDecl` at this point can be a `UsingShadowDecl` 
> > for a given using-declaration. We need to look for the underlying 
> > declaration since this is what `CheckShadow` expects.
> But when it's not a `UsingShadowDecl`, will the behavior now be incorrect? 
> e.g., if it was a `NamespaceAliasDecl`, won't this check whether you are 
> shadowing the aliased namespace as opposed to the alias name itself? Might be 
> worth some tests.
Do you mean in an example like the following ?

```
namespace Q {}
namespace M { namespace i = Q; }
using namespace M;

enum { i };
```
This is example is mistakenly rejected (I think!) because of the fact that 
currently `getAsSingle` will look through the `NamespaceAliasDecl` for `i`, and 
find the `NamespaceDecl` for `Q`. There are currently no shadow warning for 
such an example.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60956



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to