c++: Evaluate immediate invocation call arguments with mce_true [PR119150]
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap |
success
|
Build passed
|
| 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_simplebootstrap_build--master-aarch64-bootstrap |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
Commit Message
Hi!
Since Marek's r14-4140 which moved immediate invocation evaluation
from build_over_call to cp_fold_r, the following testcase is miscompiled.
The a = foo (bar ()); case is actually handled right, that is handled
in cp_fold_r and the whole CALL_EXPR is at that point evaluated by
cp_fold_immediate_r with cxx_constant_value (stmt, tf_none);
and that uses mce_true for evaluation of the argument as well as the actual
call.
But in the bool b = foo (bar ()); case we actually try to evaluate this
as non-manifestly constant-evaluated. And while
/* Make sure we fold std::is_constant_evaluated to true in an
immediate function. */
if (DECL_IMMEDIATE_FUNCTION_P (fun))
call_ctx.manifestly_const_eval = mce_true;
ensures that if consteval and __builtin_is_constant_evaluated () is true
inside of that call, this happens after arguments to the function
have been already constant evaluated in cxx_bind_parameters_in_call.
The call_ctx in that case also includes new call_ctx.call, something that
shouldn't be used for the arguments, so the following patch just arranges
to call cxx_bind_parameters_in_call with manifestly_constant_evaluated =
mce_true.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2025-03-07 Jakub Jelinek <jakub@redhat.com>
PR c++/119150
* constexpr.cc (cxx_eval_call_expression): Call
cxx_bind_parameters_in_call for immediate invocations with
manifestly_const_eval set to mce_true.
* g++.dg/cpp2a/consteval41.C: New test.
Jakub
Comments
On Fri, Mar 07, 2025 at 05:53:58PM +0100, Jakub Jelinek wrote:
> Hi!
>
> Since Marek's r14-4140 which moved immediate invocation evaluation
> from build_over_call to cp_fold_r, the following testcase is miscompiled.
>
> The a = foo (bar ()); case is actually handled right, that is handled
> in cp_fold_r and the whole CALL_EXPR is at that point evaluated by
> cp_fold_immediate_r with cxx_constant_value (stmt, tf_none);
> and that uses mce_true for evaluation of the argument as well as the actual
> call.
>
> But in the bool b = foo (bar ()); case we actually try to evaluate this
> as non-manifestly constant-evaluated. And while
> /* Make sure we fold std::is_constant_evaluated to true in an
> immediate function. */
> if (DECL_IMMEDIATE_FUNCTION_P (fun))
> call_ctx.manifestly_const_eval = mce_true;
> ensures that if consteval and __builtin_is_constant_evaluated () is true
> inside of that call, this happens after arguments to the function
> have been already constant evaluated in cxx_bind_parameters_in_call.
> The call_ctx in that case also includes new call_ctx.call, something that
> shouldn't be used for the arguments, so the following patch just arranges
> to call cxx_bind_parameters_in_call with manifestly_constant_evaluated =
> mce_true.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Seems fine, thanks.
> 2025-03-07 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/119150
> * constexpr.cc (cxx_eval_call_expression): Call
> cxx_bind_parameters_in_call for immediate invocations with
> manifestly_const_eval set to mce_true.
>
> * g++.dg/cpp2a/consteval41.C: New test.
>
> --- gcc/cp/constexpr.cc.jj 2025-03-01 09:13:17.694075636 +0100
> +++ gcc/cp/constexpr.cc 2025-03-07 13:45:35.909164389 +0100
> @@ -3074,9 +3074,21 @@ cxx_eval_call_expression (const constexp
> || cp_noexcept_operand);
>
> bool non_constant_args = false;
> - new_call.bindings
> - = cxx_bind_parameters_in_call (ctx, t, fun, non_constant_p,
> - overflow_p, &non_constant_args);
> + /* An immediate invocation is manifestly constant evaluated including the
> + arguments of the call, so use mce_true for the argument evaluation. */
> + if (DECL_IMMEDIATE_FUNCTION_P (fun)
> + && ctx->manifestly_const_eval != mce_true)
> + {
> + constexpr_ctx call_ctx = *ctx;
> + call_ctx.manifestly_const_eval = mce_true;
> + new_call.bindings
> + = cxx_bind_parameters_in_call (&call_ctx, t, fun, non_constant_p,
> + overflow_p, &non_constant_args);
> + }
> + else
> + new_call.bindings
> + = cxx_bind_parameters_in_call (ctx, t, fun, non_constant_p,
> + overflow_p, &non_constant_args);
>
> /* We build up the bindings list before we know whether we already have this
> call cached. If we don't end up saving these bindings, ggc_free them when
> --- gcc/testsuite/g++.dg/cpp2a/consteval41.C.jj 2025-03-07 13:39:34.526101144 +0100
> +++ gcc/testsuite/g++.dg/cpp2a/consteval41.C 2025-03-07 13:38:38.128871572 +0100
> @@ -0,0 +1,37 @@
> +// PR c++/119150
> +// { dg-do run { target c++20 } }
> +
> +consteval bool
> +foo (bool x)
> +{
> + return x;
> +}
> +
> +constexpr bool
> +bar ()
> +{
> +#if __cpp_if_consteval >= 202106L
> + if consteval
> + {
> + return true;
> + }
> + else
> + {
> + return false;
> + }
> +#else
> + return __builtin_is_constant_evaluated ();
> +#endif
> +}
> +
> +int
> +main ()
> +{
> + bool a = false;
> + a = foo (bar ());
> + if (!a)
> + __builtin_abort ();
> + bool b = foo (bar ());
> + if (!b)
> + __builtin_abort ();
> +}
>
> Jakub
>
Marek
On 3/7/25 11:53 AM, Jakub Jelinek wrote:
> Hi!
>
> Since Marek's r14-4140 which moved immediate invocation evaluation
> from build_over_call to cp_fold_r, the following testcase is miscompiled.
>
> The a = foo (bar ()); case is actually handled right, that is handled
> in cp_fold_r and the whole CALL_EXPR is at that point evaluated by
> cp_fold_immediate_r with cxx_constant_value (stmt, tf_none);
> and that uses mce_true for evaluation of the argument as well as the actual
> call.
>
> But in the bool b = foo (bar ()); case we actually try to evaluate this
> as non-manifestly constant-evaluated. And while
> /* Make sure we fold std::is_constant_evaluated to true in an
> immediate function. */
> if (DECL_IMMEDIATE_FUNCTION_P (fun))
> call_ctx.manifestly_const_eval = mce_true;
> ensures that if consteval and __builtin_is_constant_evaluated () is true
> inside of that call, this happens after arguments to the function
> have been already constant evaluated in cxx_bind_parameters_in_call.
> The call_ctx in that case also includes new call_ctx.call, something that
> shouldn't be used for the arguments, so the following patch just arranges
> to call cxx_bind_parameters_in_call with manifestly_constant_evaluated =
> mce_true.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2025-03-07 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/119150
> * constexpr.cc (cxx_eval_call_expression): Call
> cxx_bind_parameters_in_call for immediate invocations with
> manifestly_const_eval set to mce_true.
>
> * g++.dg/cpp2a/consteval41.C: New test.
>
> --- gcc/cp/constexpr.cc.jj 2025-03-01 09:13:17.694075636 +0100
> +++ gcc/cp/constexpr.cc 2025-03-07 13:45:35.909164389 +0100
> @@ -3074,9 +3074,21 @@ cxx_eval_call_expression (const constexp
> || cp_noexcept_operand);
>
> bool non_constant_args = false;
> - new_call.bindings
> - = cxx_bind_parameters_in_call (ctx, t, fun, non_constant_p,
> - overflow_p, &non_constant_args);
> + /* An immediate invocation is manifestly constant evaluated including the
> + arguments of the call, so use mce_true for the argument evaluation. */
> + if (DECL_IMMEDIATE_FUNCTION_P (fun)
> + && ctx->manifestly_const_eval != mce_true)
> + {
> + constexpr_ctx call_ctx = *ctx;
Why not use new_ctx for this instead of creating another copy?
I would think once we see that we have an immediate function, we want to
go ahead and set ctx->manifestly_const_eval and
new_call->manifestly_const_eval to true. It looks like we currently
cache immediate invocations separately depending on whether they're in
an enclosing immediate function context, which seems redundant.
Incidentally, I don't remember why we need a separate call_ctx either.
Jason
@@ -3074,9 +3074,21 @@ cxx_eval_call_expression (const constexp
|| cp_noexcept_operand);
bool non_constant_args = false;
- new_call.bindings
- = cxx_bind_parameters_in_call (ctx, t, fun, non_constant_p,
- overflow_p, &non_constant_args);
+ /* An immediate invocation is manifestly constant evaluated including the
+ arguments of the call, so use mce_true for the argument evaluation. */
+ if (DECL_IMMEDIATE_FUNCTION_P (fun)
+ && ctx->manifestly_const_eval != mce_true)
+ {
+ constexpr_ctx call_ctx = *ctx;
+ call_ctx.manifestly_const_eval = mce_true;
+ new_call.bindings
+ = cxx_bind_parameters_in_call (&call_ctx, t, fun, non_constant_p,
+ overflow_p, &non_constant_args);
+ }
+ else
+ new_call.bindings
+ = cxx_bind_parameters_in_call (ctx, t, fun, non_constant_p,
+ overflow_p, &non_constant_args);
/* We build up the bindings list before we know whether we already have this
call cached. If we don't end up saving these bindings, ggc_free them when
@@ -0,0 +1,37 @@
+// PR c++/119150
+// { dg-do run { target c++20 } }
+
+consteval bool
+foo (bool x)
+{
+ return x;
+}
+
+constexpr bool
+bar ()
+{
+#if __cpp_if_consteval >= 202106L
+ if consteval
+ {
+ return true;
+ }
+ else
+ {
+ return false;
+ }
+#else
+ return __builtin_is_constant_evaluated ();
+#endif
+}
+
+int
+main ()
+{
+ bool a = false;
+ a = foo (bar ());
+ if (!a)
+ __builtin_abort ();
+ bool b = foo (bar ());
+ if (!b)
+ __builtin_abort ();
+}