[PATCH] Fix PR c++/92024

[PATCH] Fix PR c++/92024

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 207 bytes --]

Hi,

this fixes a crash when -Wshadow=compatible-local is
enabled in the testcase g++.dg/parse/crash68.C


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

[-- Attachment #2: patch-pr92024.diff --]
[-- Type: application/octet-stream, Size: 1231 bytes --]

2019-10-10  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/92024
	* pt.c (instantiate_class_template_1): Avoid crash when
	CLASSTYPE_TEMPLATE_INFO is NULL.

testsuite:
2019-10-10  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/92024
	* g++.dg/parse/crash68.C: Add -Wshadow=compatible-local to additional
	options.

Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 276634)
+++ gcc/cp/pt.c	(working copy)
@@ -10973,6 +10973,9 @@ instantiate_class_template_1 (tree type)
       || uses_template_parms (type))
     return type;
 
+  if (CLASSTYPE_TEMPLATE_INFO (type) == NULL)
+    return error_mark_node;
+
   /* Figure out which template is being instantiated.  */
   templ = most_general_template (CLASSTYPE_TI_TEMPLATE (type));
   gcc_assert (TREE_CODE (templ) == TEMPLATE_DECL);
Index: gcc/testsuite/g++.dg/parse/crash68.C
===================================================================
--- gcc/testsuite/g++.dg/parse/crash68.C	(revision 276634)
+++ gcc/testsuite/g++.dg/parse/crash68.C	(working copy)
@@ -1,4 +1,6 @@
 // PR c++/84611
+// PR c++/92024
+// { dg-additional-options "-Wshadow=compatible-local" }
 
 template<typename = int>
 struct a {

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

[Jason Merrill]

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?

Jason

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

[Bernd Edlinger]

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.

The problem there is that CLASSTYPE_TI_TEMPLATE (type)
uses CLASSTYPE_TEMPLATE_INFO (type) but that is NULL.

Since other errors may return error_mark_node as well
and the template is totally erroneous anyway, I figured
we might get away with that simple fix. 


Bernd.

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

[Marek Polacek]

On Thu, Oct 10, 2019 at 06:06:40PM +0000, 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.
> 
> The problem there is that CLASSTYPE_TI_TEMPLATE (type)
> uses CLASSTYPE_TEMPLATE_INFO (type) but that is NULL.

... because the warning is called from pushtag which ran before building the
template info for 'c':
 9926       // Build template info for the new specialization.
 9927       SET_TYPE_TEMPLATE_INFO (t, build_template_info (found, arglist));

Looks like another indication that the shadow checking is done too early.

> Since other errors may return error_mark_node as well
> and the template is totally erroneous anyway, I figured
> we might get away with that simple fix. 

But it doesn't have to be broken; this valid test crashes with that option too

template<typename>
struct S {
  S () {
    struct c;
      {
	struct c {};
      }
  }
};

S<int> s;

--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

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

[Jason Merrill]

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.

Jason

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

[Bernd Edlinger]

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.

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

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 2994 bytes --]

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.
> 

So I try to make C++ behave more consistently with the code in c-decl.c,
thus dependent on warn_shadow but not on warn_shadow_local and/or
warn_shadow_compatible_local:

           if (warn_shadow)
              warning_code = OPT_Wshadow;
            else if (comptypes (TREE_TYPE (old_decl), TREE_TYPE (new_decl)))
              warning_code = OPT_Wshadow_compatible_local;
            else
              warning_code = OPT_Wshadow_local;
            warned = warning_at (DECL_SOURCE_LOCATION (new_decl), warning_code,
                                 "declaration of %qD shadows a parameter",
                                 new_decl);

I cannot remove the if (warn_shadow) since this breaks gcc.dg/pr48062.c
which uses:

#pragma GCC diagnostic ignored "-Wshadow"

to disable a -Wshadow=compatible-local warning, but while -Wno-shadow on the
command line disables also dependent warnings the pragma does not (always) do that.

