diff mbox

[c++/57709] Wshadow is too strict / has false positives

Message ID CAESRpQB_xK587jpeT35DSXnn8YwHib1RR_fDxG-e-Njo4O13mw@mail.gmail.com
State New
Headers show

Commit Message

Manuel López-Ibáñez Aug. 21, 2014, 10:01 p.m. UTC 
On 21 August 2014 23:35, Jason Merrill <jason@redhat.com> wrote:
> On 08/21/2014 04:22 PM, Manuel López-Ibáńez wrote:
>>
>> +                       && TREE_CODE (x) != FUNCTION_DECL
>> +                       && !FUNCTION_POINTER_TYPE_P (TREE_TYPE (x))))
>
>
> How about pointer to member function?

Maybe like this? BTW, I did not actually want to include the
gcc_assert(), it was just a sanity check, thus I deleted it from this
patch.

Bootstrapping and regression testing in progress. But the testcase
works as expected.

OK if it passes?


gcc/cp/ChangeLog:

2014-08-21  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.


gcc/testsuite/ChangeLog:

2014-08-21  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    PR c++/57709
    * g++.dg/Wshadow.C: New test.

Comments

Paolo Carlini Aug. 21, 2014, 10:22 p.m. UTC | #1 
Hi,

On 08/22/2014 12:01 AM, Manuel López-Ibáñez wrote:
> On 21 August 2014 23:35, Jason Merrill <jason@redhat.com> wrote:
>> On 08/21/2014 04:22 PM, Manuel López-Ibáńez wrote:
>>> +                       && TREE_CODE (x) != FUNCTION_DECL
>>> +                       && !FUNCTION_POINTER_TYPE_P (TREE_TYPE (x))))
>>
>> How about pointer to member function?
> Maybe like this? BTW, I did not actually want to include the
> gcc_assert(), it was just a sanity check, thus I deleted it from this
> patch.
Note by the way, that this patch would introduce the *first* use of 
FUNCTION_POINTER_TYPE_P in the C++ front-end. Are we sure we want to use 
that vs TYPE_PTRFN_P?!? At the moment I'm a bit tired by I seem to 
remember subtleties in this area...

Paolo.
diff mbox

Patch

Index: gcc/testsuite/g++.dg/Wshadow.C
===================================================================
--- gcc/testsuite/g++.dg/Wshadow.C	(revision 0)
+++ gcc/testsuite/g++.dg/Wshadow.C	(revision 0)
@@ -0,0 +1,15 @@ 
+// { dg-do compile }
+// { dg-options "-Wshadow" }
+// PR c++/57709 
+class C {
+  int both_var; // { dg-message "declaration" }
+  void var_and_method(void) {} // { dg-message "declaration" }
+  void m() { 
+    int 
+      both_var,  // { dg-warning "shadows" }
+      var_and_method; 
+  }
+  void m2() { 
+    void (C::*var_and_method)(void); // { dg-warning "shadows" }
+  }
+};
Index: gcc/cp/name-lookup.c
===================================================================
--- gcc/cp/name-lookup.c	(revision 214229)
+++ gcc/cp/name-lookup.c	(working copy)
@@ -1237,13 +1237,28 @@  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);
+		  member = OVL_CURRENT (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))
+			     || TYPE_PTRMEMFUNC_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