[PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

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?

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.

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
	return false.

gcc/testsuite:
	* g++.dg/expr/pr113545.C: New text.
---
 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
@@ -0,0 +1,49 @@
+// { 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));
+  xyzzy(d);
+}
-- 
2.30.2

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Marek Polacek]

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

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Mon, 22 Jan 2024 14:33:59 -0500
> From: Marek Polacek <polacek@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.

Honestly, it's more like posting a patch is more effective
than just opening a PR. ;)  And I was curious about that
constexpr thing that usually works, but blew up here.

> > 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.

The gcc_unreachable was indeed a clue in that direction.

>  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>.

B...b..but clang allows it... (and I have no clue about the
finer --or admittedly even coarser-- details of C++) ...and
not-my-code, just "porting" it.

Seriously though, thanks for the reference.  Also, maybe
something to consider for -fpermissive, if this changes to a
more graceful error path.

> 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.

Thanks for the review!

> > 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"

Oops!

> > 	return false.
> 
> More like with "break" but that's not important.

(Well, *effectively* return false.  I'd change to something
like "Replace call to gcc_unreachable with arrangement to
return false" if this were to go anywhere.)

> > gcc/testsuite:
> > 	* g++.dg/expr/pr113545.C: New text.
> 
> "test"

Gosh, horrible typos, thanks.

brgds, H-P

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Mon, 22 Jan 2024 14:33:59 -0500
> From: Marek Polacek <polacek@redhat.com>

Oh, there was more...  Also, I think I misinterpreted your
reply as meaning that the test-case is ice-on-invalid and I
wrongly replied in agreement to that misinterpretation. :)

(For others at a comparable level of (lack of) C++ knowledge
to me: my reading of
https://en.cppreference.com/w/cpp/language/constexpr is that
a constexpr function can be validly called with an
expression that isn't "constexpr" (compile-time computable,
immediately computable, core constant expressions or
whatever the term), and the test-case is a valid example (of
two such invocations).  So, an expression calling such a
function is only truly "constexpr" with the "right"
parameters.  I know this isn't C++ 102, but many of us
hacking gcc aren't originally c++ hackers; that was just
happenstance.  I was about to write "aren't C++ hackers" but
then again, C++ happened to gcc, and c++11 at that.)

> On Mon, Jan 22, 2024 at 06:02:32PM +0100, Hans-Peter Nilsson wrote:

> The problem seems to be more about conversion so g++.dg/conversion/reinterpret5.C
> or g++.dg/cpp0x/constexpr-reinterpret3.C seems more appropriate.

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.)

> > +++ b/gcc/testsuite/g++.dg/expr/pr113545.C
> > @@ -0,0 +1,49 @@
> 
> Please add
> 
> PR c++/113545

Certainly if the test is to change name and even if not
("git grep" wouldn't catch the file name).  Will do.

> > +  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).

But the expressions aren't declared constexpr, just const
(as in "here 'const' means run-time evaluation due to the
weirdness that is C++").

...oh, I see what you mean, these are valid, but you suggest
adding tests declared constexpr to check that they get a
matching error (not ICE :) !

Thanks again for the review, I think I'll at least re-work
the test-case into two separate ones.

brgds, H-P

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Mon, 22 Jan 2024 14:33:59 -0500
> From: Marek Polacek <polacek@redhat.com>

> 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

> > +  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).

Like this?  Not sure about the value of that variant, but here goes.

I checked that these behave as expected (xfail as ICE properly) without the
previosly posted patch to cp/constexpr.cc and XPASS with it applied.

Ok to commit?

-- >8 --
Subject: [PATCH] c++: testcases for PR113545 (constexpr with switch and
 passing non-constexpr parameter)

gcc/testsuite:
	PR c++/113545
	* g++.dg/cpp0x/constexpr-reinterpret3.C,
	g++.dg/cpp0x/constexpr-reinterpret4.C: New tests.
---
 .../g++.dg/cpp0x/constexpr-reinterpret3.C     | 55 +++++++++++++++++++
 .../g++.dg/cpp0x/constexpr-reinterpret4.C     | 54 ++++++++++++++++++
 2 files changed, 109 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C

diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
new file mode 100644
index 000000000000..319cc5e8bee9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
@@ -0,0 +1,55 @@
+// PR c++/113545
+// { dg-do run { target c++11 } }
+// { dg-ice "PR112545 - constexpr function with switch called for reinterpret_cast" }
+
+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));
+  xyzzy(d);
+  unsigned const char e = swbar((__UINTPTR_TYPE__) &foo);
+  xyzzy(e);
+  unsigned const char f = ifbar((__UINTPTR_TYPE__) &foo);
+  xyzzy(f);
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
new file mode 100644
index 000000000000..4d0fdf2c0a78
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
@@ -0,0 +1,54 @@
+// PR c++/113545
+// { dg-do compile { 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 constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
+  xyzzy(c);
+  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
+  xyzzy(d);
+  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
+  xyzzy(e);
+  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
+  xyzzy(f);
+}
-- 
2.30.2

Ping PATCH: testcase for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

[Hans-Peter Nilsson]

Ping for the xfailed testsuite patch below the review
(actual constexpr.cc patch to be handled separately):

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Tue, 23 Jan 2024 05:55:00 +0100
> 
> > Date: Mon, 22 Jan 2024 14:33:59 -0500
> > From: Marek Polacek <polacek@redhat.com>
> 
> > 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
> 
> > > +  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).
> 
> Like this?  Not sure about the value of that variant, but here goes.
> 
> I checked that these behave as expected (xfail as ICE properly) without the
> previosly posted patch to cp/constexpr.cc and XPASS with it applied.
> 
> Ok to commit?
> 
> -- >8 --
> Subject: [PATCH] c++: testcases for PR113545 (constexpr with switch and
>  passing non-constexpr parameter)
> 
> gcc/testsuite:
> 	PR c++/113545
> 	* g++.dg/cpp0x/constexpr-reinterpret3.C,
> 	g++.dg/cpp0x/constexpr-reinterpret4.C: New tests.
> ---
>  .../g++.dg/cpp0x/constexpr-reinterpret3.C     | 55 +++++++++++++++++++
>  .../g++.dg/cpp0x/constexpr-reinterpret4.C     | 54 ++++++++++++++++++
>  2 files changed, 109 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> 
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
> new file mode 100644
> index 000000000000..319cc5e8bee9
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
> @@ -0,0 +1,55 @@
> +// PR c++/113545
> +// { dg-do run { target c++11 } }
> +// { dg-ice "PR112545 - constexpr function with switch called for reinterpret_cast" }
> +
> +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));
> +  xyzzy(d);
> +  unsigned const char e = swbar((__UINTPTR_TYPE__) &foo);
> +  xyzzy(e);
> +  unsigned const char f = ifbar((__UINTPTR_TYPE__) &foo);
> +  xyzzy(f);
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> new file mode 100644
> index 000000000000..4d0fdf2c0a78
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> @@ -0,0 +1,54 @@
> +// PR c++/113545
> +// { dg-do compile { 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 constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
> +  xyzzy(c);
> +  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
> +  xyzzy(d);
> +  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
> +  xyzzy(e);
> +  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
> +  xyzzy(f);
> +}
> -- 
> 2.30.2
> 

Ping*2 PATCH: testcase for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Tue, 30 Jan 2024 06:18:45 +0100

> Ping for the xfailed testsuite patch below the review
> (actual constexpr.cc patch to be handled separately):

Ping*2.  Again, this is for the xfailed test-case only.

