Re: [C++ PATCH] Fix __builtin_{is_constant_evaluated,constant_p} handling in static_assert (PR c++/86524, PR c++/88446)

[Jakub Jelinek <jakub@redhat.com>]

On Thu, Dec 20, 2018 at 04:47:29PM -0500, Jason Merrill wrote:
> > So are you ok with what is in the patch below, i.e.
> >         {
> >           bool non_cst_p = false, ovf_p = false;
> >           tree a = cxx_eval_constant_expression (&new_ctx, args[i], false,
> >                                                  &non_cst_p, &ovf_p);
> >           if ((!non_cst_p && !ovf_p) || !ctx->manifestly_const_eval)
> >             args[i] = a;
> >         }
> > , or perhaps without the || !ctx->manifestly_const_eval?
> 
> I don't see how that makes a difference from what was there before; if the
> argument to cxx_eval_constant_expression is non-constant, it returns the
> argument unchanged.

If that is guaranteed, then it is ok to keep it as is I guess.
Will change it then.

> > So, if the
> > argument is a constant expression, fold to that, if it is not, just do
> > cp_fully_fold on it if it is __builtin_constant_p, otherwise nothing?
> 
> Hmm, cp_fully_fold probably also needs to add a manifestly_const_eval
> parameter to pass along to maybe_constant_value.

But if we need cp_fully_fold, doesn't that mean that the earlier
cxx_eval_constant_expression failed and thus the argument is not a constant
expression?  Should __builtin_is_constant_evaluated () evaluate to true
even if the argument is not a constant expression?
Say if there is
int v;
constexpr int foo (void)
{
  return __builtin_constant_p (v * (__builtin_is_constant_evaluated () ? 1 : 0));
}
Because v is not a constant expression,
v * (__builtin_is_constant_evaluated () ? 1 : 0) shouldn't be either.

cp_fully_fold does:
  /* FIXME cp_fold ought to be a superset of maybe_constant_value so we don't
     have to call both.  */
  if (cxx_dialect >= cxx11)
    {
      x = maybe_constant_value (x);
      /* Sometimes we are given a CONSTRUCTOR but the call above wraps it into
         a TARGET_EXPR; undo that here.  */
      if (TREE_CODE (x) == TARGET_EXPR)
        x = TARGET_EXPR_INITIAL (x);
      else if (TREE_CODE (x) == VIEW_CONVERT_EXPR
               && TREE_CODE (TREE_OPERAND (x, 0)) == CONSTRUCTOR
               && TREE_TYPE (TREE_OPERAND (x, 0)) == TREE_TYPE (x))
        x = TREE_OPERAND (x, 0);
    }
  return cp_fold_rvalue (x);
Is there a reason to call that maybe_constant_value at all when we've called
cxx_eval_constant_expression first?  Wouldn't cp_fold_rvalue (or
c_fully_fold with false as last argument) be sufficient there?

	Jakub