class C { int both_var; void var_and_method() {} void m() { int both_var, var_and_method; } }; FAIL: gcc (GCC) 4.8.2 20130625 (prerelease) FAIL: gcc (GCC) 4.9.0 20130625 (experimental) g++ -c -o shadow.o shadow.C -Wshadow -Wno-unused shadow.C:4:18: warning: declaration of ‘both_var’ shadows a member of 'this' [-Wshadow] shadow.C:4:28: warning: declaration of ‘var_and_method’ shadows a member of 'this' [-Wshadow] clang-3.3-0.4.rc2.fc18.x86_64 clang -c -o shadow.o shadow.C -Wshadow -Wno-unused shadow.C:4:18: warning: declaration shadows a field of 'C' [-Wshadow] void m() { int both_var, var_and_method; } ^ shadow.C:2:7: note: previous declaration is here int both_var; ^ 1 warning generated. ------------------------------------------------------------------------------ clang does no warn on "var_and_method" as variable vs. method are safe, if one tries to use them inappropriately one gets an error. It is difficult to enable -Wshadow for existing project as it has many needless warnings, with clang it is easier and still safe.
> clang does no warn on "var_and_method" as variable vs. method are safe, if one > tries to use them inappropriately one gets an error. Not always. Think of function pointers or pointer to member functions. clang is not loose in my mind rather than GCC is too strict.
Yeah, I agree, this looks like a clang bug (or misdesigned feature) to me.
It may not be exactly correct but from a practical standpoint clang has caught my bug while not annoying me with tons of needless changes like gcc did, FYI.
(In reply to Andrew Pinski from comment #1) > > clang does no warn on "var_and_method" as variable vs. method are safe, if one > > tries to use them inappropriately one gets an error. > > Not always. Think of function pointers or pointer to member functions. > clang is not loose in my mind rather than GCC is too strict. In GCC 4.8 I implemented: "The option -Wshadow no longer warns if a declaration shadows a function declaration, unless the former declares a function or pointer to function, because this is a common and valid case in real-world code." I think this is a useful heuristic also for member functions, no? I don't have time to work on this at the moment, but it would be useful to know whether the maintainers agree, so someone (Jan?) may start working on a patch.
(In reply to Manuel López-Ibáñez from comment #4) > "The option -Wshadow no longer warns if a declaration shadows a function > declaration, unless the former declares a function or pointer to function, > because this is a common and valid case in real-world code." > > I think this is a useful heuristic also for member functions, no? I don't > have time to work on this at the moment, but it would be useful to know > whether the maintainers agree, so someone (Jan?) may start working on a > patch. That would be fine. But it seems less important for member functions, since there's much less chance of a local variable name conflicting with some random function declared by an #include file.
(In reply to Jason Merrill from comment #5) > > That would be fine. But it seems less important for member functions, since > there's much less chance of a local variable name conflicting with some > random function declared by an #include file. Well, Jan here is complaining precisely that a local variable (not function pointer, not other function) conflicts with a member function. I would expect that to almost never happen but I guess some programmers are not very good at naming things ;-)
The following patch compiles and gives the following output: /home/manuel/t.cc:6:18: warning: declaration of ‘both_var’ shadows a member of ‘C’ [-Wshadow] void m() { int both_var, var_and_method; } ^ /home/manuel/t.cc:3:7: note: shadowed declaration is here int both_var; ^ No other testing besides that. Please feel free to take it from here. Index: name-lookup.c =================================================================== --- name-lookup.c (revision 205004) +++ name-lookup.c (working copy) @@ -1216,13 +1216,25 @@ pushdecl_maybe_friend_1 (tree x, bool is else member = NULL_TREE; if (member && !TREE_STATIC (member)) { - /* Location of previous decl is not useful in this case. */ - warning (OPT_Wshadow, "declaration of %qD shadows a member of 'this'", - x); + if (BASELINK_P (member)) + member = BASELINK_FUNCTIONS (member); + + /* Do not warn if a variable shadows a function, unless + the variable is a function or a pointer-to-function. */ + if (!(TREE_CODE (member) == METHOD_TYPE + && TREE_CODE (x) != FUNCTION_DECL + && !FUNCTION_POINTER_TYPE_P (TREE_TYPE (x)))) { + if (warning_at (input_location, + OPT_Wshadow, "declaration of %qD shadows a member of %qT", + x, current_nonlambda_class_type ()) + && DECL_P(member)) + inform (DECL_SOURCE_LOCATION (member), + "shadowed declaration is here"); + } } else if (oldglobal != NULL_TREE && (VAR_P (oldglobal) /* If the old decl is a type decl, only warn if the old decl is an explicit typedef or if both the
Created attachment 31248 [details] Comment 7 patch as a file I still get both warnings, applied the patch to: g++ (GCC) 4.9.0 20131119 (experimental) shadow2.C: In member function ‘void C::m()’: shadow2.C:4:18: warning: declaration of ‘both_var’ shadows a member of ‘C’ [-Wshadow] void m() { int both_var, var_and_method; } ^ shadow2.C:2:7: note: shadowed declaration is here int both_var; ^ shadow2.C:4:28: warning: declaration of ‘var_and_method’ shadows a member of ‘C’ [-Wshadow] void m() { int both_var, var_and_method; } ^ shadow2.C:3:8: note: shadowed declaration is here void var_and_method() {} ^ shadow2.C:4:18: warning: unused variable ‘both_var’ [-Wunused-variable] void m() { int both_var, var_and_method; } ^ shadow2.C:4:28: warning: unused variable ‘var_and_method’ [-Wunused-variable] void m() { int both_var, var_and_method; } ^
Something has changed in the C++ FE in the meanwhile. Could you try with this one? Index: name-lookup.c =================================================================== --- name-lookup.c (revision 205004) +++ name-lookup.c (working copy) @@ -1216,13 +1216,25 @@ pushdecl_maybe_friend_1 (tree x, bool is else member = NULL_TREE; if (member && !TREE_STATIC (member)) { - /* Location of previous decl is not useful in this case. */ - warning (OPT_Wshadow, "declaration of %qD shadows a member of 'this'", - x); + if (BASELINK_P (member)) + member = BASELINK_FUNCTIONS (member); + + /* Do not warn if a variable shadows a function, unless + the variable is a function or a pointer-to-function. */ + if (!(TREE_CODE (member) == FUNCTION_DECL + && TREE_CODE (x) != FUNCTION_DECL + && !FUNCTION_POINTER_TYPE_P (TREE_TYPE (x)))) { + if (warning_at (input_location, + OPT_Wshadow, "declaration of %qD shadows a member of %qT", + x, current_nonlambda_class_type ()) + && DECL_P(member)) + inform (DECL_SOURCE_LOCATION (member), + "shadowed declaration is here"); + } } else if (oldglobal != NULL_TREE && (VAR_P (oldglobal) /* If the old decl is a type decl, only warn if the old decl is an explicit typedef or if both the
Created attachment 31252 [details] Comment 9 patch as a file Yes, it works for me now, thanks: shadow2.C: In member function ‘void C::m()’: shadow2.C:4:18: warning: declaration of ‘both_var’ shadows a member of ‘C’ [-Wshadow] void m() { int both_var, var_and_method; } ^ shadow2.C:2:7: note: shadowed declaration is here int both_var; ^ shadow2.C:4:18: warning: unused variable ‘both_var’ [-Wunused-variable] void m() { int both_var, var_and_method; } ^ shadow2.C:4:28: warning: unused variable ‘var_and_method’ [-Wunused-variable] void m() { int both_var, var_and_method; } ^ BTW copy-pasting patches here is very inconvenient.
Similar inappropriate warning is generated for typedef-vs-variable as reported now by Adam Jackson. Again a mistaken use cannot harm as it causes other errors. And clang also does not warn on it. int main(void) { typedef int x; { int x = 0; return x; } }
On Sat, 14 Dec 2013, jan.kratochvil at redhat dot com wrote: > Similar inappropriate warning is generated for typedef-vs-variable as reported > now by Adam Jackson. Again a mistaken use cannot harm as it causes other > errors. And clang also does not warn on it. > > int main(void) { > typedef int x; > { > int x = 0; > return x; If that's "return (x)-1;" then the interpretation as cast or arithmetic depends on whether x is a type or a variable.
Author: manu Date: Fri Aug 22 19:12:46 2014 New Revision: 214357 URL: https://gcc.gnu.org/viewcvs?rev=214357&root=gcc&view=rev Log: gcc/cp/ChangeLog: 2014-08-22 Manuel López-Ibáñez <manu@gcc.gnu.org> PR c++/57709 * name-lookup.c (pushdecl_maybe_friend_1): Do not warn if a declaration shadows a function declaration, unless the former declares a function, pointer to function or pointer to member function, because this is a common and valid case in real-world code. * cp-tree.h (TYPE_PTRFN_P,TYPE_REFFN_P,TYPE_PTRMEMFUNC_P): Improve description. gcc/testsuite/ChangeLog: 2014-08-22 Manuel López-Ibáñez <manu@gcc.gnu.org> PR c++/57709 * g++.dg/Wshadow.C: New test. Added: trunk/gcc/testsuite/g++.dg/Wshadow.C Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/cp-tree.h trunk/gcc/cp/name-lookup.c trunk/gcc/testsuite/ChangeLog
Fixed in gcc 5.0
-Wshadow still trigger false positive when a base member functions is imported with the "using" keyword, as in the following example (tested with gcc 5.1): template<typename T> struct BaseClass { BaseClass(int size) : m_size(size) {} int size() { return m_size; } int m_size; }; template<typename T> struct Foo : BaseClass<T> { typedef BaseClass<T> Base; Foo(int size) : Base(size) {} using Base::size; }; $ g++-mp-5 gcc_shadow.cpp -c -Wshadow gcc_shadow.cpp: In constructor 'Foo<T>::Foo(int)': gcc_shadow.cpp:9:17: warning: declaration of 'size' shadows a member of 'Foo<T>' [-Wshadow] Foo(int size) : Base(size) {} ^ gcc_shadow.cpp:10:15: note: shadowed declaration is here using Base::size; Note that clang does not warn in this case, so it should be possible to figure out that in this case, the imported "size" symbol is a function and not a variable.
(In reply to Gael Guennebaud from comment #15) > -Wshadow still trigger false positive when a base member functions is > imported with the "using" keyword, as in the following example (tested with > gcc 5.1): Please open a new PR. This one is fixed. > Note that clang does not warn in this case, so it should be possible to > figure out that in this case, the imported "size" symbol is a function and > not a variable. Sure, probably TREE_CODE (member) != FUNCTION_DECL but there must be a way to check that it does represent a function_decl (or member-function). Run gcc under gdb in your testcase and break at the call to warning_at and use "p debug_tree(member)" to understand what member is in your testcase.