On 3/3/20 8:03 AM, Jakub Jelinek wrote:
Hi!
We ICE on the following testcase since I've added the SAVE_EXPR-like
constexpr handling where the TARGET_EXPR initializer (and cleanup) is
evaluated only once (because it might have side-effects like new or delete
expressions in it).
The problem is if the TARGET_EXPR (but I guess in theory SAVE_EXPR too)
initializer is *non_constant_p. We still remember the result, but already
not that it is *non_constant_p. Normally that wouldn't be a big problem,
if something is *non_constant_p, we only or into it and so the whole
expression will be non-constant too. Except in the builtins handling,
we try to evaluate the arguments with non_constant_p pointing into a dummy1
bool which we ignore. This is because some builtins might fold into a
constant even if they don't have a constexpr argument. Unfortunately if
we evaluate the TARGET_EXPR first in the argument of such a builtin and then
once again, we don't set *non_constant_p.
So, either we don't remember the TARGET_EXPR/SAVE_EXPR result if it wasn't
constant, like the following patch does, or we could remember it, but in
some way that would make it clear that it is non-constant (e.g. by
pushing into the global->values SAVE_EXPR, SAVE_EXPR entry and perhaps
for TARGET_EXPR don't remember it on TARGET_EXPR_SLOT, but the TARGET_EXPR
itself and similarly push TARGET_EXPR, TARGET_EXPR and if we see those
after the lookup, diagnose + set *non_constant_p. Or we could perhaps
during the builtin argument evaluation push expressions into a different
save_expr vec and undo them afterwards.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
2020-03-03 Jakub Jelinek <ja...@redhat.com>
PR c++/93998
* constexpr.c (cxx_eval_constant_expression)
<case TARGET_EXPR, case SAVE_EXPR>: Don't record anything if
*non_constant_p is true.
* g++.dg/ext/pr93998.C: New test.
--- gcc/cp/constexpr.c.jj 2020-02-27 09:28:46.227958669 +0100
+++ gcc/cp/constexpr.c 2020-03-02 18:29:38.014333067 +0100
@@ -5474,9 +5474,10 @@ cxx_eval_constant_expression (const cons
r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 1),
false,
non_constant_p, overflow_p);
- if (!*non_constant_p)
- /* Adjust the type of the result to the type of the temporary. */
- r = adjust_temp_type (TREE_TYPE (t), r);
+ if (*non_constant_p)
+ break;
+ /* Adjust the type of the result to the type of the temporary. */
+ r = adjust_temp_type (TREE_TYPE (t), r);
if (TARGET_EXPR_CLEANUP (t) && !CLEANUP_EH_ONLY (t))
ctx->global->cleanups->safe_push (TARGET_EXPR_CLEANUP (t));
r = unshare_constructor (r);
@@ -5528,6 +5529,8 @@ cxx_eval_constant_expression (const cons
{
r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0), false,
non_constant_p, overflow_p);
+ if (*non_constant_p)
+ break;
ctx->global->values.put (t, r);
if (ctx->save_exprs)
ctx->save_exprs->safe_push (t);
--- gcc/testsuite/g++.dg/ext/pr93998.C.jj 2020-03-02 18:40:14.843965039
+0100
+++ gcc/testsuite/g++.dg/ext/pr93998.C 2020-03-02 18:39:27.486661682 +0100
@@ -0,0 +1,14 @@
+// PR c++/93998
+// { dg-do compile { target c++11 } }
+
+struct C
+{
+ constexpr bool operator== (C x) const noexcept { return v == x.v; }
+ int v;
+};
+
+int
+foo (const C a, const C b, bool c)
+{
+ return __builtin_expect (!!(a == b || c), 1) ? 0 : 1;
+}
Jakub
