> 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. 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.)" > > With this patch, the code generated for the (inlined) call to > > ifbar equals that to swbar, except for the comparisons being > > in another order. > > > > gcc/cp: > > PR c++/113545 > > * constexpr.cc (label_matches): Replace call to_unreachable with > > "to gcc_unreachable" > > > return false. > > More like with "break" but that's not important. > > > gcc/testsuite: (Deleted -- see separate patch) > > --- > > gcc/cp/constexpr.cc | 3 +- > > gcc/testsuite/g++.dg/expr/pr113545.C | 49 +++++++++++++++++++++++++++++ > > 2 files changed, 51 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/g++.dg/expr/pr113545.C > > > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > > index 6350fe154085..30caf3322fff 100644 > > --- a/gcc/cp/constexpr.cc > > +++ b/gcc/cp/constexpr.cc > > @@ -6922,7 +6922,8 @@ label_matches (const constexpr_ctx *ctx, tree > > *jump_target, tree stmt) > > break; > > > > default: > > - gcc_unreachable (); > > + /* Something else, like CONVERT_EXPR. Unknown whether it matches. > > */ > > + break; > > } > > return false; > > } > > diff --git a/gcc/testsuite/g++.dg/expr/pr113545.C > > b/gcc/testsuite/g++.dg/expr/pr113545.C > > new file mode 100644 > > index 000000000000..914ffdeb8e16 brgds, H-P
