* [PATCH] Fix PR c++/92024 @ 2019-10-10 15:19 Bernd Edlinger 2019-10-10 17:50 ` Jason Merrill 0 siblings, 1 reply; 11+ messages in thread From: Bernd Edlinger @ 2019-10-10 15:19 UTC (permalink / raw) To: gcc-patches, Jason Merrill, Nathan Sidwell [-- 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 { ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix PR c++/92024 2019-10-10 15:19 [PATCH] Fix PR c++/92024 Bernd Edlinger @ 2019-10-10 17:50 ` Jason Merrill 2019-10-10 18:23 ` Bernd Edlinger 0 siblings, 1 reply; 11+ messages in thread From: Jason Merrill @ 2019-10-10 17:50 UTC (permalink / raw) To: Bernd Edlinger, gcc-patches, Nathan Sidwell 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix PR c++/92024 2019-10-10 17:50 ` Jason Merrill @ 2019-10-10 18:23 ` Bernd Edlinger 2019-10-10 18:26 ` Marek Polacek 2019-10-11 16:34 ` Jason Merrill 0 siblings, 2 replies; 11+ messages in thread From: Bernd Edlinger @ 2019-10-10 18:23 UTC (permalink / raw) To: Jason Merrill, gcc-patches, Nathan Sidwell 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix PR c++/92024 2019-10-10 18:23 ` Bernd Edlinger @ 2019-10-10 18:26 ` Marek Polacek 2019-10-11 16:34 ` Jason Merrill 1 sibling, 0 replies; 11+ messages in thread From: Marek Polacek @ 2019-10-10 18:26 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Jason Merrill, gcc-patches, Nathan Sidwell 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix PR c++/92024 2019-10-10 18:23 ` Bernd Edlinger 2019-10-10 18:26 ` Marek Polacek @ 2019-10-11 16:34 ` Jason Merrill 2019-10-11 17:07 ` Bernd Edlinger 2019-10-12 18:49 ` Bernd Edlinger 1 sibling, 2 replies; 11+ messages in thread From: Jason Merrill @ 2019-10-11 16:34 UTC (permalink / raw) To: Bernd Edlinger, gcc-patches, Nathan Sidwell 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix PR c++/92024 2019-10-11 16:34 ` Jason Merrill @ 2019-10-11 17:07 ` Bernd Edlinger 2019-10-12 18:49 ` Bernd Edlinger 1 sibling, 0 replies; 11+ messages in thread From: Bernd Edlinger @ 2019-10-11 17:07 UTC (permalink / raw) To: Jason Merrill, gcc-patches, Nathan Sidwell 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix PR c++/92024 2019-10-11 16:34 ` Jason Merrill 2019-10-11 17:07 ` Bernd Edlinger @ 2019-10-12 18:49 ` Bernd Edlinger 2019-10-12 18:56 ` Sandra Loosemore ` (2 more replies) 1 sibling, 3 replies; 11+ messages in thread From: Bernd Edlinger @ 2019-10-12 18:49 UTC (permalink / raw) To: Jason Merrill, gcc-patches, Nathan Sidwell, Sandra Loosemore [-- 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; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix PR c++/92024 2019-10-12 18:49 ` Bernd Edlinger @ 2019-10-12 18:56 ` Sandra Loosemore 2019-10-18 13:21 ` Bernd Edlinger 2019-10-30 20:26 ` Jason Merrill 2 siblings, 0 replies; 11+ messages in thread From: Sandra Loosemore @ 2019-10-12 18:56 UTC (permalink / raw) To: Bernd Edlinger, Jason Merrill, gcc-patches, Nathan Sidwell 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix PR c++/92024 2019-10-12 18:49 ` Bernd Edlinger 2019-10-12 18:56 ` Sandra Loosemore @ 2019-10-18 13:21 ` Bernd Edlinger 2019-10-26 9:01 ` [PING**2] " Bernd Edlinger 2019-10-30 20:26 ` Jason Merrill 2 siblings, 1 reply; 11+ messages in thread From: Bernd Edlinger @ 2019-10-18 13:21 UTC (permalink / raw) To: Jason Merrill, gcc-patches, Nathan Sidwell, Sandra Loosemore 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. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PING**2] [PATCH] Fix PR c++/92024 2019-10-18 13:21 ` Bernd Edlinger @ 2019-10-26 9:01 ` Bernd Edlinger 0 siblings, 0 replies; 11+ messages in thread From: Bernd Edlinger @ 2019-10-26 9:01 UTC (permalink / raw) To: Jason Merrill, gcc-patches, Nathan Sidwell, Sandra Loosemore 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. >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix PR c++/92024 2019-10-12 18:49 ` Bernd Edlinger 2019-10-12 18:56 ` Sandra Loosemore 2019-10-18 13:21 ` Bernd Edlinger @ 2019-10-30 20:26 ` Jason Merrill 2 siblings, 0 replies; 11+ messages in thread From: Jason Merrill @ 2019-10-30 20:26 UTC (permalink / raw) To: Bernd Edlinger, gcc-patches, Nathan Sidwell, Sandra Loosemore 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-10-30 19:43 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-10 15:19 [PATCH] Fix PR c++/92024 Bernd Edlinger 2019-10-10 17:50 ` Jason Merrill 2019-10-10 18:23 ` Bernd Edlinger 2019-10-10 18:26 ` Marek Polacek 2019-10-11 16:34 ` Jason Merrill 2019-10-11 17:07 ` Bernd Edlinger 2019-10-12 18:49 ` Bernd Edlinger 2019-10-12 18:56 ` Sandra Loosemore 2019-10-18 13:21 ` Bernd Edlinger 2019-10-26 9:01 ` [PING**2] " Bernd Edlinger 2019-10-30 20:26 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).