> Date: Thu, 8 Feb 2024 10:44:31 -0500
> From: Marek Polacek <pola...@redhat.com>
> Cc: ja...@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 <pola...@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 <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.
> > 
> > > > 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

Reply via email to