Re: [PATCH] c++: constantness of local var in constexpr fn [PR111703, PR112269]

[Jason Merrill <jason@redhat.com>]

On 11/1/23 11:07, Patrick Palka wrote:
> On Tue, 31 Oct 2023, Patrick Palka wrote:
> 
>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>> trunk?  Does it look OK for release branches as well for sake of PR111703?
>>
>> -- >8 --
>>
>> potential_constant_expression was incorrectly treating most local
>> variables from a constexpr function as (potentially) constant because it
>> wasn't considering the 'now' parameter.  This patch fixes this by
>> relaxing some var_in_maybe_constexpr_fn checks accordingly, which turns
>> out to partially fix two recently reported regressions:
>>
>> PR111703 is a regression caused by r11-550-gf65a3299a521a4 for
>> restricting constexpr evaluation during warning-dependent folding.
>> The mechanism is intended to restrict only constant evaluation of the
>> instantiated non-dependent expression, but it also ends up restricting
>> constant evaluation (as part of satisfaction) during instantiation of
>> the expression, in particular when resolving the ck_rvalue conversion of
>> the 'x' argument into a copy constructor call.
> 
> Oops, this analysis is inaccurate for this specific testcase (although
> the general idea is the same)...  We don't call fold_for_warn on 'f(x)'
> but rather on its 'x' argument that has been processed by
> convert_arguments into an IMPLICIT_CONV_EXPR.  And it's the
> instantiation of this IMPLICIT_CONV_EXPR that turns it into a copy
> constructor call.  There is no ck_rvalue conversion at all here since
> 'f' is a function pointer, not an actual function, and so ICSes don't
> get computed (IIUC).  If 'f' is changed to be an actual function then
> there's no issue since build_over_call doesn't perform argument
> conversions when in a template context and therefore doesn't call
> check_function_arguments on the converted arguments (from which the
> problematic fold_for_warn call occurs).

Sounds like a we want to adjust a call to a function pointer to compute 
conversions without performing them in template context?

In any case, the patch is OK.

>> This seems like a bug in
>> the mechanism[1], though I don't know if we want to refine the mechanism
>> or get rid of it completely since the original testcases which motivated
>> the mechanism are fixed more simply by r13-1225-gb00b95198e6720.  In any
>> case, this patch partially fixes this by making us correctly treat 'x'
>> and therefore 'f(x)' in the below testcase as non-constant, which
>> prevents the problematic warning-dependent folding from occurring at
>> all.  If this bug crops up again then I figure we could decide what to
>> do with the mechanism then.
>>
>> PR112269 is caused by r14-4796-g3e3d73ed5e85e7 for merging tsubst_copy
>> into tsubst_copy_and_build.  tsubst_copy used to exit early when 'args'
>> was empty, behavior which that commit deliberately didn't preserve.
>> This early exit masked the fact that COMPLEX_EXPR wasn't handled by
>> tsubst at all, and is a tree code that apparently we could see during
>> warning-dependent folding on some targets.  A complete fix is to add
>> handling for this tree code in tsubst_expr, but this patch should fix
>> the reported testsuite failures since the situations where COMPLEX_EXPR
>> crops up in <complex> turn out to not be constant expressions in the
>> first place after this patch.
>>
>> [1]: The mechanism incorrectly assumes that instantiation of the
>> non-dependent expression shouldn't induce any template instantiation
>> since ahead of time checking of the expression should've already induced
>> whatever template instantiation was needed, but in this case although
>> overload resolution was performed ahead of time, a ck_rvalue conversion
>> gets resolved to a copy constructor call only at instantiation time.
>>
>> 	PR c++/111703
>>
>> gcc/cp/ChangeLog:
>>
>> 	* constexpr.cc (potential_constant_expression_1) <case VAR_DECL>:
>> 	Only consider var_in_maybe_constexpr_fn if 'now' is false.
>> 	<case INDIRECT_REF>: Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* g++.dg/cpp2a/concepts-fn8.C: New test.
>> ---
>>   gcc/cp/constexpr.cc                       |  4 ++--
>>   gcc/testsuite/g++.dg/cpp2a/concepts-fn8.C | 24 +++++++++++++++++++++++
>>   2 files changed, 26 insertions(+), 2 deletions(-)
>>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-fn8.C
>>
>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
>> index c05760e6789..8a6b210144a 100644
>> --- a/gcc/cp/constexpr.cc
>> +++ b/gcc/cp/constexpr.cc
>> @@ -9623,7 +9623,7 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
>>   	  return RECUR (DECL_VALUE_EXPR (t), rval);
>>   	}
>>         if (want_rval
>> -	  && !var_in_maybe_constexpr_fn (t)
>> +	  && (now || !var_in_maybe_constexpr_fn (t))
>>   	  && !type_dependent_expression_p (t)
>>   	  && !decl_maybe_constant_var_p (t)
>>   	  && (strict
>> @@ -9737,7 +9737,7 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
>>           STRIP_NOPS (x);
>>           if (is_this_parameter (x) && !is_capture_proxy (x))
>>   	  {
>> -	    if (!var_in_maybe_constexpr_fn (x))
>> +	    if (now || !var_in_maybe_constexpr_fn (x))
>>   	      {
>>   		if (flags & tf_error)
>>   		  constexpr_error (loc, fundef_p, "use of %<this%> in a "
>> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-fn8.C b/gcc/testsuite/g++.dg/cpp2a/concepts-fn8.C
>> new file mode 100644
>> index 00000000000..3f63a5b28d7
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-fn8.C
>> @@ -0,0 +1,24 @@
>> +// PR c++/111703
>> +// { dg-do compile { target c++20 } }
>> +
>> +template<class T>
>> +constexpr bool always_true() { return true; }
>> +
>> +struct P {
>> +  P() = default;
>> +
>> +  template<class T>
>> +    requires (always_true<T>()) // { dg-bogus "used before its definition" }
>> +  constexpr P(const T&) { }
>> +
>> +  int n, m;
>> +};
>> +
>> +void (*f)(P);
>> +
>> +template<class T>
>> +constexpr bool g() {
>> +  P x;
>> +  f(x); // { dg-bogus "from here" }
>> +  return true;
>> +}
>> -- 
>> 2.42.0.526.g3130c155df
>>
>>
>