On 2/6/24 19:23, Hans-Peter Nilsson wrote:
Date: Mon, 22 Jan 2024 14:33:59 -0500
From: Marek Polacek <pola...@redhat.com>
On Mon, Jan 22, 2024 at 06:02:32PM +0100, Hans-Peter Nilsson wrote:
I don't really know whether this is the right way to treat
CONVERT_EXPR as below, but... Regtested native
x86_64-linux-gnu. Ok to commit?
Thanks for taking a look at this problem.
Thanks for the initial review.
brgds, H-P
-- >8 --
That gcc_unreachable at the default-label seems to be over
the top. It seems more correct to just say "that's not
constant" to whatever's not known (to be constant), when
looking for matches in switch-statements.
Unfortunately this doesn't seem correct to me; I don't think we
should have gotten that far. It appears that we lose track of
the reinterpret_cast, which is not allowed in a constant expression:
<http://eel.is/c++draft/expr.const#5.15>.
cp_convert -> ... -> convert_to_integer_1 gives us a CONVERT_EXPR
but we only set REINTERPRET_CAST_P on NOP_EXPRs:
expr = cp_convert (type, expr, complain);
if (TREE_CODE (expr) == NOP_EXPR)
/* Mark any nop_expr that created as a reintepret_cast. */
REINTERPRET_CAST_P (expr) = true;
so when evaluating baz we get (long unsigned int) &foo, which
passes verify_constant.
I don't have a good suggestion yet, sorry.
But, with this patch, we're letting the non-constant case
take the same path and failing for the same reason, albeit
much later than desired, for the switch code as for the
if-chain code. Isn't that better than the current ICE?
I mean, if there's a risk of accepts-invalid (like, some
non-constant case incorrectly "constexpr'd"), then that risk
is as already there, for the if-chain case.
Anyway, this is a bit too late in the release season and
isn't a regression, thus I can't argue for it being a
suitable stop-gap measure...
I'm unassigning myself from the PR as I have no clue how to
fix the actual non-constexpr-operand-seen-too-late bug.
I think it would be better to check in cxx_eval_switch_expr that the
condition is an INTEGER_CST, since VERIFY_CONSTANT isn't specific enough
in this case, like the attached:
Though, I'm asking again; any clue regarding:
"I briefly considered one of the cpp[0-9a-z]* subdirectories
but found no rule.
Isn't constexpr c++11 and therefor cpp0x isn't a good match
(contrary to the many constexpr tests therein)?
What *is* the actual rule for putting a test in
g++.dg/cpp0x, cpp1x and cpp1y (et al)?
(I STFW but found nothing.)"
C++11 was called C++0x until it was actually done, a couple of years
later than expected. Since that experience the C++ committee has moved
to time-based rather than feature-based releases...
Incidentally, these testcases seem to require C++14; you can't have a
switch in a constexpr function in C++11.
Jason
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 2ebb1470dd5..fa346fe01c9 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -7106,6 +7106,16 @@ cxx_eval_switch_expr (const constexpr_ctx *ctx, tree t,
cond = cxx_eval_constant_expression (ctx, cond, vc_prvalue,
non_constant_p, overflow_p);
VERIFY_CONSTANT (cond);
+ if (TREE_CODE (cond) != INTEGER_CST)
+ {
+ /* If the condition doesn't reduce to an INTEGER_CST it isn't a usable
+ switch condition even if it's constant enough for other things
+ (c++/113545). */
+ gcc_checking_assert (ctx->quiet);
+ *non_constant_p = true;
+ return t;
+ }
+
*jump_target = cond;
tree body
