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

[Jason Merrill <jason@redhat.com>]

On 4/23/24 08:52, Jakub Jelinek wrote:
> On Mon, Apr 15, 2024 at 02:19:36PM +0200, Jakub Jelinek wrote:
>> 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.
> 
> I've now tested that version and it passed bootstrap/regtest on x86_64-linux
> and i686-linux.  But as I said earlier, trying to tweak the patch further
> doesn't work on the constexpr-dtor{5,6}.C tests.

OK.

> 2024-04-23  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/111284
> 	* constexpr.cc (cxx_bind_parameters_in_call): For PARM_DECLs with
> 	TREE_ADDRESSABLE types use vc_glvalue rather than vc_prvalue for
> 	cxx_eval_constant_expression and if it doesn't have the same
> 	type as it should, cast the reference type to reference to type
> 	before convert_from_reference and instead of adjust_temp_type
> 	take address of the arg, cast to reference to type and then
> 	convert_from_reference.
> 	(cxx_eval_constant_expression) <case PARM_DECL>: For lval case
> 	on parameters with TREE_ADDRESSABLE types lookup result in
> 	ctx->globals if possible.  Otherwise if lookup in ctx->globals
> 	was successful for parameter with TREE_ADDRESSABLE type,
> 	recurse with vc_prvalue on the returned value.
> 
> 	* g++.dg/cpp1z/constexpr-111284.C: New test.
> 	* g++.dg/cpp1y/constexpr-lifetime7.C: Expect one error on a different
> 	line.
> 
> --- 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,18 @@ 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.  */
> +	  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);
>         /* Check we aren't dereferencing a null pointer when calling a non-static
>   	 member function, which is undefined behaviour.  */
>         if (i == 0 && DECL_OBJECT_MEMBER_FUNCTION_P (fun)
> @@ -1909,7 +1914,16 @@ 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 (arg)))
> +		{
> +		  arg = build_fold_addr_expr (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;
> @@ -7499,9 +7513,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))
> --- gcc/testsuite/g++.dg/cpp1z/constexpr-111284.C.jj	2024-03-07 16:27:48.113651999 +0100
> +++ gcc/testsuite/g++.dg/cpp1z/constexpr-111284.C	2024-03-07 16:26:49.565466606 +0100
> @@ -0,0 +1,19 @@
> +// PR c++/111284
> +// { dg-do compile { target c++17 } }
> +
> +struct S {
> +  S () = default;
> +  constexpr S (const S &) noexcept : s{this} {}
> +  constexpr S & operator= (const S &) noexcept { return *this; }
> +  constexpr bool foo () const noexcept { return s == this; }
> +  S *s = this;
> +};
> +
> +constexpr bool
> +bar (S x) noexcept
> +{
> +  return x.foo ();
> +}
> +
> +static_assert (bar (S {}), "");
> +static_assert ([] (S x) { return x.foo (); } (S {}), "");
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C.jj	2023-12-13 19:09:33.252657826 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-lifetime7.C	2024-03-07 19:49:49.342334090 +0100
> @@ -87,7 +87,7 @@ constexpr bool n1 = test_access<NonTrivi
>   constexpr bool n2 = test_modification<NonTrivial>();  // { dg-message "in .constexpr." "" { target c++20 } }
>   constexpr bool n3 = test_scope<NonTrivial>();         // { dg-message "in .constexpr." "" { target c++20 } }
>   constexpr bool n4 = test_destroy_temp<NonTrivial>();  // { dg-message "in .constexpr." "" { target c++20 } }
> -constexpr bool n5 = test_parameter(NonTrivial{});     // { dg-error "destroying" "" { target c++20 } }
> +constexpr bool n5 = test_parameter(NonTrivial{});     // { dg-message "in .constexpr." "" { target c++20 } }
>   constexpr bool n6 = test_bindings<NonTrivial>();
>   #endif
>   
> 
> 
> 	Jakub
>