On 9/1/23 09:24, Jakub Jelinek wrote:
On Thu, Aug 31, 2023 at 03:52:22PM -0400, Jason Merrill wrote:
On 8/31/23 03:20, Jakub Jelinek wrote:
As the following testcase shows, while check_local_shadow diagnoses most of
the [basic.scope.block]/2 violations, it doesn't diagnose when parameter's
name is redeclared inside of the compound-stmt of a function-try-block.
There is in that case an extra scope (sk_try with parent artificial
sk_block with for FUNCTION_NEEDS_BODY_BLOCK another sk_block and only then
sk_function_param).
The following patch fixes that.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-08-31 Jakub Jelinek <ja...@redhat.com>
PR c++/52953
* cp-tree.h (struct language_function): Add x_in_function_try_block
member.
How about adding a flag to cp_binding_level instead? Maybe to mark the
artificial sk_block level as such, which we could use for both this case and
the FUNCTION_NEEDS_BODY_BLOCK cases.
So like this?
It actually changes behaviour on the
void foo (int x) try {} catch (int x) {} case, where previously
this triggered the
|| (TREE_CODE (old) == PARM_DECL
&& (current_binding_level->kind == sk_catch
|| current_binding_level->level_chain->kind == sk_catch)
&& in_function_try_handler))
{
auto_diagnostic_group d;
if (permerror (DECL_SOURCE_LOCATION (decl),
"redeclaration of %q#D", decl))
inform (DECL_SOURCE_LOCATION (old),
"%q#D previously declared here", old);
diagnostics (note, just the current_binding_level->kind == sk_catch
case), while now it triggers already the earlier
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);
error. If you think it is important to differentiate that,
I guess I could guard the while (b->artificial) loop with say
+ if (!in_function_try_handler
+ || current_binding_level->kind != sk_catch)
while (b->artificial)
b = b->level_chain;
and adjust the 2 testcases.
I don't mind the change of diagnostic. Can we remove the
in_function_try_handler test, then, if it's no longer reachable? OK
with that change.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
or with modification?
2023-09-01 Jakub Jelinek <ja...@redhat.com>
PR c++/52953
* name-lookup.h (struct cp_binding_level): Add artificial bit-field.
Formatting fixes.
* name-lookup.cc (check_local_shadow): Skip artificial bindings when
checking if parameter scope is parent scope. Don't special case
FUNCTION_NEEDS_BODY_BLOCK.
* decl.cc (begin_function_body): Set
current_binding_level->artificial.
* semantics.cc (begin_function_try_block): Likewise.
* g++.dg/diagnostic/redeclaration-3.C: New test.
* g++.dg/parse/pr31952-3.C: Expect different diagnostic wording.
--- gcc/cp/name-lookup.h.jj 2023-08-21 11:57:33.105460770 +0200
+++ gcc/cp/name-lookup.h 2023-09-01 10:15:20.137943395 +0200
@@ -292,11 +292,11 @@ struct GTY(()) cp_binding_level {
only valid if KIND == SK_TEMPLATE_PARMS. */
BOOL_BITFIELD explicit_spec_p : 1;
- /* true means make a BLOCK for this level regardless of all else. */
+ /* True means make a BLOCK for this level regardless of all else. */
unsigned keep : 1;
/* Nonzero if this level can safely have additional
- cleanup-needing variables added to it. */
+ cleanup-needing variables added to it. */
unsigned more_cleanups_ok : 1;
unsigned have_cleanups : 1;
@@ -308,9 +308,13 @@ struct GTY(()) cp_binding_level {
unsigned defining_class_p : 1;
/* True for SK_FUNCTION_PARMS of a requires-expression. */
- unsigned requires_expression: 1;
+ unsigned requires_expression : 1;
- /* 22 bits left to fill a 32-bit word. */
+ /* True for artificial blocks which should be ignored when finding
+ parent scope. */
+ unsigned artificial : 1;
+
+ /* 21 bits left to fill a 32-bit word. */
};
/* The binding level currently in effect. */
--- gcc/cp/name-lookup.cc.jj 2023-08-31 14:31:06.055762306 +0200
+++ gcc/cp/name-lookup.cc 2023-09-01 10:21:03.658118594 +0200
@@ -3146,8 +3146,10 @@ check_local_shadow (tree decl)
them there. */
cp_binding_level *b = current_binding_level->level_chain;
- if (FUNCTION_NEEDS_BODY_BLOCK (current_function_decl))
- /* Skip the ctor/dtor cleanup level. */
+ /* Skip artificially added scopes which aren't present
+ in the C++ standard, e.g. for function-try-block or
+ ctor/dtor cleanups. */
+ while (b->artificial)
b = b->level_chain;
/* [basic.scope.param] A parameter name shall not be redeclared
--- gcc/cp/decl.cc.jj 2023-08-31 20:48:21.127722180 +0200
+++ gcc/cp/decl.cc 2023-09-01 10:18:40.125134543 +0200
@@ -18002,6 +18002,7 @@ begin_function_body (void)
keep_next_level (true);
tree stmt = begin_compound_stmt (BCS_FN_BODY);
+ current_binding_level->artificial = 1;
if (processing_template_decl)
/* Do nothing now. */;
--- gcc/cp/semantics.cc.jj 2023-08-31 14:31:06.102761661 +0200
+++ gcc/cp/semantics.cc 2023-09-01 10:17:49.044851974 +0200
@@ -1624,6 +1624,7 @@ begin_function_try_block (tree *compound
/* This outer scope does not exist in the C++ standard, but we need
a place to put __FUNCTION__ and similar variables. */
*compound_stmt = begin_compound_stmt (0);
+ current_binding_level->artificial = 1;
r = begin_try_block ();
FN_TRY_BLOCK_P (r) = 1;
return r;
--- gcc/testsuite/g++.dg/diagnostic/redeclaration-3.C.jj 2023-09-01
09:40:12.061707730 +0200
+++ gcc/testsuite/g++.dg/diagnostic/redeclaration-3.C 2023-09-01
10:32:36.315429575 +0200
@@ -0,0 +1,204 @@
+// PR c++/52953
+// { dg-do compile }
+// { dg-options "-pedantic-errors -Wno-switch-unreachable" }
+
+void
+foo (int x) // { dg-message "'int x' previously declared
here" }
+{
+ int x; // { dg-error "declaration of 'int x'
shadows a parameter" }
+}
+
+void
+bar (int x) // { dg-message "'int x' previously declared
here" }
+try
+{
+ int x; // { dg-error "declaration of 'int x'
shadows a parameter" }
+}
+catch (...)
+{
+}
+
+volatile int v;
+
+void
+baz ()
+{
+#if __cplusplus >= 201103L
+ auto f = [] (int x) { 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" }
+ {
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ }
+ if (int x = 0) // { dg-message "'int x' previously declared
here" }
+ ;
+ else
+ {
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ }
+ if (int x = 1) // { dg-message "'int x' previously declared
here" }
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ if (int x = 0) // { dg-message "'int x' previously declared
here" }
+ ;
+ else
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ switch (int x = 1) // { dg-message "'int x' previously declared
here" }
+ {
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ default:;
+ }
+ switch (int x = 1) // { dg-message "'int x' previously declared
here" }
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ while (int x = v) // { dg-message "'int x' previously declared
here" }
+ {
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ }
+ while (int x = v) // { dg-message "'int x' previously declared
here" }
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ for (int x = v; x; ++x) // { dg-message "'int x' previously declared
here" }
+ {
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ }
+ for (int x = v; x; ++x) // { dg-message "'int x' previously declared
here" }
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ for (; int x = v; ) // { dg-message "'int x' previously declared
here" }
+ {
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ }
+ for (; int x = v; ) // { dg-message "'int x' previously declared
here" }
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ try
+ {
+ }
+ catch (int x) // { dg-message "'int x' previously
declared here" }
+ {
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ }
+ if (int x = 1)
+ if (int x = 1)
+ ;
+ if (int x = 0)
+ ;
+ else
+ if (int x = 1)
+ ;
+ if (int x = 1)
+ switch (int x = 1)
+ ;
+ if (int x = 0)
+ while (int x = v)
+ ;
+ if (int x = 0)
+ for (int x = v; x; ++x)
+ ;
+ switch (int x = 1)
+ switch (int x = 1)
+ {
+ case 1:;
+ }
+ while (int x = 0)
+ if (int x = 1)
+ ;
+ for (int x = v; x; ++x)
+ for (int x = v; x; ++x)
+ ;
+}
+
+void
+qux (int x) // { dg-message "'int x' previously declared
here" }
+try
+{
+}
+catch (int x) // { dg-error "declaration of 'int x'
shadows a parameter" }
+{
+}
+
+void
+corge (int x) // { dg-message "'int x' previously declared
here" }
+try
+{
+}
+catch (...)
+{
+ int x; // { dg-error "redeclaration of 'int
x'" }
+}
+
+void
+fred (int x) // { dg-message "'int x' previously declared
here" }
+try
+{
+}
+catch (int)
+{
+}
+catch (long)
+{
+ int x; // { dg-error "redeclaration of 'int
x'" }
+}
+
+void
+garply (int x)
+{
+ try
+ {
+ int x;
+ }
+ catch (...)
+ {
+ int x;
+ }
+}
+
+struct S
+{
+ S (int x) // { dg-message "'int x' previously declared
here" }
+ try : s (x)
+ {
+ 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 (...)
+ {
+ int x; // { dg-error "redeclaration of 'int
x'" }
+ }
+ int t;
+};
+
+struct U
+{
+ U (int x) : u (x)
+ {
+ try
+ {
+ int x;
+ }
+ catch (...)
+ {
+ int x;
+ }
+ }
+ int u;
+};
+
+struct V
+{
+ V (int x) : v (x)
+ {
+ {
+ int x;
+ }
+ }
+ int v;
+};
--- gcc/testsuite/g++.dg/parse/pr31952-3.C.jj 2020-01-14 20:02:46.918607812
+0100
+++ gcc/testsuite/g++.dg/parse/pr31952-3.C 2023-09-01 14:41:12.306964831
+0200
@@ -6,7 +6,7 @@ try
{
return 0;
}
-catch (int bar) // { dg-error "redeclaration" }
+catch (int bar) // { dg-error "shadows a parameter" }
{
return 1;
}
Jakub
