Re: [PATCH] c++: Fix constexpr evaluation of parameters passed by invisible reference [PR111284]

[Jakub Jelinek <jakub@redhat.com>]

On Fri, Apr 12, 2024 at 03:05:26PM -0400, Jason Merrill wrote:
> > --- gcc/cp/constexpr.cc.jj	2024-02-13 10:29:57.979155641 +0100
> > +++ gcc/cp/constexpr.cc	2024-03-07 19:35:01.032412221 +0100
> > @@ -1877,13 +1877,21 @@ cxx_bind_parameters_in_call (const const
> >   	  x = build_address (x);
> >   	}
> >         if (TREE_ADDRESSABLE (type))
> > -	/* Undo convert_for_arg_passing work here.  */
> > -	x = convert_from_reference (x);
> > -      /* Normally we would strip a TARGET_EXPR in an initialization context
> > -	 such as this, but here we do the elision differently: we keep the
> > -	 TARGET_EXPR, and use its CONSTRUCTOR as the value of the parm.  */
> > -      arg = cxx_eval_constant_expression (ctx, x, vc_prvalue,
> > -					  non_constant_p, overflow_p);
> > +	{
> > +	  /* Undo convert_for_arg_passing work here.  */
> > +	  if (TYPE_REF_P (TREE_TYPE (x))
> > +	      && !same_type_p (type, TREE_TYPE (TREE_TYPE (x))))
> > +	    x = cp_fold_convert (build_reference_type (type), x);
> > +	  x = convert_from_reference (x);
> > +	  arg = cxx_eval_constant_expression (ctx, x, vc_glvalue,
> > +					      non_constant_p, overflow_p);
> > +	}
> > +      else
> > +	/* Normally we would strip a TARGET_EXPR in an initialization context
> > +	   such as this, but here we do the elision differently: we keep the
> > +	   TARGET_EXPR, and use its CONSTRUCTOR as the value of the parm.  */
> > +	arg = cxx_eval_constant_expression (ctx, x, vc_prvalue,
> > +					    non_constant_p, overflow_p);
> 
> It seems simpler to move the convert_for_reference after the
> cxx_eval_constant_expression rather than duplicate the call to
> cxx_eval_constant_expression.

They weren't the same, one was trying to evaluate the convert_from_reference
with vc_glvalue, the other evaluates it without that and with vc_prvalue.
Now, I guess the
+     /* Undo convert_for_arg_passing work here.  */
+     if (TYPE_REF_P (TREE_TYPE (x))
+         && !same_type_p (type, TREE_TYPE (TREE_TYPE (x))))
+       x = cp_fold_convert (build_reference_type (type), x);
part could be thrown away, given the other !same_type_p check (that one is
needed because adjust_temp_type can't deal with that), at least
when I remove that
GXX_TESTSUITE_STDS=98,11,14,17,20,23,26 make check-g++ RUNTESTFLAGS="dg.exp='constexpr-dtor* pr114426.C constexpr-111284.C constexpr-lifetime7.C'"
still passes.  But if I try to remove even more, like in the patch below,
then I see
FAIL: g++.dg/cpp2a/constexpr-dtor5.C  -std=c++20 (test for excess errors)
FAIL: g++.dg/cpp2a/constexpr-dtor5.C  -std=c++23 (test for excess errors)
FAIL: g++.dg/cpp2a/constexpr-dtor5.C  -std=c++26 (test for excess errors)
FAIL: g++.dg/cpp2a/constexpr-dtor7.C  -std=c++20  (test for errors, line 6)
FAIL: g++.dg/cpp2a/constexpr-dtor7.C  -std=c++20 (test for excess errors)
FAIL: g++.dg/cpp2a/constexpr-dtor7.C  -std=c++23  (test for errors, line 6)
FAIL: g++.dg/cpp2a/constexpr-dtor7.C  -std=c++23 (test for excess errors)
FAIL: g++.dg/cpp2a/constexpr-dtor7.C  -std=c++26  (test for errors, line 6)
FAIL: g++.dg/cpp2a/constexpr-dtor7.C  -std=c++26 (test for excess errors)
regressions.
E.g.
.../g++.dg/cpp2a/constexpr-dtor5.C:30:20: error: non-constant condition for static assertion
.../g++.dg/cpp2a/constexpr-dtor5.C:13:7: error: modification of '<anonymous>' from outside current evaluation is not a constant expression
.../g++.dg/cpp2a/constexpr-dtor5.C:31:20: error: non-constant condition for static assertion
.../g++.dg/cpp2a/constexpr-dtor5.C:13:7: error: modification of '<anonymous>' from outside current evaluation is not a constant expression
.../g++.dg/cpp2a/constexpr-dtor5.C:32:20: error: non-constant condition for static assertion
.../g++.dg/cpp2a/constexpr-dtor5.C:13:7: error: modification of '<anonymous>' from outside current evaluation is not a constant expression
.../g++.dg/cpp2a/constexpr-dtor5.C:13:7: error: modification of '<anonymous>' from outside current evaluation is not a constant expression
.../g++.dg/cpp2a/constexpr-dtor5.C:13:7: error: modification of '<anonymous>' from outside current evaluation is not a constant expression
.../g++.dg/cpp2a/constexpr-dtor5.C:13:7: error: modification of '<anonymous>' from outside current evaluation is not a constant expression

--- gcc/cp/constexpr.cc.jj	2024-02-13 10:29:57.979155641 +0100
+++ gcc/cp/constexpr.cc	2024-03-07 19:35:01.032412221 +0100
@@ -1871,9 +1871,6 @@ cxx_bind_parameters_in_call (const const
 	  x = ctx->object;
 	  x = build_address (x);
 	}
-      if (TREE_ADDRESSABLE (type))
-	/* Undo convert_for_arg_passing work here.  */
-	x = convert_from_reference (x);
       /* Normally we would strip a TARGET_EXPR in an initialization context
 	 such as this, but here we do the elision differently: we keep the
 	 TARGET_EXPR, and use its CONSTRUCTOR as the value of the parm.  */
@@ -1904,7 +1901,13 @@ cxx_bind_parameters_in_call (const const
 	{
 	  /* Make sure the binding has the same type as the parm.  But
 	     only for constant args.  */
-	  if (!TYPE_REF_P (type))
+	  if (TREE_ADDRESSABLE (type))
+	    {
+	      if (!same_type_p (type, TREE_TYPE (TREE_TYPE (arg))))
+		arg = cp_fold_convert (build_reference_type (type), arg);
+	      arg = convert_from_reference (arg);
+	    }
+	  else if (!TYPE_REF_P (type))
 	    arg = adjust_temp_type (type, arg);
 	  if (!TREE_CONSTANT (arg))
 	    *non_constant_args = true;
@@ -7494,9 +7497,19 @@ cxx_eval_constant_expression (const cons
 
     case PARM_DECL:
       if (lval && !TYPE_REF_P (TREE_TYPE (t)))
-	/* glvalue use.  */;
+	{
+	  /* glvalue use.  */
+	  if (TREE_ADDRESSABLE (TREE_TYPE (t)))
+	    if (tree v = ctx->global->get_value (t))
+	      r = v;
+	}
       else if (tree v = ctx->global->get_value (t))
-	r = v;
+	{
+	  r = v;
+	  if (TREE_ADDRESSABLE (TREE_TYPE (t)))
+	    r = cxx_eval_constant_expression (ctx, r, vc_prvalue,
+					      non_constant_p, overflow_p);
+	}
       else if (lval)
 	/* Defer in case this is only used for its type.  */;
       else if (ctx->global->is_outside_lifetime (t))

	Jakub