> 
> > From: Hans-Peter Nilsson <hp@axis.com>
> > Date: Tue, 23 Jan 2024 05:55:00 +0100
> > 
> > > Date: Mon, 22 Jan 2024 14:33:59 -0500
> > > From: Marek Polacek <polacek@redhat.com>
> > 
> > > 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
> > 
> > > > +  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).
> > 
> > Like this?  Not sure about the value of that variant, but here goes.
> > 
> > I checked that these behave as expected (xfail as ICE properly) without the
> > previosly posted patch to cp/constexpr.cc and XPASS with it applied.
> > 
> > Ok to commit?
> > 
> > -- >8 --
> > Subject: [PATCH] c++: testcases for PR113545 (constexpr with switch and
> >  passing non-constexpr parameter)
> > 
> > gcc/testsuite:
> > 	PR c++/113545
> > 	* g++.dg/cpp0x/constexpr-reinterpret3.C,
> > 	g++.dg/cpp0x/constexpr-reinterpret4.C: New tests.
> > ---
> >  .../g++.dg/cpp0x/constexpr-reinterpret3.C     | 55 +++++++++++++++++++
> >  .../g++.dg/cpp0x/constexpr-reinterpret4.C     | 54 ++++++++++++++++++
> >  2 files changed, 109 insertions(+)
> >  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
> >  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> > 
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
> > new file mode 100644
> > index 000000000000..319cc5e8bee9
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
> > @@ -0,0 +1,55 @@
> > +// PR c++/113545
> > +// { dg-do run { target c++11 } }
> > +// { dg-ice "PR112545 - constexpr function with switch called for reinterpret_cast" }
> > +
> > +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));
> > +  xyzzy(d);
> > +  unsigned const char e = swbar((__UINTPTR_TYPE__) &foo);
> > +  xyzzy(e);
> > +  unsigned const char f = ifbar((__UINTPTR_TYPE__) &foo);
> > +  xyzzy(f);
> > +}
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> > new file mode 100644
> > index 000000000000..4d0fdf2c0a78
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> > @@ -0,0 +1,54 @@
> > +// PR c++/113545
> > +// { dg-do compile { 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 constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
> > +  xyzzy(c);
> > +  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
> > +  xyzzy(d);
> > +  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
> > +  xyzzy(e);
> > +  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
> > +  xyzzy(f);
> > +}
> > -- 
> > 2.30.2
> > 
> 

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Mon, 22 Jan 2024 14:33:59 -0500
> From: Marek Polacek <polacek@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

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 2987 bytes --]

On 2/6/24 19:23, Hans-Peter Nilsson wrote:
>> Date: Mon, 22 Jan 2024 14:33:59 -0500
>> From: Marek Polacek <polacek@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

[-- Attachment #2: switch.patch --]
[-- Type: text/x-patch, Size: 704 bytes --]

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

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Marek Polacek]

On Wed, Feb 07, 2024 at 04:32:57PM -0500, Jason Merrill wrote:
> On 2/6/24 19:23, Hans-Peter Nilsson wrote:
> > > Date: Mon, 22 Jan 2024 14:33:59 -0500
> > > From: Marek Polacek <polacek@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

The patch makes sense to me, although I'm afraid that losing the
REINTERPRET_CAST_P flag will cause other issues.

HP, sorry that I never got back to you.  I would be more than happy to
take the patch above, add some tests and test/bootstrap it, unless you
want to do that yourself.

Thanks & sorry again,

Marek

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Wed, 7 Feb 2024 21:11:59 -0500
> From: Marek Polacek <polacek@redhat.com>

> On Wed, Feb 07, 2024 at 04:32:57PM -0500, Jason Merrill wrote:
> > On 2/6/24 19:23, Hans-Peter Nilsson wrote:
> > > > Date: Mon, 22 Jan 2024 14:33:59 -0500
> > > > From: Marek Polacek <polacek@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.

> > 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
> 
> The patch makes sense to me, although I'm afraid that losing the
> REINTERPRET_CAST_P flag will cause other issues.
> 
> HP, sorry that I never got back to you.  I would be more than happy to
> take the patch above, add some tests and test/bootstrap it, unless you
> want to do that yourself.
> 
> Thanks & sorry again,

No worries, feel very welcome to deal with handling the
actual fix.  Also, you're better prepared than me, when it
comes to dealing with any possible fallout. :)

I'll send an updated version of the test-cases, moving them
to the C++14 test directory (cpp1y, right?) and qualify them
as c++14 instead, as Jason pointed out.

brgds, H-P

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Marek Polacek]

