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).
> 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
>
>
