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. > 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. > 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: > * g++.dg/expr/pr113545.C: New text. "test" > --- > 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 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/expr/pr113545.C The problem seems to be more about conversion so g++.dg/conversion/reinterpret5.C or g++.dg/cpp0x/constexpr-reinterpret3.C seems more appropriate. > @@ -0,0 +1,49 @@ Please add PR c++/113545 > +// { dg-do run { target c++11 } } > + > +char foo; > + > +// This one caught a call to gcc_unreachable in > +// cp/constexpr.cc:label_matches, when passed a convert_expr from the > +// cast in the call. > +constexpr unsigned char swbar(__UINTPTR_TYPE__ baz) > +{ > + switch (baz) > + { > + case 13: > + return 11; > + case 14: > + return 78; > + case 2048: > + return 13; > + default: > + return 42; > + } > +} > + > +// For reference, the equivalent* if-statements. > +constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz) > +{ > + if (baz == 13) > + return 11; > + else if (baz == 14) > + return 78; > + else if (baz == 2048) > + return 13; > + else > + return 42; > +} > + > +__attribute__ ((__noipa__)) > +void xyzzy(int x) > +{ > + if (x != 42) > + __builtin_abort (); > +} > + > +int main() > +{ > + unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); > + xyzzy(c); > + unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); I suppose we should also test a C-style cast (which leads to a reinterpret_cast in this case). Maybe check we get an error when c/d are constexpr (that used to ICE). > + xyzzy(d); > +} > -- > 2.30.2 > Marek