On Thu, Feb 08, 2024 at 04:40:40PM +0100, Hans-Peter Nilsson wrote:
> > Date: Wed, 7 Feb 2024 21:11:59 -0500
> > From: Marek Polacek <polacek@redhat.com>
> 
> > On Wed, Feb 07, 2024 at 04:32:57PM -0500, Jason Merrill wrote:
> > > On 2/6/24 19:23, Hans-Peter Nilsson wrote:
> > > > > Date: Mon, 22 Jan 2024 14:33:59 -0500
> > > > > From: Marek Polacek <polacek@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.
> 
> > > 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
> > 
> > The patch makes sense to me, although I'm afraid that losing the
> > REINTERPRET_CAST_P flag will cause other issues.
> > 
> > HP, sorry that I never got back to you.  I would be more than happy to
> > take the patch above, add some tests and test/bootstrap it, unless you
> > want to do that yourself.
> > 
> > Thanks & sorry again,
> 
> No worries, feel very welcome to deal with handling the
> actual fix.  Also, you're better prepared than me, when it
> comes to dealing with any possible fallout. :)
> 
> I'll send an updated version of the test-cases, moving them
> to the C++14 test directory (cpp1y, right?) and qualify them
> as c++14 instead, as Jason pointed out.

Right, cpp1y is c++14.  Note that we wouldn't push the tests separately
to the patch itself, unless it's something that already works.  Thanks,

Marek

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Thu, 8 Feb 2024 10:44:31 -0500
> From: Marek Polacek <polacek@redhat.com>
> Cc: jason@redhat.com, gcc-patches@gcc.gnu.org
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> On Thu, Feb 08, 2024 at 04:40:40PM +0100, Hans-Peter Nilsson wrote:
> > > Date: Wed, 7 Feb 2024 21:11:59 -0500
> > > From: Marek Polacek <polacek@redhat.com>
> > 
> > > On Wed, Feb 07, 2024 at 04:32:57PM -0500, Jason Merrill wrote:
> > > > On 2/6/24 19:23, Hans-Peter Nilsson wrote:
> > > > > > Date: Mon, 22 Jan 2024 14:33:59 -0500
> > > > > > From: Marek Polacek <polacek@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.
> > 
> > > > 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
> > > 
> > > The patch makes sense to me, although I'm afraid that losing the
> > > REINTERPRET_CAST_P flag will cause other issues.
> > > 
> > > HP, sorry that I never got back to you.  I would be more than happy to
> > > take the patch above, add some tests and test/bootstrap it, unless you
> > > want to do that yourself.
> > > 
> > > Thanks & sorry again,
> > 
> > No worries, feel very welcome to deal with handling the
> > actual fix.  Also, you're better prepared than me, when it
> > comes to dealing with any possible fallout. :)
> > 
> > I'll send an updated version of the test-cases, moving them
> > to the C++14 test directory (cpp1y, right?) and qualify them
> > as c++14 instead, as Jason pointed out.
> 
> Right, cpp1y is c++14.  Note that we wouldn't push the tests separately
> to the patch itself, unless it's something that already works.  Thanks,
> 
> Marek

But, the tests work.  They passes as-is, as they track the
ICE, but will XPASS (that part) once a fix is committed (at
which time: "I checked that these behave as expected (xfail
as ICE properly) without the previosly posted patch to
cp/constexpr.cc and XPASS with it applied."

Once the fix works, the xfail for the ICE should be removed.
(Hm, better comment on the patches in a reply to that message. :)

The point is that for this type of bug it's useful to have a
test-case tracking it, before a fix is committed.

brgds, H-P

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Marek Polacek]