So instead I'd like to adjust the doc of -Wshadow to reflect the implementation
and remove the if(warn_shadow_local) to have C and C++ behave identical and
hopefully now in sync with the doc.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-pr92024.diff --]
[-- Type: text/x-patch; name="patch-pr92024.diff", Size: 8547 bytes --]

2019-10-12  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* doc/invoke.texi (-Wshadow, -Wshadow=global
	-Wshadow=local, -Wshadow=compatible-local): Update documentation.

cp:
2019-10-12  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/92024
	* name-lookup.c (check_local_shadow): Shadowing TYPE_DECLs
	is always a -Wshadow=compatible-local warning, unless
	-Wshadow is used.

testsuite:
2019-10-12  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/92024
	* g++.dg/parse/crash70.C: New test.
	* c-c++-common/Wshadow-1.c: New test.

Index: gcc/cp/name-lookup.c
===================================================================
--- gcc/cp/name-lookup.c	(revision 276893)
+++ gcc/cp/name-lookup.c	(working copy)
@@ -2750,29 +2750,31 @@ check_local_shadow (tree decl)
 	 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))))
+      else 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)
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 276893)
+++ gcc/doc/invoke.texi	(working copy)
@@ -342,7 +342,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-pragmas  -Wno-prio-ctor-dtor  -Wredundant-decls @gol
 -Wrestrict  -Wno-return-local-addr @gol
 -Wreturn-type  -Wsequence-point  -Wshadow  -Wno-shadow-ivar @gol
--Wshadow=global,  -Wshadow=local,  -Wshadow=compatible-local @gol
+-Wshadow=global  -Wshadow=local  -Wshadow=compatible-local @gol
 -Wshift-overflow  -Wshift-overflow=@var{n} @gol
 -Wshift-count-negative  -Wshift-count-overflow  -Wshift-negative-value @gol
 -Wsign-compare  -Wsign-conversion  -Wfloat-conversion @gol
@@ -6308,7 +6308,7 @@ can be used to suppress such a warning.
 @opindex Wno-discarded-array-qualifiers
 @opindex Wdiscarded-array-qualifiers
 Do not warn if type qualifiers on arrays which are pointer targets
-are being discarded. Typically, the compiler warns if a
+are being discarded.  Typically, the compiler warns if a
 @code{const int (*)[]} variable is passed to a function that
 takes a @code{int (*)[]} parameter.  This option can be used to
 suppress such a warning.
@@ -6507,9 +6507,13 @@ allowed in GCC@.  It is not supported by ISO C90.
 @opindex Wno-shadow
 Warn whenever a local variable or type declaration shadows another
 variable, parameter, type, class member (in C++), or instance variable
-(in Objective-C) or whenever a built-in function is shadowed. Note
+(in Objective-C) or whenever a built-in function is shadowed.  Note
 that in C++, the compiler warns if a local variable shadows an
 explicit typedef, but not if it shadows a struct/class/enum.
+If this warning is enabled, it includes also all instances of
+local shadowing.  This means that @option{-Wno-shadow=local}
+and @option{-Wno-shadow=compatible-local} are ignored when
+@option{-Wshadow} is used.
 Same as @option{-Wshadow=global}.
 
 @item -Wno-shadow-ivar @r{(Objective-C only)}
@@ -6520,20 +6524,19 @@ Objective-C method.
 
 @item -Wshadow=global
 @opindex Wshadow=global
-The default for @option{-Wshadow}. Warns for any (global) shadowing.
-This warning is enabled by @option{-Wshadow=global}.
+Warn for any shadowing.
+Same as @option{-Wshadow}.
 
 @item -Wshadow=local
 @opindex Wshadow=local
 Warn when a local variable shadows another local variable or parameter.
-This warning is enabled by @option{-Wshadow=local}.
 
 @item -Wshadow=compatible-local
 @opindex Wshadow=compatible-local
 Warn when a local variable shadows another local variable or parameter
-whose type is compatible with that of the shadowing variable. In C++,
+whose type is compatible with that of the shadowing variable.  In C++,
 type compatibility here means the type of the shadowing variable can be
-converted to that of the shadowed variable. The creation of this flag
+converted to that of the shadowed variable.  The creation of this flag
 (in addition to @option{-Wshadow=local}) is based on the idea that when
 a local variable shadows another one of incompatible type, it is most
 likely intentional, not a bug or typo, as shown in the following example:
