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

[Jason Merrill <jason at redhat dot com>]

On 12/12/18 5:30 PM, Jakub Jelinek wrote:
> Hi!
>
> The following patch fixes __builtin_is_constant_evaluated and
> __builtin_constant_p handling during static_assert evaluation.
> finish_static_assert calls fold_non_dependent_expr and complains if the
> result is not a constant expression, instead of requiring a constant
> expression, which causes __builtin_is_constant_evaluated () during the
> evaluation to be not considered as constant expression and
> __builtin_constant_p calls too if they appear in constexpr functions.
>
> The patch makes sure that manifestly_const_eval is true while evaluating
> the expression and also makes sure to fold __builtin_constant_p when that is
> true even when in constexpr functions.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2018-12-12  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR c++/86524
> 	PR c++/88446
> 	* cp-tree.h (fold_non_dependent_expr): Add manifestly_const_eval
> 	argument.
> 	* constexpr.c (cxx_eval_builtin_function_call): Evaluate
> 	__builtin_constant_p if ctx->manifestly_const_eval even in constexpr
> 	functions.  For arguments to builtins, if ctx->manifestly_const_eval
> 	try to first evaluate arguments with it, but if that doesn't result
> 	in a constant expression, retry without it.  Fix comment typo.
> 	(fold_non_dependent_expr): Add manifestly_const_eval argument, pass
> 	it through to cxx_eval_outermost_constant_expr and
> 	maybe_constant_value.
> 	* semantics.c (finish_static_assert): Call fold_non_dependent_expr
> 	with true as manifestly_const_eval.
>
> 	* g++.dg/cpp1y/constexpr-86524.C: New test.
> 	* g++.dg/cpp2a/is-constant-evaluated4.C: New test.
> 	* g++.dg/cpp2a/is-constant-evaluated5.C: New test.
> 	* g++.dg/cpp2a/is-constant-evaluated6.C: New test.
>
> --- gcc/cp/cp-tree.h.jj	2018-12-12 09:32:27.408535853 +0100
> +++ gcc/cp/cp-tree.h	2018-12-12 11:07:24.250459779 +0100
> @@ -7665,7 +7665,9 @@ extern tree cxx_constant_value			(tree,
>   extern tree cxx_constant_init			(tree, tree = NULL_TREE);
>   extern tree maybe_constant_value		(tree, tree = NULL_TREE, bool = false);
>   extern tree maybe_constant_init			(tree, tree = NULL_TREE, bool = false);
> -extern tree fold_non_dependent_expr		(tree, tsubst_flags_t = tf_warning_or_error);
> +extern tree fold_non_dependent_expr		(tree,
> +						 tsubst_flags_t = tf_warning_or_error,
> +						 bool = false);
>   extern tree fold_simple				(tree);
>   extern bool is_sub_constant_expr                (tree);
>   extern bool reduced_constant_expression_p       (tree);
> --- gcc/cp/constexpr.c.jj	2018-12-12 09:34:17.531736075 +0100
> +++ gcc/cp/constexpr.c	2018-12-12 11:30:33.986756914 +0100
> @@ -1198,6 +1198,7 @@ cxx_eval_builtin_function_call (const co
>        in a constexpr function until we have values for the parameters.  */
>     if (bi_const_p
>         && ctx->quiet
> +      && !ctx->manifestly_const_eval

I think we want to replace the "quiet" check with manifestly_const_eval, 
rather than adding to it.

> @@ -1222,7 +1223,6 @@ cxx_eval_builtin_function_call (const co
>        return constant false for a non-constant argument.  */
>     constexpr_ctx new_ctx = *ctx;
>     new_ctx.quiet = true;
> -  bool dummy1 = false, dummy2 = false;
>     for (i = 0; i < nargs; ++i)
>       {
>         args[i] = CALL_EXPR_ARG (t, i);
> @@ -1231,10 +1231,23 @@ cxx_eval_builtin_function_call (const co
>   	 of the builtin, verify it here.  */
>         if (!builtin_valid_in_constant_expr_p (fun)
>   	  || potential_constant_expression (args[i]))
> -	args[i] = cxx_eval_constant_expression (&new_ctx, args[i], false,
> -						&dummy1, &dummy2);
> +	{
> +	  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)
> +	    {
> +	      new_ctx.manifestly_const_eval = false;
> +	      non_cst_p = false;
> +	      ovf_p = false;
> +	      a = cxx_eval_constant_expression (&new_ctx, args[i], false,
> +						&non_cst_p, &ovf_p);
> +	      new_ctx.manifestly_const_eval = true;
> +	    }

Why retry without manifestly_const_eval?  For static initialization we 
don't want to try constant evaluation again with 
__builtin_is_constant_evaluated false if it failed with it true.

Jason