On Thu, Feb 08, 2024 at 05:07:21PM +0100, Hans-Peter Nilsson wrote:
> > Date: Thu, 8 Feb 2024 10:44:31 -0500
> > From: Marek Polacek <polacek@redhat.com>
> > Cc: jason@redhat.com, gcc-patches@gcc.gnu.org
> > Content-Type: text/plain; charset=us-ascii
> > Content-Disposition: inline
> > 
> > On Thu, Feb 08, 2024 at 04:40:40PM +0100, Hans-Peter Nilsson wrote:
> > > > Date: Wed, 7 Feb 2024 21:11:59 -0500
> > > > From: Marek Polacek <polacek@redhat.com>
> > > 
> > > > On Wed, Feb 07, 2024 at 04:32:57PM -0500, Jason Merrill wrote:
> > > > > On 2/6/24 19:23, Hans-Peter Nilsson wrote:
> > > > > > > Date: Mon, 22 Jan 2024 14:33:59 -0500
> > > > > > > From: Marek Polacek <polacek@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.
> > > 
> > > > > 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
> > > > 
> > > > The patch makes sense to me, although I'm afraid that losing the
> > > > REINTERPRET_CAST_P flag will cause other issues.
> > > > 
> > > > HP, sorry that I never got back to you.  I would be more than happy to
> > > > take the patch above, add some tests and test/bootstrap it, unless you
> > > > want to do that yourself.
> > > > 
> > > > Thanks & sorry again,
> > > 
> > > No worries, feel very welcome to deal with handling the
> > > actual fix.  Also, you're better prepared than me, when it
> > > comes to dealing with any possible fallout. :)
> > > 
> > > I'll send an updated version of the test-cases, moving them
> > > to the C++14 test directory (cpp1y, right?) and qualify them
> > > as c++14 instead, as Jason pointed out.
> > 
> > Right, cpp1y is c++14.  Note that we wouldn't push the tests separately
> > to the patch itself, unless it's something that already works.  Thanks,
> > 
> > Marek
> 
> But, the tests work.  They passes as-is, as they track the
> ICE, but will XPASS (that part) once a fix is committed (at
> which time: "I checked that these behave as expected (xfail
> as ICE properly) without the previosly posted patch to
> cp/constexpr.cc and XPASS with it applied."

I'm confused; are you planning to use the dg-ice directive I invented
some years ago?  I wanted to encourage people to add tests for
old unfixed PRs so that if a fix fixes an (un)related older problem, we
know it before pushing the patch.
(I don't think an XFAIL will work for an ICE -- that prompted dg-ice.)
 
> Once the fix works, the xfail for the ICE should be removed.
> (Hm, better comment on the patches in a reply to that message. :)
> 
> The point is that for this type of bug it's useful to have a
> test-case tracking it, before a fix is committed.

I'd tend to agree but here we already have a fix, so one commit seems
better than multiple commits.  But if that's what you want to do, I
guess I'm not going to stand in your way.

Marek

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Thu, 8 Feb 2024 11:22:47 -0500
> From: Marek Polacek <polacek@redhat.com>

> I'm confused; are you planning to use the dg-ice directive I invented
> some years ago?

Please, let's keep the discussion about the test-cases in
that thread.

brgds, H-P

[PATCH v2]: testcases for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

[Hans-Peter Nilsson]

> Date: Wed, 7 Feb 2024 16:32:57 -0500
> From: Jason Merrill <jason@redhat.com>

> Incidentally, these testcases seem to require C++14; you can't have a 
> switch in a constexpr function in C++11.

Update, v2 (from v1 that had a few requests from Marek
resolved from v0 that was posted together with my patch^Whack):
Move from cpp0x to cpp1y.  Qualify with c++14 instead of
c++11.  Add a one-liner commit-message.

I checked that these DTRT.  Currently:
Using "make check-gcc-c++ RUNTESTFLAGS=dg.exp=constexpr-reinterpret\*":

Running /x/gcc/gcc/testsuite/g++.dg/dg.exp ...

                === g++ Summary ===

# of expected passes            33
# of expected failures          3
# of unresolved testcases       3
# of unsupported tests          4

...and that there's an XPASS when a ICE-resolving patch is
applied, testing each of my hack and Jason's, both yield:

Using /x/gcc/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
Running /x/gcc/gcc/testsuite/g++.dg/dg.exp ...
XPASS: g++.dg/cpp1y/constexpr-reinterpret3.C  -std=c++14 (internal compiler error)
XPASS: g++.dg/cpp1y/constexpr-reinterpret3.C  -std=c++17 (internal compiler error)
XPASS: g++.dg/cpp1y/constexpr-reinterpret3.C  -std=c++20 (internal compiler error)

And (since it appears the benefit isn't obvious) why commit
a test-cases before the fix?  Well, I'm not dead sure the
fix both gets there soon, and that it then stays there.
Even if it does so within decent time, as an exception
(IIUC) because we're in stage 4 and the bug is not a
regression, it could easily be reverted if it'd uncover some
other wart that's not sufficiently easily resolved.

Also, the fix seems to me sufficiently remote to the
gcc_assert, that the execution path leading to it could be
diverted; hidden or resolved while fixing something else,
and then it'd be nice to know that it fixed *this* bug too.

To wit: More dg-ice test-cases!  Don't fear adding xfails or
dg-ices!  Marek, you write that dg-ice support.  Thank you!

Ok to commit?

-- >8 --

gcc/testsuite:

Test-cases, with constexpr-reinterpret3.C dg-ice:ing the PR c++/113545 bug.

	PR c++/113545
	* g++.dg/cpp1y/constexpr-reinterpret3.C,
	g++.dg/cpp1y/constexpr-reinterpret4.C: New tests.
---
 .../g++.dg/cpp1y/constexpr-reinterpret3.C     | 55 +++++++++++++++++++
 .../g++.dg/cpp1y/constexpr-reinterpret4.C     | 54 ++++++++++++++++++
 2 files changed, 109 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C

diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
new file mode 100644
index 000000000000..ed914383f78b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
@@ -0,0 +1,55 @@
+// PR c++/113545
+// { dg-do run { target c++14 } }
+// { dg-ice "PR112545 - constexpr function with switch called for reinterpret_cast" }
+
+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));
+  xyzzy(d);
+  unsigned const char e = swbar((__UINTPTR_TYPE__) &foo);
+  xyzzy(e);
+  unsigned const char f = ifbar((__UINTPTR_TYPE__) &foo);
+  xyzzy(f);
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
new file mode 100644
index 000000000000..9aaa6e463bc6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
@@ -0,0 +1,54 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+
+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 constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
+  xyzzy(c);
+  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
+  xyzzy(d);
+  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
+  xyzzy(e);
+  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
+  xyzzy(f);
+}
-- 
2.30.2

