[v2] c++: fn redecl in fn scope wrongly accepted [PR116239]
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Test passed
|
Commit Message
On Wed, Sep 04, 2024 at 12:28:49PM -0400, Jason Merrill wrote:
> On 8/30/24 3:40 PM, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > -- >8 --
> > Redeclaration such as
> >
> > void f(void);
> > consteval void f(void);
> >
> > is invalid. In a namespace scope, we detect the collision in
> > validate_constexpr_redeclaration, but not when one declaration is
> > at block scope.
> >
> > When we have
> >
> > void f(void);
> > void g() { consteval void f(void); }
> >
> > we call pushdecl on the second f and call push_local_extern_decl_alias.
> > It finds the namespace-scope f:
> >
> > for (ovl_iterator iter (binding); iter; ++iter)
> > if (decls_match (decl, *iter, /*record_versions*/false))
> > {
> > alias = *iter;
> > break;
> > }
> >
> > but decls_match says they match so we just set DECL_LOCAL_DECL_ALIAS
> > (and do not call another pushdecl leading to duplicate_decls which
> > would detect mismatching return types, for example). I don't think
> > we want to change decls_match, so a simple fix is to detect the
> > problem in push_local_extern_decl_alias.
> >
> > PR c++/116239
> >
> > gcc/cp/ChangeLog:
> >
> > * cp-tree.h (validate_constexpr_redeclaration): Declare.
> > * decl.cc (validate_constexpr_redeclaration): No longer static.
> > * name-lookup.cc (push_local_extern_decl_alias): Call
> > validate_constexpr_redeclaration.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/diagnostic/redeclaration-6.C: New test.
> > ---
> > gcc/cp/cp-tree.h | 1 +
> > gcc/cp/decl.cc | 2 +-
> > gcc/cp/name-lookup.cc | 3 ++
> > .../g++.dg/diagnostic/redeclaration-6.C | 34 +++++++++++++++++++
> > 4 files changed, 39 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/diagnostic/redeclaration-6.C
> >
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index 2eeb5e3e8b1..1a763b683de 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -6992,6 +6992,7 @@ extern bool member_like_constrained_friend_p (tree);
> > extern bool fns_correspond (tree, tree);
> > extern int decls_match (tree, tree, bool = true);
> > extern bool maybe_version_functions (tree, tree, bool);
> > +extern bool validate_constexpr_redeclaration (tree, tree);
> > extern bool merge_default_template_args (tree, tree, bool);
> > extern tree duplicate_decls (tree, tree,
> > bool hiding = false,
> > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> > index 6458e96bded..4ad68d609d7 100644
> > --- a/gcc/cp/decl.cc
> > +++ b/gcc/cp/decl.cc
> > @@ -1412,7 +1412,7 @@ check_redeclaration_exception_specification (tree new_decl,
> > /* Return true if OLD_DECL and NEW_DECL agree on constexprness.
> > Otherwise issue diagnostics. */
> > -static bool
> > +bool
> > validate_constexpr_redeclaration (tree old_decl, tree new_decl)
> > {
> > old_decl = STRIP_TEMPLATE (old_decl);
> > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> > index 70ad4cbf3b5..6777d97ac2e 100644
> > --- a/gcc/cp/name-lookup.cc
> > +++ b/gcc/cp/name-lookup.cc
> > @@ -3708,6 +3708,9 @@ push_local_extern_decl_alias (tree decl)
> > }
> > }
> > + if (!validate_constexpr_redeclaration (alias, decl))
> > + return;
> > +
> > retrofit_lang_decl (decl);
> > DECL_LOCAL_DECL_ALIAS (decl) = alias;
> > }
>
> I don't think we need this in the case that we built a new alias and pushed
> it; in that case alias is built from decl, and should certainly match
> already. So let's put this call before we decide to build a new alias,
> either in the loop or just after it.
That's right. How about this?
dg.exp passed.
-- >8 --
Redeclaration such as
void f(void);
consteval void f(void);
is invalid. In a namespace scope, we detect the collision in
validate_constexpr_redeclaration, but not when one declaration is
at block scope.
When we have
void f(void);
void g() { consteval void f(void); }
we call pushdecl on the second f and call push_local_extern_decl_alias.
It finds the namespace-scope f:
for (ovl_iterator iter (binding); iter; ++iter)
if (decls_match (decl, *iter, /*record_versions*/false))
{
alias = *iter;
break;
}
but decls_match says they match so we just set DECL_LOCAL_DECL_ALIAS
(and do not call another pushdecl leading to duplicate_decls which
would detect mismatching return types, for example). I don't think
we want to change decls_match, so a simple fix is to detect the
problem in push_local_extern_decl_alias.
PR c++/116239
gcc/cp/ChangeLog:
* cp-tree.h (validate_constexpr_redeclaration): Declare.
* decl.cc (validate_constexpr_redeclaration): No longer static.
* name-lookup.cc (push_local_extern_decl_alias): Call
validate_constexpr_redeclaration.
gcc/testsuite/ChangeLog:
* g++.dg/diagnostic/redeclaration-6.C: New test.
---
gcc/cp/cp-tree.h | 1 +
gcc/cp/decl.cc | 2 +-
gcc/cp/name-lookup.cc | 2 ++
.../g++.dg/diagnostic/redeclaration-6.C | 34 +++++++++++++++++++
4 files changed, 38 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/diagnostic/redeclaration-6.C
base-commit: c755c7a32590e2eef5a8b062b9756c1513910246
Comments
On 9/4/24 1:18 PM, Marek Polacek wrote:
> On Wed, Sep 04, 2024 at 12:28:49PM -0400, Jason Merrill wrote:
>>> + if (!validate_constexpr_redeclaration (alias, decl))
>>> + return;
>>> +
>>> retrofit_lang_decl (decl);
>>> DECL_LOCAL_DECL_ALIAS (decl) = alias;
>>> }
>>
>> I don't think we need this in the case that we built a new alias and pushed
>> it; in that case alias is built from decl, and should certainly match
>> already. So let's put this call before we decide to build a new alias,
>> either in the loop or just after it.
>
> That's right. How about this?
Hmm, I'd still prefer to have it *before* the !alias case for locality;
putting it as an else to a 66-line if seems obscure.
But OK however.
> dg.exp passed.
>
> -- >8 --
> Redeclaration such as
>
> void f(void);
> consteval void f(void);
>
> is invalid. In a namespace scope, we detect the collision in
> validate_constexpr_redeclaration, but not when one declaration is
> at block scope.
>
> When we have
>
> void f(void);
> void g() { consteval void f(void); }
>
> we call pushdecl on the second f and call push_local_extern_decl_alias.
> It finds the namespace-scope f:
>
> for (ovl_iterator iter (binding); iter; ++iter)
> if (decls_match (decl, *iter, /*record_versions*/false))
> {
> alias = *iter;
> break;
> }
>
> but decls_match says they match so we just set DECL_LOCAL_DECL_ALIAS
> (and do not call another pushdecl leading to duplicate_decls which
> would detect mismatching return types, for example). I don't think
> we want to change decls_match, so a simple fix is to detect the
> problem in push_local_extern_decl_alias.
>
> PR c++/116239
>
> gcc/cp/ChangeLog:
>
> * cp-tree.h (validate_constexpr_redeclaration): Declare.
> * decl.cc (validate_constexpr_redeclaration): No longer static.
> * name-lookup.cc (push_local_extern_decl_alias): Call
> validate_constexpr_redeclaration.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/diagnostic/redeclaration-6.C: New test.
> ---
> gcc/cp/cp-tree.h | 1 +
> gcc/cp/decl.cc | 2 +-
> gcc/cp/name-lookup.cc | 2 ++
> .../g++.dg/diagnostic/redeclaration-6.C | 34 +++++++++++++++++++
> 4 files changed, 38 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/diagnostic/redeclaration-6.C
>
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 2eeb5e3e8b1..1a763b683de 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -6992,6 +6992,7 @@ extern bool member_like_constrained_friend_p (tree);
> extern bool fns_correspond (tree, tree);
> extern int decls_match (tree, tree, bool = true);
> extern bool maybe_version_functions (tree, tree, bool);
> +extern bool validate_constexpr_redeclaration (tree, tree);
> extern bool merge_default_template_args (tree, tree, bool);
> extern tree duplicate_decls (tree, tree,
> bool hiding = false,
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 6458e96bded..4ad68d609d7 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -1412,7 +1412,7 @@ check_redeclaration_exception_specification (tree new_decl,
> /* Return true if OLD_DECL and NEW_DECL agree on constexprness.
> Otherwise issue diagnostics. */
>
> -static bool
> +bool
> validate_constexpr_redeclaration (tree old_decl, tree new_decl)
> {
> old_decl = STRIP_TEMPLATE (old_decl);
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index 70ad4cbf3b5..98e15878657 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -3706,6 +3706,8 @@ push_local_extern_decl_alias (tree decl)
> /* Adjust visibility. */
> determine_visibility (alias);
> }
> + else if (!validate_constexpr_redeclaration (alias, decl))
> + return;
> }
>
> retrofit_lang_decl (decl);
> diff --git a/gcc/testsuite/g++.dg/diagnostic/redeclaration-6.C b/gcc/testsuite/g++.dg/diagnostic/redeclaration-6.C
> new file mode 100644
> index 00000000000..ed8d4af7792
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/diagnostic/redeclaration-6.C
> @@ -0,0 +1,34 @@
> +// PR c++/116239
> +// { dg-do compile { target c++20 } }
> +
> +consteval void f1();
> +void f2();
> +constexpr void f3();
> +void f4();
> +consteval void f5();
> +constexpr void f6();
> +
> +void
> +g ()
> +{
> + void f1(); // { dg-error "differs in .consteval." }
> + consteval void f2(); // { dg-error "differs in .consteval." }
> +
> + void f3(); // { dg-error "differs in .constexpr." }
> + constexpr void f4(); // { dg-error "differs in .constexpr." }
> +
> + consteval void f5();
> + constexpr void f6();
> +
> + void f7();
> + consteval void f7(); // { dg-error "differs in .consteval." }
> +
> + consteval void f8();
> + void f8(); // { dg-error "differs in .consteval." }
> +
> + void f9();
> + constexpr void f9(); // { dg-error "differs in .constexpr." }
> +
> + constexpr void f10();
> + void f10(); // { dg-error "differs in .constexpr." }
> +}
>
> base-commit: c755c7a32590e2eef5a8b062b9756c1513910246
@@ -6992,6 +6992,7 @@ extern bool member_like_constrained_friend_p (tree);
extern bool fns_correspond (tree, tree);
extern int decls_match (tree, tree, bool = true);
extern bool maybe_version_functions (tree, tree, bool);
+extern bool validate_constexpr_redeclaration (tree, tree);
extern bool merge_default_template_args (tree, tree, bool);
extern tree duplicate_decls (tree, tree,
bool hiding = false,
@@ -1412,7 +1412,7 @@ check_redeclaration_exception_specification (tree new_decl,
/* Return true if OLD_DECL and NEW_DECL agree on constexprness.
Otherwise issue diagnostics. */
-static bool
+bool
validate_constexpr_redeclaration (tree old_decl, tree new_decl)
{
old_decl = STRIP_TEMPLATE (old_decl);
@@ -3706,6 +3706,8 @@ push_local_extern_decl_alias (tree decl)
/* Adjust visibility. */
determine_visibility (alias);
}
+ else if (!validate_constexpr_redeclaration (alias, decl))
+ return;
}
retrofit_lang_decl (decl);
new file mode 100644
@@ -0,0 +1,34 @@
+// PR c++/116239
+// { dg-do compile { target c++20 } }
+
+consteval void f1();
+void f2();
+constexpr void f3();
+void f4();
+consteval void f5();
+constexpr void f6();
+
+void
+g ()
+{
+ void f1(); // { dg-error "differs in .consteval." }
+ consteval void f2(); // { dg-error "differs in .consteval." }
+
+ void f3(); // { dg-error "differs in .constexpr." }
+ constexpr void f4(); // { dg-error "differs in .constexpr." }
+
+ consteval void f5();
+ constexpr void f6();
+
+ void f7();
+ consteval void f7(); // { dg-error "differs in .consteval." }
+
+ consteval void f8();
+ void f8(); // { dg-error "differs in .consteval." }
+
+ void f9();
+ constexpr void f9(); // { dg-error "differs in .constexpr." }
+
+ constexpr void f10();
+ void f10(); // { dg-error "differs in .constexpr." }
+}