Re: [PATCH] Fix PR c++/92024

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

On 10/11/19 6:31 PM, Jason Merrill wrote:
> On 10/10/19 2:06 PM, Bernd Edlinger wrote:
>> On 10/10/19 7:49 PM, Jason Merrill wrote:
>>> On 10/10/19 10:42 AM, Bernd Edlinger wrote:
>>>> Hi,
>>>>
>>>> this fixes a crash when -Wshadow=compatible-local is
>>>> enabled in the testcase g++.dg/parse/crash68.C
>>>
>>> Why does that flag cause this crash?
>>>
>>
>> gcc/cp/name-lookup.c:
>>
>>        if (warn_shadow)
>>          warning_code = OPT_Wshadow;
>>        else if (warn_shadow_local)
>>          warning_code = OPT_Wshadow_local;
>>        else if (warn_shadow_compatible_local
>>                 && (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
>>                     || (!dependent_type_p (TREE_TYPE (decl))
>>                         && !dependent_type_p (TREE_TYPE (old))
>>                         /* If the new decl uses auto, we don't yet know
>>                            its type (the old type cannot be using auto
>>                            at this point, without also being
>>                            dependent).  This is an indication we're
>>                            (now) doing the shadow checking too
>>                            early.  */
>>                         && !type_uses_auto (TREE_TYPE (decl))
>>                         && can_convert (TREE_TYPE (old), TREE_TYPE (decl),
>>                                         tf_none))))
>>          warning_code = OPT_Wshadow_compatible_local;
>>
>> if -Wshadow=compatible-local is used, the can_convert function crashes
>> in instantiate_class_template_1.
> 
> Right, checking can_convert is problematic here, as it can cause template instantiations that change the semantics of the program.  Or, in this case, crash.
> 

Ah, alright.

I think shadowing one type with another type of the same name is always a no no.
How about always warning (and not asking for can_convert) when either decl is a TYPE_DECL?

like this:

Index: gcc/cp/name-lookup.c
===================================================================
--- gcc/cp/name-lookup.c	(Revision 276886)
+++ gcc/cp/name-lookup.c	(Arbeitskopie)
@@ -2741,8 +2741,7 @@
 	  return;
 	}
 
-      /* If '-Wshadow=compatible-local' is specified without other
-	 -Wshadow= flags, we will warn only when the type of the
+      /* -Wshadow=compatible-local warns only when the type of the
 	 shadowing variable (DECL) can be converted to that of the
 	 shadowed parameter (OLD_LOCAL). The reason why we only check
 	 if DECL's type can be converted to OLD_LOCAL's type (but not the
@@ -2750,29 +2749,29 @@
 	 parameter, more than often they would use the variable
 	 thinking (mistakenly) it's still the parameter. It would be
 	 rare that users would use the variable in the place that
-	 expects the parameter but thinking it's a new decl.  */
+	 expects the parameter but thinking it's a new decl.
+	 If either object is a TYPE_DECL -Wshadow=compatible-local
+	 warns regardless of whether one of the types involved
+	 is a subclass of the other, since that is never okay.  */
 
       enum opt_code warning_code;
-      if (warn_shadow)
-	warning_code = OPT_Wshadow;
-      else if (warn_shadow_local)
-	warning_code = OPT_Wshadow_local;
-      else if (warn_shadow_compatible_local
-	       && (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
-		   || (!dependent_type_p (TREE_TYPE (decl))
-		       && !dependent_type_p (TREE_TYPE (old))
-		       /* If the new decl uses auto, we don't yet know
-			  its type (the old type cannot be using auto
-			  at this point, without also being
-			  dependent).  This is an indication we're
-			  (now) doing the shadow checking too
-			  early.  */
-		       && !type_uses_auto (TREE_TYPE (decl))
-		       && can_convert (TREE_TYPE (old), TREE_TYPE (decl),
-				       tf_none))))
+      if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
+	  || TREE_CODE (decl) == TYPE_DECL
+	  || TREE_CODE (old) == TYPE_DECL
+	  || (!dependent_type_p (TREE_TYPE (decl))
+	      && !dependent_type_p (TREE_TYPE (old))
+	      /* If the new decl uses auto, we don't yet know
+		 its type (the old type cannot be using auto
+		 at this point, without also being
+		 dependent).  This is an indication we're
+		 (now) doing the shadow checking too
+		 early.  */
+	      && !type_uses_auto (TREE_TYPE (decl))
+	      && can_convert (TREE_TYPE (old), TREE_TYPE (decl),
+			      tf_none)))
 	warning_code = OPT_Wshadow_compatible_local;
       else
-	return;
+	warning_code = OPT_Wshadow_local;
 
       const char *msg;
       if (TREE_CODE (old) == PARM_DECL)


Bernd.