https://godbolt.org/g/54XRMT void f(int a) { struct A { void g(int a) {} A() { int a; } }; } 3 : <source>:3:16: warning: declaration shadows a local variable [-Wshadow] void g(int a) {} ^ 1 : <source>:1:12: note: previous declaration is here void f(int a) { ^ 4 : <source>:4:15: warning: declaration shadows a local variable [-Wshadow] A() { int a; } ^ 1 : <source>:1:12: note: previous declaration is here void f(int a) { ^ 2 warnings generated. The local variable `a` of the function `f` can't be accessed from a method of the function-local class A, thus no shadowing occurs and no diagnostic is needed.
BTW, gcc's -Wshadow diagnostic correctly ignores local variables separated by a local class: https://godbolt.org/g/fp7cmx.
Sent https://reviews.llvm.org/D35941 for review.
No diagnostic is *ever* "needed" in re shadowing; but this code is certainly confusing and I'm glad Clang gives these two diagnostics. I don't think this is a "false" positive at all: the code is confusing because of shadowing, and I'm glad Clang warns about it so that the programmer can go fix their code. Of course if Clang were refusing to compile the code, that would be a bug --- it's definitely *legal* code; it's just not *good* code and so it deserves the warnings.
(In reply to Arthur O'Dwyer from comment #3) > No diagnostic is *ever* "needed" in re shadowing; but this code is certainly > confusing and I'm glad Clang gives these two diagnostics. I don't think this > is a "false" positive at all: the code is confusing because of shadowing, > and I'm glad Clang warns about it so that the programmer can go fix their > code. > > Of course if Clang were refusing to compile the code, that would be a bug > --- it's definitely *legal* code; it's just not *good* code and so it > deserves the warnings. (cross-posting from https://reviews.llvm.org/D35941) IIUC, most cases where -Wshadow warnings are issued is when a declaration from an enclosing scope would be accessible if there was no declaration that shadows it. In this case the the local variable of the function would not be accessible inside the local class anyway, so the inner variable with the same name is not confusing (at least, much less confusing than if it would prevent access to the variable of an outer scope). This is close to shadowing of uncaptured locals in lambdas (which -Wshadow doesn't warn about, only -Wshadow-all/-Wshadow-uncaptured-local), but it seems to have even less potential for being misleading. We had a few of requests internally to mute -Wshadow for this case. Another data point is that GCC doesn't warn in this case. But if I'm overseeing reasons to issue a warning in this case, we could add an analogue of -Wshadow-uncaptured-local for this case. WDYT?
The fix was committed a while ago: https://reviews.llvm.org/rL309569.
Merged to 5.0 in r310674.