[PATCH v3]: testcases for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

[Hans-Peter Nilsson]

Bah.  Linaro's CI didn't like that there were UNRESOLVEDs
due to this patch.  Running it "as usual" didn't show
anything suspicious.  Sure, there were "# of unresolved
testcases 3" in the summary (see v2), but no error or other
special message from dejagnu.  Perhaps there could be a way
to have dg-ice automatically downgrade a dg-do run that
failed due to the ICE to a dg-do compile (or just not emit
the UNRESOLVED note), but pragmatically, this is a rare
enough case to not bother.  It looks like these were the
only UNRESOLVEDs in that CI run, so I'm just going to make a
mental note and move on.

For more comments, please see v2 of this patch.

v3: Change dg-do run to dg-do compile to avoid an UNRESOLVED.
Tested as with v2.  Ok to commit?

-- >8 --

Test-cases, with constexpr-reinterpret3.C dg-ice:ing the PR c++/113545 bug.

Regarding the request in the comment, a dg-do run when there's an ICE
will cause some CI's to signal an error for the run being "UNRESOLVED"
(compilation failed to produce executable).  Note that dejagnu (1.6.3)
itself doesn't consider this an error.

gcc/testsuite:
	PR c++/113545
	* g++.dg/cpp1y/constexpr-reinterpret3.C,
	g++.dg/cpp1y/constexpr-reinterpret4.C: New tests.
---
 .../g++.dg/cpp1y/constexpr-reinterpret3.C     | 56 +++++++++++++++++++
 .../g++.dg/cpp1y/constexpr-reinterpret4.C     | 54 ++++++++++++++++++
 2 files changed, 110 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C

diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
new file mode 100644
index 000000000000..6c396bff72b6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
@@ -0,0 +1,56 @@
+// PR c++/113545
+// { dg-do run { target c++14 } }
+// Please change the above "dg-do compile" to "dg-do run" when the ICE is resolved.
+// { dg-ice "PR112545 - constexpr function with switch called for reinterpret_cast" }
+
+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));
+  xyzzy(d);
+  unsigned const char e = swbar((__UINTPTR_TYPE__) &foo);
+  xyzzy(e);
+  unsigned const char f = ifbar((__UINTPTR_TYPE__) &foo);
+  xyzzy(f);
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
new file mode 100644
index 000000000000..9aaa6e463bc6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
@@ -0,0 +1,54 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+
+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 constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
+  xyzzy(c);
+  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
+  xyzzy(d);
+  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
+  xyzzy(e);
+  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
+  xyzzy(f);
+}
-- 
2.30.2

Re: [PATCH v4]: testcases for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

[Hans-Peter Nilsson]

Oops, I managed to send a version that only added a comment,
but still had a dg-do run.  Anyway, here's v4: actually
change the "dg-do run", not just adding a comment.  Sending
as a self-contained fresh patch for the benefit of
aforementioned CI.  See v2 and v3 for more.  Sorry!

Ok to commit?

-- >8 --

Test-cases, with constexpr-reinterpret3.C dg-ice:ing the PR c++/113545 bug.

Regarding the request in the comment, A dg-do run when there's an ICE
will cause some CI's to signal an error for the run being "UNRESOLVED"
(compilation failed to produce executable).  Note that dejagnu (1.6.3)
itself doesn't consider this an error.

gcc/testsuite:
	PR c++/113545
	* g++.dg/cpp1y/constexpr-reinterpret3.C,
	g++.dg/cpp1y/constexpr-reinterpret4.C: New tests.
---
 .../g++.dg/cpp1y/constexpr-reinterpret3.C     | 56 +++++++++++++++++++
 .../g++.dg/cpp1y/constexpr-reinterpret4.C     | 54 ++++++++++++++++++
 2 files changed, 110 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C

diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
new file mode 100644
index 000000000000..51feb2e558e7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
@@ -0,0 +1,56 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+// Please change the above "dg-do compile" to "dg-do run" when the ICE is resolved.
+// { dg-ice "PR112545 - constexpr function with switch called for reinterpret_cast" }
+
+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));
+  xyzzy(d);
+  unsigned const char e = swbar((__UINTPTR_TYPE__) &foo);
+  xyzzy(e);
+  unsigned const char f = ifbar((__UINTPTR_TYPE__) &foo);
+  xyzzy(f);
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
new file mode 100644
index 000000000000..9aaa6e463bc6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
@@ -0,0 +1,54 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+
+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 constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
+  xyzzy(c);
+  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
+  xyzzy(d);
+  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
+  xyzzy(e);
+  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
+  xyzzy(f);
+}
-- 
2.30.2


