On 9/1/23 09:34, Jakub Jelinek wrote:
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?
OK.
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
