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 <[email protected]>
> >
> > 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 <[email protected]>
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
