On Mon, Feb 18, 2019 at 04:04:15PM -1000, Jason Merrill wrote:
> > --- gcc/cp/constexpr.c.jj   2019-02-17 17:09:47.113351897 +0100
> > +++ gcc/cp/constexpr.c      2019-02-18 19:34:57.995136395 +0100
> > @@ -1269,6 +1301,49 @@ cxx_eval_builtin_function_call (const co
> >         return t;
> >       }
> > +  if (fndecl_built_in_p (fun, BUILT_IN_NORMAL))
> > +    switch (DECL_FUNCTION_CODE (fun))
> > +      {
> > +      case BUILT_IN_ADD_OVERFLOW:
> > +      case BUILT_IN_SADD_OVERFLOW:
> > +      case BUILT_IN_SADDL_OVERFLOW:
> > +      case BUILT_IN_SADDLL_OVERFLOW:
> > +      case BUILT_IN_UADD_OVERFLOW:
> > +      case BUILT_IN_UADDL_OVERFLOW:
> > +      case BUILT_IN_UADDLL_OVERFLOW:
> > +      case BUILT_IN_SUB_OVERFLOW:
> > +      case BUILT_IN_SSUB_OVERFLOW:
> > +      case BUILT_IN_SSUBL_OVERFLOW:
> > +      case BUILT_IN_SSUBLL_OVERFLOW:
> > +      case BUILT_IN_USUB_OVERFLOW:
> > +      case BUILT_IN_USUBL_OVERFLOW:
> > +      case BUILT_IN_USUBLL_OVERFLOW:
> > +      case BUILT_IN_MUL_OVERFLOW:
> > +      case BUILT_IN_SMUL_OVERFLOW:
> > +      case BUILT_IN_SMULL_OVERFLOW:
> > +      case BUILT_IN_SMULLL_OVERFLOW:
> > +      case BUILT_IN_UMUL_OVERFLOW:
> > +      case BUILT_IN_UMULL_OVERFLOW:
> > +      case BUILT_IN_UMULLL_OVERFLOW:
> > +   /* These builtins will fold into
> > +      (cast)
> > +        ((something = __real__ SAVE_EXPR <.???_OVERFLOW (cst1, cst2)>),
> > +         __imag__ SAVE_EXPR <.???_OVERFLOW (cst1, cst2)>)
> > +      which fails is_constant_expression.  */
> > +   if (TREE_CODE (args[0]) != INTEGER_CST
> > +       || TREE_CODE (args[1]) != INTEGER_CST
> > +       || !potential_constant_expression (args[2]))
> > +     {
> > +       if (!*non_constant_p && !ctx->quiet)
> > +         error ("%q+E is not a constant expression", new_call);
> > +       *non_constant_p = true;
> > +       return t;
> > +     }
> > +   return cxx_eval_constant_expression (&new_ctx, new_call, lval,
> > +                                        non_constant_p, overflow_p);
> > +      default:
> > +   break;
> > +      }
> 
> What is this for?  Won't this recursive cxx_eval_constant_expression come
> back to this function again?  If the expression is constant, shouldn't it
> have been folded by fold_builtin_call_array?

This is for the constexpr-arith-overflow.C testcase.
The arguments are INTEGER_CST, INTEGER_CST and ADDR_EXPR of a VAR_DECL or
PARM_DECL, and fold_builtin_call_array returns new_call:
(z = REALPART_EXPR <SAVE_EXPR <.ADD_OVERFLOW (0, 0)>>;, (bool) IMAGPART_EXPR 
<SAVE_EXPR <.ADD_OVERFLOW (0, 0)>>;);
where this doesn't pass is_constant_expression because of the z store.
cxx_eval_constant_expression is able to evaluate this, as
z = 0;
false;
in this case.
I guess builtins.c folding could be improved and simplify it to
(z = 0; (bool) false;);
but that still doesn't pass is_constant_expression check.
For C++14 it passes potential_constant_expression though, and that
is what I've used for these builtins in the first iteration, but
the testcase happened to pass even for C++11 and
potential_constant_expression is false here.  Though, perhaps we are going
too far for C++11 here and should reject it, after all, people have the
possibility to use __builtin_*_overflow_p now which should be usable even in
C++11.  The reason why it passed with C++11 is that when parsing we saw
a __builtin_add_overflow (0, 0, &z) call and potential_constant_expression
said it is ok, then folded it into that
(z = REALPART_EXPR <SAVE_EXPR <.ADD_OVERFLOW (0, 0)>>;, (bool) IMAGPART_EXPR 
<SAVE_EXPR <.ADD_OVERFLOW (0, 0)>>;);
which is not potential_constant_expression, but nothing called it again
and cxx_eval_constant_expression can handle it.

> > @@ -1358,6 +1433,9 @@ cxx_bind_parameters_in_call (const const
> >       x = ctx->object;
> >       x = build_address (x);
> >     }
> > +      if (TREE_ADDRESSABLE (type) && TYPE_REF_P (TREE_TYPE (x)))
> > +   /* Undo convert_for_arg_passing work here.  */
> > +   x = build_fold_indirect_ref_loc (EXPR_LOCATION (x), x);
> 
> Not convert_from_reference?

Will change.

> > @@ -4036,6 +4113,10 @@ label_matches (const constexpr_ctx *ctx,
> >     }
> >         break;
> > +    case BREAK_STMT:
> > +    case CONTINUE_STMT:
> > +      break;
> > +
> 
> Let's add a comment that these are handled directly in cxx_eval_loop_expr.

Ok, will do.

        Jakub

Reply via email to