The following program results in a warning about the constructor parameter <i> shadowing Foo's data member <i> when compiled with clang r181693: struct Foo { int i; Foo(int i) : i(i) {} }; This results in: % clang++ -v -Wshadow -c clang.cpp clang version 3.4 (trunk 181693) [...] clang.cpp:4:13: warning: declaration shadows a field of 'Foo' [-Wshadow] Foo(int i) : i(i) {} ^ clang.cpp:2:9: note: previous declaration is here int i; ^ 1 warning generated. While the warning is correct, it is IMHO a common idiom to name the constructor's parameters identical to the class's data member they initialize. In that case the warning is unhelpful. Of course there are workarounds: 1. Rename the parameters, e.g. add a trailing underscore. 2. Use C++11's uniform initialization syntax where possible. That is why I'm not sure if clang's current behavior should be changed (one possible change would be to only warn if the parameter is used in the constructor's body instead of only in the member initialization list).
This has come up multiple times in LLDB coding style discussion and on chromium-dev. It'd also be nice if we could keep warning when there's ambiguity in the way that the ctor parameter is used, though, as in: struct Foo { int i; Foo(int i) : i(i) { i++; } // modifies the parameter, not member };
*** Bug 21110 has been marked as a duplicate of this bug. ***
Yes this generates hundreds of useless warnings! It should only warn if the name is used inside the constructor body.
+rtrieu in case he's interested in this one
Function of interest is Sema::CheckShadow. Something along the lines of: Index: SemaDecl.cpp =================================================================== --- SemaDecl.cpp (revision 260978) +++ SemaDecl.cpp (working copy) @@ -6344,10 +6344,18 @@ return; // Fields are not shadowed by variables in C++ static methods. - if (isa<FieldDecl>(ShadowedDecl)) + if (isa<FieldDecl>(ShadowedDecl)) { if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(NewDC)) if (MD->isStatic()) return; + if (auto CD = dyn_cast<CXXConstructorDecl>(NewDC)) { + if (dyn_cast<ParmVarDecl>(D)) { + return; + } + } + } if (VarDecl *shadowedVar = dyn_cast<VarDecl>(ShadowedDecl)) if (shadowedVar->isExternC()) {
Here's something that kind of works but needs more extensive testing: http://reviews.llvm.org/D18271
r267957. If you want the old behavior, use -Wshadow-all.
Much better now, thanks! Though it still warns about a similar pattern as some people don't use field name prefixes: void setFoo(Foo* f) { this->f = f; } int doBoo(int a, int f) { return a + f; }