On 1/21/25 9:54 AM, Jason Merrill wrote:
On 1/20/25 5:58 PM, Marek Polacek wrote:
On Mon, Jan 20, 2025 at 12:39:03PM -0500, Jason Merrill wrote:
On 1/20/25 12:27 PM, Marek Polacek wrote:
On Mon, Jan 20, 2025 at 11:46:44AM -0500, Jason Merrill wrote:
On 1/20/25 10:27 AM, Marek Polacek wrote:
On Fri, Jan 17, 2025 at 06:38:45PM -0500, Jason Merrill wrote:
On 1/17/25 1:31 PM, Marek Polacek wrote:
On Fri, Jan 17, 2025 at 08:10:24AM -0500, Jason Merrill wrote:
On 1/16/25 8:04 PM, Marek Polacek wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
The recent r15-6369 unfortunately caused a bad wrong-code issue.
Here we have
TARGET_EXPR <D.2996, (void) (D.2996 =
{.status=0, .data={._vptr.Foo=&_ZTV3Foo + 16}})>
and call cp_fold_r -> maybe_constant_init with object=D.2996. In
cxx_eval_outermost_constant_expr we now take the type of the
object
if present. An object can't have type 'void' and so we
continue to
evaluate the initializer. That evaluates into a VOID_CST,
meaning
we disregard the whole initializer, and terrible things ensue.
In that case, I'd think we want to use the value of
'object' (which should
be in ctx.ctor?) instead of the return value of
cxx_eval_constant_expression.
Ah, I'm sorry I didn't choose that approach. Maybe like this,
then?
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK. Maybe also add an assert that TREE_TYPE (r) is close enough
to type?
Thanks. dg.exp passed with this extra assert:
@@ -8986,7 +8986,11 @@ cxx_eval_outermost_constant_expr (tree t,
bool allow_non_constant,
/* If we got a non-simple TARGET_EXPR, the initializer was a
sequence
of statements, and the result ought to be stored in
ctx.ctor. */
if (r == void_node && !constexpr_dtor && ctx.ctor)
- r = ctx.ctor;
+ {
+ r = ctx.ctor;
+ gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p
+ (TREE_TYPE (r), type));
+ }
I was thinking to add that assert in general, not just in this
case, to
catch any other instances of trying to return the wrong type.
Unfortunately this
+ /* Check we are not trying to return the wrong type. */
+ gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p
+ (initialized_type (r), type)
Why not just TREE_TYPE (r)?
Adjusted to use TREE_TYPE now.
+ || error_operand_p (type));
breaks too much, e.g. constexpr-prvalue2.C with struct A x struct B,
or pr82128.C
*(((struct C *) a)->D.2903._vptr.A + 8)
x
int (*) ()
I've also tried can_convert, or similar_type_p but no luck. Any
thoughts?
Those both sound like the sort of bugs the assert is intended to
catch. But
I suppose we can't add it without fixing them first.
In the latter case, probably by adding an explicit conversion from
the vtbl
slot type to the desired function pointer type.
In the former case, I don't see a constant-expression, so we
shouldn't be
trying to check the type of a nonexistent constant result?
As discussed earlier, this patch just returns the original expression if
the types don't match:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK, thanks!
-- >8 --
The recent r15-6369 unfortunately caused a bad wrong-code issue.
Here we have
TARGET_EXPR <D.2996, (void) (D.2996 =
{.status=0, .data={._vptr.Foo=&_ZTV3Foo + 16}})>
and call cp_fold_r -> maybe_constant_init with object=D.2996. In
cxx_eval_outermost_constant_expr we now take the type of the object
if present. An object can't have type 'void' and so we continue to
evaluate the initializer. That evaluates into a VOID_CST, meaning
we disregard the whole initializer, and terrible things ensue.
For non-simple TARGET_EXPRs, we should return ctx.ctor rather than
the result of cxx_eval_constant_expression.
PR c++/118396
PR c++/118523
gcc/cp/ChangeLog:
* constexpr.cc (cxx_eval_outermost_constant_expr): For non-simple
TARGET_EXPRs, return ctx.ctor rather than the result of
cxx_eval_constant_expression. If TYPE and the type of R don't
match, return the original expression.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-prvalue4.C: New test.
* g++.dg/cpp1y/constexpr-prvalue3.C: New test.
Reviewed-by: Jason Merrill <ja...@redhat.com>
---
gcc/cp/constexpr.cc | 9 +++-
.../g++.dg/cpp0x/constexpr-prvalue4.C | 33 ++++++++++++++
.../g++.dg/cpp1y/constexpr-prvalue3.C | 45 +++++++++++++++++++
3 files changed, 86 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 7ff38f8b5e5..9f950ffed74 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -8983,6 +8983,11 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
r = cxx_eval_constant_expression (&ctx, r, vc_prvalue,
&non_constant_p, &overflow_p);
+ /* If we got a non-simple TARGET_EXPR, the initializer was a sequence
+ of statements, and the result ought to be stored in ctx.ctor. */
+ if (r == void_node && !constexpr_dtor && ctx.ctor)
+ r = ctx.ctor;
+
if (!constexpr_dtor)
verify_constant (r, allow_non_constant, &non_constant_p,
&overflow_p);
else
@@ -9087,7 +9092,9 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
return r;
else if (non_constant_p && TREE_CONSTANT (r))
r = mark_non_constant (r);
- else if (non_constant_p)
+ else if (non_constant_p
+ /* Check we are not trying to return the wrong type. */
+ || !same_type_ignoring_top_level_qualifiers_p (type, TREE_TYPE
(r)))
return t;
Actually, I guess we also want to give an error if !allow_non_constant.
Jason