@@ -6552,16 +6555,15 @@ for (SomeIterator i = SomeObj.begin(); i != SomeOb
 @end smallexample
 
 Since the two variable @code{i} in the example above have incompatible types,
-enabling only @option{-Wshadow=compatible-local} will not emit a warning.
+enabling only @option{-Wshadow=compatible-local} does not emit a warning.
 Because their types are incompatible, if a programmer accidentally uses one
-in place of the other, type checking will catch that and emit an error or
-warning. So not warning (about shadowing) in this case will not lead to
-undetected bugs. Use of this flag instead of @option{-Wshadow=local} can
+in place of the other, type checking is expected to catch that and emit an
+error or warning.  Use of this flag instead of @option{-Wshadow=local} can
 possibly reduce the number of warnings triggered by intentional shadowing.
-Note that this does also mean that shadowing @code{const char *i} by
-@code{char *i} will not emit a warning.
+Note that this also means that shadowing @code{const char *i} by
+@code{char *i} does not emit a warning.
 
-This warning is enabled by @option{-Wshadow=compatible-local}.
+This warning is also enabled by @option{-Wshadow=local}.
 
 @item -Wlarger-than=@var{byte-size}
 @opindex Wlarger-than=
Index: gcc/testsuite/c-c++-common/Wshadow-1.c
===================================================================
--- gcc/testsuite/c-c++-common/Wshadow-1.c	(revision 0)
+++ gcc/testsuite/c-c++-common/Wshadow-1.c	(working copy)
@@ -0,0 +1,24 @@
+/* { dg-do-compile } */
+/* { dg-additional-options "-Wshadow=local -Wno-shadow=compatible-local" } */
+int c;
+void foo(int *c, int *d)   /* { dg-bogus   "Wshadow" } */
+{
+  int *e = d;
+  {
+    int d = 0;             /* { dg-warning "Wshadow=local" } */
+  }
+  {
+    int *e = 0;            /* { dg-bogus   "Wshadow=compatible-local" } */
+  }
+}
+#pragma GCC diagnostic warning "-Wshadow"
+void bar(int *c, int *d)   /* { dg-warning "Wshadow" } */
+{
+  int *e = d;
+  {
+    int d = 0;             /* { dg-warning "Wshadow" } */
+  }
+  {
+    int *e = 0;            /* { dg-warning "Wshadow" } */
+  }
+}
Index: gcc/testsuite/g++.dg/parse/crash70.C
===================================================================
--- gcc/testsuite/g++.dg/parse/crash70.C	(revision 0)
+++ gcc/testsuite/g++.dg/parse/crash70.C	(working copy)
@@ -0,0 +1,14 @@
+// PR c++/92024
+// { dg-additional-options "-Wshadow=compatible-local" }
+
+template<typename>
+struct S {
+  S () {
+    struct c;
+      {
+	struct c {}; // { dg-warning "Wshadow=compatible-local" }
+      }
+  }
+};
+
+S<int> s;

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

[Sandra Loosemore]

On 10/12/19 12:10 PM, Bernd Edlinger wrote:

> [snip snip] 
> So instead I'd like to adjust the doc of -Wshadow to reflect the implementation
> and remove the if(warn_shadow_local) to have C and C++ behave identical and
> hopefully now in sync with the doc.
> 
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?

The documentation changes look good to me.

-Sandra

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

[Bernd Edlinger]

Ping...

for the c++ FE and testsuite changes in the updated patch
here: https://gcc.gnu.org/ml/gcc-patches/2019-10/msg00916.html


Thanks
Bernd.




On 10/12/19 8:10 PM, Bernd Edlinger wrote:
> 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:
>>>
>>> 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.
>>
> 
> So I try to make C++ behave more consistently with the code in c-decl.c,
> thus dependent on warn_shadow but not on warn_shadow_local and/or
> warn_shadow_compatible_local:
> 
>            if (warn_shadow)
>               warning_code = OPT_Wshadow;
>             else if (comptypes (TREE_TYPE (old_decl), TREE_TYPE (new_decl)))
>               warning_code = OPT_Wshadow_compatible_local;
>             else
>               warning_code = OPT_Wshadow_local;
>             warned = warning_at (DECL_SOURCE_LOCATION (new_decl), warning_code,
>                                  "declaration of %qD shadows a parameter",
>                                  new_decl);
> 
> I cannot remove the if (warn_shadow) since this breaks gcc.dg/pr48062.c
> which uses:
> 
> #pragma GCC diagnostic ignored "-Wshadow"
> 
> to disable a -Wshadow=compatible-local warning, but while -Wno-shadow on the
> command line disables also dependent warnings the pragma does not (always) do that.
> 
> So instead I'd like to adjust the doc of -Wshadow to reflect the implementation
> and remove the if(warn_shadow_local) to have C and C++ behave identical and
> hopefully now in sync with the doc.
> 
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 

[PING**2] [PATCH] Fix PR c++/92024

[Bernd Edlinger]

Ping...

On 10/18/19 3:19 PM, Bernd Edlinger wrote:
> Ping...
> 
> for the c++ FE and testsuite changes in the updated patch
> here: https://gcc.gnu.org/ml/gcc-patches/2019-10/msg00916.html
> 
> 
> Thanks
> Bernd.
> 
> 
> 
> 
> On 10/12/19 8:10 PM, Bernd Edlinger wrote:
>> 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:
>>>>
>>>> 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.
>>>
>>
>> So I try to make C++ behave more consistently with the code in c-decl.c,
>> thus dependent on warn_shadow but not on warn_shadow_local and/or
>> warn_shadow_compatible_local:
>>
>>            if (warn_shadow)
>>               warning_code = OPT_Wshadow;
>>             else if (comptypes (TREE_TYPE (old_decl), TREE_TYPE (new_decl)))
>>               warning_code = OPT_Wshadow_compatible_local;
>>             else
>>               warning_code = OPT_Wshadow_local;
>>             warned = warning_at (DECL_SOURCE_LOCATION (new_decl), warning_code,
>>                                  "declaration of %qD shadows a parameter",
>>                                  new_decl);
>>
>> I cannot remove the if (warn_shadow) since this breaks gcc.dg/pr48062.c
>> which uses:
>>
>> #pragma GCC diagnostic ignored "-Wshadow"
>>
>> to disable a -Wshadow=compatible-local warning, but while -Wno-shadow on the
>> command line disables also dependent warnings the pragma does not (always) do that.
>>
>> So instead I'd like to adjust the doc of -Wshadow to reflect the implementation
>> and remove the if(warn_shadow_local) to have C and C++ behave identical and
>> hopefully now in sync with the doc.
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>

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

[Jason Merrill]

On 10/12/19 2:10 PM, Bernd Edlinger wrote:
> 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.
>>
> 
> So I try to make C++ behave more consistently with the code in c-decl.c,
> thus dependent on warn_shadow but not on warn_shadow_local and/or
> warn_shadow_compatible_local:
> 
>             if (warn_shadow)
>                warning_code = OPT_Wshadow;
>              else if (comptypes (TREE_TYPE (old_decl), TREE_TYPE (new_decl)))
>                warning_code = OPT_Wshadow_compatible_local;
>              else
>                warning_code = OPT_Wshadow_local;
>              warned = warning_at (DECL_SOURCE_LOCATION (new_decl), warning_code,
>                                   "declaration of %qD shadows a parameter",
>                                   new_decl);
> 
> I cannot remove the if (warn_shadow) since this breaks gcc.dg/pr48062.c
> which uses:
> 
> #pragma GCC diagnostic ignored "-Wshadow"
> 
> to disable a -Wshadow=compatible-local warning, but while -Wno-shadow on the
> command line disables also dependent warnings the pragma does not (always) do that.
> 
> So instead I'd like to adjust the doc of -Wshadow to reflect the implementation
> and remove the if(warn_shadow_local) to have C and C++ behave identical and
> hopefully now in sync with the doc.
> 
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?

OK, thanks.

Jason