On Thu, Aug 31, 2023 at 05:46:28PM -0400, Jason Merrill wrote: > I've suggested this to Core.
Thanks. > > So, I'm not really sure what to do. Intuitively the patch seems right > > because even block externs redeclare stuff and change meaning of the > > identifiers and void foo () { int i; extern int i (int); } is rejected > > by all compilers. > > I think this direction makes sense, though we might pedwarn on these rather > than error to reduce possible breakage. It wasn't clear to me whether you want to make those pedwarns just for the DECL_EXTERNAL cases, ones that actually changed, or all others as well (which were errors or permerrors depending on the case). I've implemented the former, kept existing behavior of !DECL_EXTERNAL. > > 2023-08-31 Jakub Jelinek <ja...@redhat.com> > > > > PR c++/52953 > > * name-lookup.cc (check_local_shadow): Defer punting on > > DECL_EXTERNAL (decl) from the start of function to right before > > the -Wshadow* checks. > > Don't we want to consider externs for the -Wshadow* checks as well? I think that is a good idea (though dunno how much it will trigger in real-world), but there is one case I've excluded, the global variable shadowing case, because warning that int z; void foo () { extern int z; z = 1; } shadows the global var would be incorrect, it is the same var. It is true that int y; namespace N { void bar () { extern int y; y = 1; } } shadows ::y but it is unclear how to differentiate those two cases with the information we have at check_local_shadow time. I've also found one spot which wasn't using auto_diagnostic_group d; on a pair of error_at/inform. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2023-09-01 Jakub Jelinek <ja...@redhat.com> PR c++/52953 * name-lookup.cc (check_local_shadow): Don't punt early for DECL_EXTERNAL decls, instead just disable the shadowing of namespace decls check for those and emit a pedwarn rather than error_at for those. Add missing auto_diagnostic_group. Formatting fix. * g++.dg/diagnostic/redeclaration-4.C: New test. * g++.dg/diagnostic/redeclaration-5.C: New test. * g++.dg/warn/Wshadow-19.C: New test. --- gcc/cp/name-lookup.cc.jj 2023-09-01 10:21:03.658118594 +0200 +++ gcc/cp/name-lookup.cc 2023-09-01 11:30:10.868516494 +0200 @@ -3096,10 +3096,6 @@ check_local_shadow (tree decl) if (TREE_CODE (decl) == PARM_DECL && !DECL_CONTEXT (decl)) return; - /* External decls are something else. */ - if (DECL_EXTERNAL (decl)) - return; - tree old = NULL_TREE; cp_binding_level *old_scope = NULL; if (cxx_binding *binding = outer_binding (DECL_NAME (decl), NULL, true)) @@ -3130,11 +3126,9 @@ check_local_shadow (tree decl) && DECL_CONTEXT (old) == lambda_function (current_lambda_expr ()) && TREE_CODE (old) == PARM_DECL && DECL_NAME (decl) != this_identifier) - { - error_at (DECL_SOURCE_LOCATION (old), - "lambda parameter %qD " - "previously declared as a capture", old); - } + error_at (DECL_SOURCE_LOCATION (old), + "lambda parameter %qD " + "previously declared as a capture", old); return; } /* Don't complain if it's from an enclosing function. */ @@ -3156,10 +3150,18 @@ check_local_shadow (tree decl) in the outermost block of the function definition. */ if (b->kind == sk_function_parms) { - error_at (DECL_SOURCE_LOCATION (decl), - "declaration of %q#D shadows a parameter", decl); - inform (DECL_SOURCE_LOCATION (old), - "%q#D previously declared here", old); + auto_diagnostic_group d; + bool emit = true; + if (DECL_EXTERNAL (decl)) + emit = pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wpedantic, + "declaration of %q#D shadows a parameter", + decl); + else + error_at (DECL_SOURCE_LOCATION (decl), + "declaration of %q#D shadows a parameter", decl); + if (emit) + inform (DECL_SOURCE_LOCATION (old), + "%q#D previously declared here", old); return; } } @@ -3185,10 +3187,16 @@ check_local_shadow (tree decl) && (old_scope->kind == sk_cond || old_scope->kind == sk_for)) { auto_diagnostic_group d; - error_at (DECL_SOURCE_LOCATION (decl), - "redeclaration of %q#D", decl); - inform (DECL_SOURCE_LOCATION (old), - "%q#D previously declared here", old); + bool emit = true; + if (DECL_EXTERNAL (decl)) + emit = pedwarn (DECL_SOURCE_LOCATION (decl), OPT_Wpedantic, + "redeclaration of %q#D", decl); + else + error_at (DECL_SOURCE_LOCATION (decl), + "redeclaration of %q#D", decl); + if (emit) + inform (DECL_SOURCE_LOCATION (old), + "%q#D previously declared here", old); return; } /* C++11: @@ -3314,6 +3322,7 @@ check_local_shadow (tree decl) || (TREE_CODE (old) == TYPE_DECL && (!DECL_ARTIFICIAL (old) || TREE_CODE (decl) == TYPE_DECL))) + && !DECL_EXTERNAL (decl) && !instantiating_current_function_p () && !warning_suppressed_p (decl, OPT_Wshadow)) /* XXX shadow warnings in outer-more namespaces */ --- gcc/testsuite/g++.dg/diagnostic/redeclaration-4.C.jj 2023-09-01 10:46:15.646025458 +0200 +++ gcc/testsuite/g++.dg/diagnostic/redeclaration-4.C 2023-09-01 10:46:15.646025458 +0200 @@ -0,0 +1,167 @@ +// PR c++/52953 +// { dg-do compile } +// { dg-options "-pedantic-errors -Wno-switch-unreachable" } + +void +foo (int x) // { dg-message "'int x' previously declared here" } +{ + extern int x; // { dg-error "declaration of 'int x' shadows a parameter" } +} + +void +bar (int x) // { dg-message "'int x' previously declared here" } +try +{ + extern int x; // { dg-error "declaration of 'int x' shadows a parameter" } +} +catch (...) +{ +} + +volatile int v; + +void +baz () +{ +#if __cplusplus >= 201103L + auto f = [] (int x) { extern int x; };// { dg-error "declaration of 'int x' shadows a parameter" "" { target c++11 } } + // { dg-message "'int x' previously declared here" "" { target c++11 } .-1 } +#endif + if (int x = 1) // { dg-message "'int x' previously declared here" } + { + extern int x; // { dg-error "redeclaration of 'int x'" } + } + if (int x = 0) // { dg-message "'int x' previously declared here" } + ; + else + { + extern int x; // { dg-error "redeclaration of 'int x'" } + } + if (int x = 1) // { dg-message "'int x' previously declared here" } + extern int x; // { dg-error "redeclaration of 'int x'" } + if (int x = 0) // { dg-message "'int x' previously declared here" } + ; + else + extern int x; // { dg-error "redeclaration of 'int x'" } + switch (int x = 1) // { dg-message "'int x' previously declared here" } + { + extern int x; // { dg-error "redeclaration of 'int x'" } + default:; + } + switch (int x = 1) // { dg-message "'int x' previously declared here" } + extern int x; // { dg-error "redeclaration of 'int x'" } + while (int x = v) + { + extern int x; // { dg-error "'int x' conflicts with a previous declaration" } + } + while (int x = v) + extern int x; // { dg-error "'int x' conflicts with a previous declaration" } + for (int x = v; x; ++x) // { dg-message "'int x' previously declared here" } + { + extern int x; // { dg-error "redeclaration of 'int x'" } + } + for (int x = v; x; ++x) // { dg-message "'int x' previously declared here" } + extern int x; // { dg-error "redeclaration of 'int x'" } + for (; int x = v; ) + { + extern int x; // { dg-error "'int x' conflicts with a previous declaration" } + } + for (; int x = v; ) + extern int x; // { dg-error "'int x' conflicts with a previous declaration" } + try + { + } + catch (int x) // { dg-message "'int x' previously declared here" } + { + extern int x; // { dg-error "redeclaration of 'int x'" } + } +} + +void +corge (int x) // { dg-message "'int x' previously declared here" } +try +{ +} +catch (...) +{ + extern int x; // { dg-error "redeclaration of 'int x'" } +} + +void +fred (int x) // { dg-message "'int x' previously declared here" } +try +{ +} +catch (int) +{ +} +catch (long) +{ + extern int x; // { dg-error "redeclaration of 'int x'" } +} + +void +garply (int x) +{ + try + { + extern int x; + } + catch (...) + { + extern int x; + } +} + +struct S +{ + S (int x) // { dg-message "'int x' previously declared here" } + try : s (x) + { + extern int x; // { dg-error "declaration of 'int x' shadows a parameter" } + } + catch (...) + { + } + int s; +}; + +struct T +{ + T (int x) // { dg-message "'int x' previously declared here" } + try : t (x) + { + } + catch (...) + { + extern int x; // { dg-error "redeclaration of 'int x'" } + } + int t; +}; + +struct U +{ + U (int x) : u (x) + { + try + { + extern int x; + } + catch (...) + { + extern int x; + } + } + int u; +}; + +struct V +{ + V (int x) : v (x) + { + { + extern int x; + } + } + int v; +}; --- gcc/testsuite/g++.dg/diagnostic/redeclaration-5.C.jj 2023-09-01 10:46:15.646025458 +0200 +++ gcc/testsuite/g++.dg/diagnostic/redeclaration-5.C 2023-09-01 10:46:15.646025458 +0200 @@ -0,0 +1,167 @@ +// PR c++/52953 +// { dg-do compile } +// { dg-options "-pedantic-errors -Wno-switch-unreachable" } + +void +foo (int x) // { dg-message "'int x' previously declared here" } +{ + extern int x (int); // { dg-error "declaration of 'int x\\\(int\\\)' shadows a parameter" } +} + +void +bar (int x) // { dg-message "'int x' previously declared here" } +try +{ + extern int x (int); // { dg-error "declaration of 'int x\\\(int\\\)' shadows a parameter" } +} +catch (...) +{ +} + +volatile int v; + +void +baz () +{ +#if __cplusplus >= 201103L + auto f = [] (int x) { extern int x (int); };// { dg-error "declaration of 'int x\\\(int\\\)' shadows a parameter" "" { target c++11 } } + // { dg-message "'int x' previously declared here" "" { target c++11 } .-1 } +#endif + if (int x = 1) // { dg-message "'int x' previously declared here" } + { + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } + } + if (int x = 0) // { dg-message "'int x' previously declared here" } + ; + else + { + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } + } + if (int x = 1) // { dg-message "'int x' previously declared here" } + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } + if (int x = 0) // { dg-message "'int x' previously declared here" } + ; + else + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } + switch (int x = 1) // { dg-message "'int x' previously declared here" } + { + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } + default:; + } + switch (int x = 1) // { dg-message "'int x' previously declared here" } + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } + while (int x = v) + { + extern int x (int); // { dg-error "'int x\\\(int\\\)' redeclared as different kind of entity" } + } + while (int x = v) + extern int x (int); // { dg-error "'int x\\\(int\\\)' redeclared as different kind of entity" } + for (int x = v; x; ++x) // { dg-message "'int x' previously declared here" } + { + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } + } + for (int x = v; x; ++x) // { dg-message "'int x' previously declared here" } + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } + for (; int x = v; ) + { + extern int x (int); // { dg-error "'int x\\\(int\\\)' redeclared as different kind of entity" } + } + for (; int x = v; ) + extern int x (int); // { dg-error "'int x\\\(int\\\)' redeclared as different kind of entity" } + try + { + } + catch (int x) // { dg-message "'int x' previously declared here" } + { + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } + } +} + +void +corge (int x) // { dg-message "'int x' previously declared here" } +try +{ +} +catch (...) +{ + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } +} + +void +fred (int x) // { dg-message "'int x' previously declared here" } +try +{ +} +catch (int) +{ +} +catch (long) +{ + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } +} + +void +garply (int x) +{ + try + { + extern int x (int); + } + catch (...) + { + extern int x (int); + } +} + +struct S +{ + S (int x) // { dg-message "'int x' previously declared here" } + try : s (x) + { + extern int x (int); // { dg-error "declaration of 'int x\\\(int\\\)' shadows a parameter" } + } + catch (...) + { + } + int s; +}; + +struct T +{ + T (int x) // { dg-message "'int x' previously declared here" } + try : t (x) + { + } + catch (...) + { + extern int x (int); // { dg-error "redeclaration of 'int x\\\(int\\\)'" } + } + int t; +}; + +struct U +{ + U (int x) : u (x) + { + try + { + extern int x (int); + } + catch (...) + { + extern int x (int); + } + } + int u; +}; + +struct V +{ + V (int x) : v (x) + { + { + extern int x (int); + } + } + int v; +}; --- gcc/testsuite/g++.dg/warn/Wshadow-19.C.jj 2023-09-01 11:35:21.092200057 +0200 +++ gcc/testsuite/g++.dg/warn/Wshadow-19.C 2023-09-01 11:37:15.997598483 +0200 @@ -0,0 +1,27 @@ +// { dg-do compile } +// { dg-options "-Wshadow" } + +void +foo (int x) +{ + int y = 1; + { + extern int x; // { dg-warning "declaration of 'int x' shadows a parameter" } + extern int y; // { dg-warning "declaration of 'y' shadows a previous local" } + } +#if __cplusplus >= 201102L + auto fn = [x] () { extern int x; return 0; }; // { dg-warning "declaration of 'x' shadows a lambda capture" "" { target c++11 } } +#endif +} + +int z; + +struct S +{ + int x; + void foo () + { + extern int x; // { dg-warning "declaration of 'x' shadows a member of 'S'" } + extern int z; + } +}; Jakub