On Fri, Sep 01, 2023 at 03:24:54PM +0200, Jakub Jelinek via Gcc-patches wrote:
> 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.
BTW, that in_function_try_handler case doesn't work correctly
(before/after this patch),
void
foo (int x)
try {
}
catch (int)
{
try {
} catch (int x)
{
}
try {
} catch (int)
{
int x;
}
}
(which is valid) is rejected, because
|| (TREE_CODE (old) == PARM_DECL
&& (current_binding_level->kind == sk_catch
|| current_binding_level->level_chain->kind == sk_catch)
&& in_function_try_handler))
is true but nothing verified that for the first case
current_binding_level->level_chain->kind == sk_function_params
(with perhaps artificial scopes in between and in the latter case
with one extra level in between).
Here is an untested variant of the patch which does diagnostics of the
in_function_try_handler cases only if it is proven there are no intervening
non-artificial scopes, but uses the old wording + permerror for that case
like before.
Another possibility would be to use permerror for that case but the
"declaration of %q#D shadows a parameter" wording (then we'd need to adjust
both testcases, each on one line).
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. Diagnose the in_function_try_handler
cases in the b->kind == sk_function_parms test, verify no
non-artificial intervening scopes but use permerror for that case with
different wording. Add missing auto_diagnostic_group.
* 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.
--- gcc/cp/name-lookup.h.jj 2023-09-01 12:15:22.574619674 +0200
+++ gcc/cp/name-lookup.h 2023-09-01 16:11:47.838401045 +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-09-01 12:15:22.566619785 +0200
+++ gcc/cp/name-lookup.cc 2023-09-01 16:19:12.567335710 +0200
@@ -3146,18 +3146,34 @@ 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. */
+ if (in_function_try_handler && b->kind == sk_catch)
+ b = b->level_chain;
+
+ /* 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
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;
+ /* The [basic.scope.block]/(2.3) - handle of a function-try-block
+ used to emit a different diagnostics, preserve that. */
+ if (in_function_try_handler
+ && (current_binding_level->kind == sk_catch
+ || current_binding_level->level_chain->kind == sk_catch))
+ emit = permerror (DECL_SOURCE_LOCATION (decl),
+ "redeclaration of %q#D", 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;
}
}
@@ -3194,17 +3210,10 @@ check_local_shadow (tree decl)
shall not be redeclared in the outermost block of the handler.
3.3.3/2: A parameter name shall not be redeclared (...) in
the outermost block of any handler associated with a
- function-try-block.
- 3.4.1/15: The function parameter names shall not be redeclared
- in the exception-declaration nor in the outermost block of a
- handler for the function-try-block. */
- else if ((TREE_CODE (old) == VAR_DECL
- && old_scope == current_binding_level->level_chain
- && old_scope->kind == sk_catch)
- || (TREE_CODE (old) == PARM_DECL
- && (current_binding_level->kind == sk_catch
- || current_binding_level->level_chain->kind == sk_catch)
- && in_function_try_handler))
+ function-try-block. */
+ else if (TREE_CODE (old) == VAR_DECL
+ && old_scope == current_binding_level->level_chain
+ && old_scope->kind == sk_catch)
{
auto_diagnostic_group d;
if (permerror (DECL_SOURCE_LOCATION (decl),
--- gcc/cp/semantics.cc.jj 2023-09-01 12:15:22.650618611 +0200
+++ gcc/cp/semantics.cc 2023-09-01 16:11:47.844400963 +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/cp/decl.cc.jj 2023-09-01 15:07:40.937661100 +0200
+++ gcc/cp/decl.cc 2023-09-01 16:11:47.842400991 +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/testsuite/g++.dg/diagnostic/redeclaration-3.C.jj 2023-09-01
16:11:47.844400963 +0200
+++ gcc/testsuite/g++.dg/diagnostic/redeclaration-3.C 2023-09-01
16:24:21.865117426 +0200
@@ -0,0 +1,225 @@
+// 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 "redeclaration of 'int
x'" }
+{
+}
+
+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;
+};
+
+void
+foobar (int x)
+try
+{
+}
+catch (int)
+{
+ try
+ {
+ }
+ catch (int x)
+ {
+ }
+ try
+ {
+ } catch (int)
+ {
+ int x;
+ }
+}
Jakub
