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.