> From: Hans-Peter Nilsson <hp@axis.com>
> CC: <polacek@redhat.com>, <gcc-patches@gcc.gnu.org>
> Content-Type: text/plain; charset="iso-8859-1"
> Date: Fri, 9 Feb 2024 16:30:43 +0100
> 
> Bah.  Linaro's CI didn't like that there were UNRESOLVEDs
> due to this patch.  Running it "as usual" didn't show
> anything suspicious.  Sure, there were "# of unresolved
> testcases 3" in the summary (see v2), but no error or other
> special message from dejagnu.  Perhaps there could be a way
> to have dg-ice automatically downgrade a dg-do run that
> failed due to the ICE to a dg-do compile (or just not emit
> the UNRESOLVED note), but pragmatically, this is a rare
> enough case to not bother.  It looks like these were the
> only UNRESOLVEDs in that CI run, so I'm just going to make a
> mental note and move on.
> 
> For more comments, please see v2 of this patch.
> 
> v3: Change dg-do run to dg-do compile to avoid an UNRESOLVED.
> Tested as with v2.  Ok to commit?
> 
> -- >8 --
> 
> Test-cases, with constexpr-reinterpret3.C dg-ice:ing the PR c++/113545 bug.
> 
> Regarding the request in the comment, a dg-do run when there's an ICE
> will cause some CI's to signal an error for the run being "UNRESOLVED"
> (compilation failed to produce executable).  Note that dejagnu (1.6.3)
> itself doesn't consider this an error.
> 
> gcc/testsuite:
> 	PR c++/113545
> 	* g++.dg/cpp1y/constexpr-reinterpret3.C,
> 	g++.dg/cpp1y/constexpr-reinterpret4.C: New tests.
> ---
>  .../g++.dg/cpp1y/constexpr-reinterpret3.C     | 56 +++++++++++++++++++
>  .../g++.dg/cpp1y/constexpr-reinterpret4.C     | 54 ++++++++++++++++++
>  2 files changed, 110 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
> 
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
> new file mode 100644
> index 000000000000..6c396bff72b6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
> @@ -0,0 +1,56 @@
> +// PR c++/113545
> +// { dg-do run { target c++14 } }
> +// Please change the above "dg-do compile" to "dg-do run" when the ICE is resolved.
> +// { dg-ice "PR112545 - constexpr function with switch called for reinterpret_cast" }
> +
> +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));
> +  xyzzy(d);
> +  unsigned const char e = swbar((__UINTPTR_TYPE__) &foo);
> +  xyzzy(e);
> +  unsigned const char f = ifbar((__UINTPTR_TYPE__) &foo);
> +  xyzzy(f);
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
> new file mode 100644
> index 000000000000..9aaa6e463bc6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
> @@ -0,0 +1,54 @@
> +// PR c++/113545
> +// { dg-do compile { target c++14 } }
> +
> +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 constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
> +  xyzzy(c);
> +  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
> +  xyzzy(d);
> +  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
> +  xyzzy(e);
> +  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
> +  xyzzy(f);
> +}
> -- 
> 2.30.2
> 

Re: [PATCH v4]: testcases for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

[Jason Merrill]

On 2/9/24 11:02, Hans-Peter Nilsson wrote:
> Oops, I managed to send a version that only added a comment,
> but still had a dg-do run.  Anyway, here's v4: actually
> change the "dg-do run", not just adding a comment.  Sending
> as a self-contained fresh patch for the benefit of
> aforementioned CI.  See v2 and v3 for more.  Sorry!
> 
> Ok to commit?

OK.

> -- >8 --
> 
> Test-cases, with constexpr-reinterpret3.C dg-ice:ing the PR c++/113545 bug.
> 
> Regarding the request in the comment, A dg-do run when there's an ICE
> will cause some CI's to signal an error for the run being "UNRESOLVED"
> (compilation failed to produce executable).  Note that dejagnu (1.6.3)
> itself doesn't consider this an error.
> 
> gcc/testsuite:
> 	PR c++/113545
> 	* g++.dg/cpp1y/constexpr-reinterpret3.C,
> 	g++.dg/cpp1y/constexpr-reinterpret4.C: New tests.
> ---
>   .../g++.dg/cpp1y/constexpr-reinterpret3.C     | 56 +++++++++++++++++++
>   .../g++.dg/cpp1y/constexpr-reinterpret4.C     | 54 ++++++++++++++++++
>   2 files changed, 110 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
> 
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
> new file mode 100644
> index 000000000000..51feb2e558e7
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
> @@ -0,0 +1,56 @@
> +// PR c++/113545
> +// { dg-do compile { target c++14 } }
> +// Please change the above "dg-do compile" to "dg-do run" when the ICE is resolved.
> +// { dg-ice "PR112545 - constexpr function with switch called for reinterpret_cast" }
> +
> +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));
> +  xyzzy(d);
> +  unsigned const char e = swbar((__UINTPTR_TYPE__) &foo);
> +  xyzzy(e);
> +  unsigned const char f = ifbar((__UINTPTR_TYPE__) &foo);
> +  xyzzy(f);
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
> new file mode 100644
> index 000000000000..9aaa6e463bc6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
> @@ -0,0 +1,54 @@
> +// PR c++/113545
> +// { dg-do compile { target c++14 } }
> +
> +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 constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
> +  xyzzy(c);
> +  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>(&foo)); // { dg-error "conversion from pointer type" }
> +  xyzzy(d);
> +  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
> +  xyzzy(e);
> +  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) &foo); // { dg-error "conversion from pointer type" }
> +  xyzzy(f);
> +}
1