On 10/10/23 13:20, Marek Polacek wrote:
Thanks for looking into this. It's kept me occupied for quite a while.
Thanks, nice work.
On Tue, Aug 29, 2023 at 03:26:46PM -0400, Jason Merrill wrote:
On 8/23/23 15:49, Marek Polacek wrote:
+struct A {
+ int x;
+ int y = id(x);
+};
+
+template<class T>
+constexpr int k(int) { // k<int> is not an immediate function because
A(42) is a
+ return A(42).y; // constant expression and thus not
immediate-escalating
+}
Needs use(s) of k to test the comment.
True, and that revealed what I think is a bug in the standard.
In the test I'm saying:
// ??? [expr.const]#example-9 says:
// k<int> is not an immediate function because A(42) is a
// constant expression and thus not immediate-escalating
// But I think the call to id(x) is *not* a constant expression and thus
// it is an immediate-escalating expression. Therefore k<int> *is*
// an immediate function. So we get the error below. clang++ agrees.
id(x) is not a constant expression because x isn't constant.
Not when considering id(x) by itself, but in the evaluation of A(42),
the member x has just been initialized to constant 42. And A(42) is
constant-evaluated because "An aggregate initialization is an immediate
invocation if it evaluates a default member initializer that has a
subexpression that is an immediate-escalating expression."
I assume clang doesn't handle this passage properly yet because it was
added during core review of the paper, for parity between aggregate
initialization and constructor escalation.
This can be a follow-up patch.
So. I think we want to refrain from instantiating things early
given how many problems that caused. On the other hand, stashing
all the immediate-escalating decls into immediate_escalating_decls
and walking their bodies isn't going to be cheap. I've checked
how big the vectors can get, but our testsuite isn't the best litmus
test because it's mostly smallish testcases without many #includes.
The worst offender is uninit-pr105562.C with
(gdb) p immediate_escalating_decls->length()
$2 = 2204
(gdb) p deferred_escalating_exprs->length()
$3 = 501
Compiling uninit-pr105562.C with g++13 and g++14 with this patch:
real 7.51 real 7.67
user 7.32 user 7.49
sys 0.15 sys 0.14
I've made sure not to walk the same bodies twice. But there's room
for further optimization; I suppose we could escalate instantiated
functions right away rather than putting them into
immediate_escalating_decls and waiting till later.
Absolutely; if we see a call to a known consteval function, we should
escalate...immediately. As the patch seems to do already?
I'm not certain
if I can just look at DECL_TEMPLATE_INSTANTIATED.
I'm not sure what you mean, but a constexpr function being instantiated
doesn't necessarily imply that everything it calls has been
instantiated, so we might not know yet if it needs to escalate.
I suppose some
functions cannot possibly be promoted because they don't contain
any CALL_EXPRs. So we may be able to rule them out while doing
cp_fold_r early.
Yes. Or, the only immediate-escalating functions referenced have
already been checked.
We can also do some escalation during constexpr evaluation: all the
functions involved need to be instantiated for the evaluation, and if we
encounter an immediate-escalating expression while evaluating a call to
an immediate-escalating function, we can promote it then. Though we
can't necessarily mark it as not needing promotion, as there might be
i-e exprs in branches that the particular evaluation doesn't take.
If a function is trivial, can it ever be promoted?
This might be a question for CWG. clang thinks so:
struct A {
consteval A() = default;
consteval A(const A&) = default;
int i;
};
struct B : A { }; // B constructors promote to consteval
B b; // ok, constant-initialization
B b2(b); // error, immediate invocation isn't constant
And so on.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
This patch implements P2564, described at <wg21.link/p2564>, whereby
certain functions are promoted to consteval. For example:
consteval int id(int i) { return i; }
template <typename T>
constexpr int f(T t)
{
return t + id(t); // id causes f<int> to be promoted to consteval
}
void g(int i)
{
f (3);
}
now compiles. Previously the code was ill-formed: we would complain
that 't' in 'f' is not a constant expression. Since 'f' is now
consteval, it means that the call to id(t) is in an immediate context,
so doesn't have to produce a constant -- this is how we allow consteval
functions composition. But making 'f<int>' consteval also means that
the call to 'f' in 'g' must yield a constant; failure to do so results
in an error. I made the effort to have cc1plus explain to us what's
going on. For example, calling f(i) produces this neat diagnostic:
w.C:11:11: error: call to consteval function 'f<int>(i)' is not a constant
expression
11 | f (i);
| ~~^~~
w.C:11:11: error: 'i' is not a constant expression
w.C:6:22: note: 'constexpr int f(T) [with T = int]' was promoted to an
immediate function because its body contains an immediate-escalating expression
'id(t)'
6 | return t + id(t); // id causes f<int> to be promoted to
consteval
| ~~^~~
which hopefully makes it clear what's going on.
Implementing this proposal has been tricky. One problem was delayed
instantiation: instantiating a function can set off a domino effect
where one call promotes a function to consteval but that then means
that another function should also be promoted, etc.
In v1, I addressed the delayed instantiation problem by instantiating
trees early, so that we can escalate functions right away. That caused
a number of problems, and in certain cases, like consteval-prop3.C, it
can't work, because we need to wait till EOF to see the definition of
the function anyway.
Indeed. I briefly thought that maybe we could just poison consteval
functions (declared or escalated) so that the back-end complains if we
try to emit a reference to them, but that wouldn't catch optimized-away
references.
Overeager instantiation tends to cause diagnostic
problems too.
e.g. constexpr-inst1.C. Though I notice that clang and EDG do the eager
instantiation for that testcase even in C++11 mode.
Relatedly, for
template <class T> consteval T id (T t) { return t; }
template <class T> constexpr T f(T t);
void g(int i) { f(42); }
template <class T> constexpr T f(T t) { return id(t); }
clang doesn't try to constant-evaluate f(42) because f wasn't defined
yet at that point (as required by
https://eel.is/c++draft/expr.const#5.4), just gives an error.
And if f is explicitly declared consteval, we also reject it, so it
seems surprising to be more permissive with promoted consteval.
This also doesn't need to be resolved now.
Therefore, we have to perform the escalation before gimplifying, but
after instantiate_pending_templates. That's not easy because we have
no way to walk all the trees. In the v2 patch, I use two vectors: one
to store function decls that may become consteval and another to > remember
references to immediate-escalating functions. Unfortunately
the latter must also stash functions that call immediate-escalating
functions. Consider:
int g(int i)
{
f<int>(i); // f is immediate-escalating
}
where g itself is not immediate-escalating, but we have to make sure
that if f gets promoted to consteval, we give an error.
I'm not sure you need the former list: if you immediately promote when
you see a known immediate invocation, an i-e fn you might want to
promote will be one that calls other i-e fns, so it'll also be on the
second list.
A new option, -fno-immediate-escalation, is provided to suppress
escalating functions.
v2 also adds a new flag, DECL_ESCALATED_P, so that we don't escalate
a function multiple times, and so that we can distinguish between
explicitly consteval functions and functions that have been promoted
to consteval.
That name suggests to me that it's been promoted, not just that we've
checked it for promotion; maybe
DECL_ESCALATION_{CHECKED,RESOLVED,KNOWN}_P?
@@ -55,6 +64,8 @@ enum fold_flags {
ff_mce_false = 1 << 1,
/* Whether we're being called from cp_fold_immediate. */
ff_fold_immediate = 1 << 2,
+ /* Whether we're escalating immediate-escalating functions. */
+ ff_escalating = 1 << 3,
If we always escalate when we see a known immediate invocation, this
means recurse. And maybe we could use at_eof for that?
};
using fold_flags_t = int;
@@ -428,6 +439,176 @@ lvalue_has_side_effects (tree e)
return TREE_SIDE_EFFECTS (e);
}
+/* Return true if FN is an immediate-escalating function. */
+
+static bool
+immediate_escalating_function_p (tree fn)
+{
+ if (!fn || !flag_immediate_escalation)
+ return false;
+
+ gcc_checking_assert (TREE_CODE (fn) == FUNCTION_DECL);
Maybe check DECL_IMMEDIATE_FUNCTION_P early, rather than multiple times
below...
+ /* An immediate-escalating function is
+ -- the call operator of a lambda that is not declared with the consteval
+ specifier */
+ if (LAMBDA_FUNCTION_P (fn) && !DECL_IMMEDIATE_FUNCTION_P (fn))
+ return true;
+ /* -- a defaulted special member function that is not declared with the
+ consteval specifier */
+ special_function_kind sfk = special_memfn_p (fn);
+ if (sfk != sfk_none
+ && DECL_DEFAULTED_FN (fn)
+ && !DECL_IMMEDIATE_FUNCTION_P (fn))
+ return true;
+ /* -- a function that results from the instantiation of a templated entity
+ defined with the constexpr specifier. */
+ return is_instantiation_of_constexpr (fn);
+}
+
+/* Promote FN to an immediate function, including its clones, if it is
+ an immediate-escalating function. Return true if we did promote;
+ false otherwise. */
+
+static bool
+maybe_promote_function_to_consteval (tree fn)
+{
+ if (fn
+ && !DECL_IMMEDIATE_FUNCTION_P (fn)
...and in all the callers?
+ && immediate_escalating_function_p (fn))
+ {
+ SET_DECL_IMMEDIATE_FUNCTION_P (fn);
+ DECL_ESCALATED_P (fn) = true;
+ tree clone;
+ FOR_EACH_CLONE (clone, fn)
+ {
+ SET_DECL_IMMEDIATE_FUNCTION_P (clone);
+ DECL_ESCALATED_P (clone) = true;
+ }
+ return true;
+ }
+
+ return false;
+}
+
+/* Remember that the current function declaration contains a call to
+ a function that might be promoted to consteval later. */
+
+static void
+maybe_store_cfun_for_late_checking ()
+{
+ if (!current_function_decl
+ || !flag_immediate_escalation
+ || immediate_escalating_function_p (current_function_decl))
+ return;
+
+ if (deferred_escalating_exprs
+ /* Don't put duplicates into the vec so that we don't walk
+ a function multiple times. Most likely the last one
+ added will be the same. */
+ && (deferred_escalating_exprs->last () == current_function_decl
+ || deferred_escalating_exprs->contains (current_function_decl)))
Doing ->contains on a vec is slow, better to use a hash_set.
+ return;
+
+ vec_safe_push (deferred_escalating_exprs, current_function_decl);
+}
+
+/* Find an immediate-escalating expression or conversion in *TP.
+ If DATA_ is non-null, this function will promote function to
+ consteval as it goes; otherwise, we're just looking for what
+ made a function consteval, for diagnostic purposes. This
+ function assumes that *TP was instantiated. */
+
+static tree
+find_escalating_expr_r (tree *tp, int *walk_subtrees, void *data)
Can this merge with cp_fold_immediate_r? They seem to be doing
essentially the same thing.
@@ -485,6 +666,41 @@ cp_gimplify_arg (tree *arg_p, gimple_seq *pre_p,
location_t call_location,
}
+/* Figure out if DECL should be promoted to consteval and if so, maybe also
+ promote the function we are in currently. CALL is the CALL_EXPR of DECL.
+ EVALP is where we may store the result of cxx_constant_value so that we
+ don't have to evaluate the same tree again in cp_fold_immediate_r. */
+
+static void
+maybe_escalate_decl_and_cfun (tree decl, tree call, tree *evalp)
+{
+ /* Compiler-generated functions don't seem like good candidates for
+ promoting. */
+ if (cp_unevaluated_operand || DECL_ARTIFICIAL (decl))
As discussed above, we probably don't want to exclude
implicitly-declared special member functions.
@@ -1073,20 +1309,42 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees,
void *data_)
from cp_fold_r and we must let it recurse on the expression with
cp_fold. */
break;
+
case PTRMEM_CST:
- if (TREE_CODE (PTRMEM_CST_MEMBER (stmt)) == FUNCTION_DECL
- && DECL_IMMEDIATE_FUNCTION_P (PTRMEM_CST_MEMBER (stmt)))
- {
- if (!data->pset.add (stmt) && (complain & tf_error))
- {
- error_at (PTRMEM_CST_LOCATION (stmt),
- "taking address of an immediate function %qD",
- PTRMEM_CST_MEMBER (stmt));
- *stmt_p = build_zero_cst (TREE_TYPE (stmt));
- }
- return error_mark_node;
- }
- break;
+ case ADDR_EXPR:
+ {
+ tree decl = (code == PTRMEM_CST
+ ? PTRMEM_CST_MEMBER (stmt)
+ : TREE_OPERAND (stmt, 0));
+ if (TREE_CODE (decl) != FUNCTION_DECL)
+ break;
+ if (code == ADDR_EXPR && ADDR_EXPR_DENOTES_CALL_P (stmt))
+ break;
+ if (immediate_invocation_p (decl))
+ {
+ if (maybe_promote_function_to_consteval (current_function_decl))
+ break;
+ if (complain & tf_error)
+ {
+ /* ??? Why don't we use data->pset for ADDR_EXPR too? */
Does doing that affect the testsuite? My guess is that it was done for
PTRMEM_CST to avoid duplicate errors, and isn't needed for ADDR_EXPR,
but it also shouldn't hurt for ADDR_EXPR.
+ if (code == ADDR_EXPR || !data->pset.add (stmt))
+ {
+ taking_address_of_imm_fn_error (stmt, decl);
+ *stmt_p = build_zero_cst (TREE_TYPE (stmt));
+ }
+ /* If we're giving hard errors, continue the walk rather than
+ bailing out after the first error. */
+ break;
+ }
+ return error_mark_node;
+ }
+ /* Not consteval yet, but could become one, in which case it's invalid
+ to take its address. */
+ else if (!(data->flags & ff_fold_immediate)
Why check ff_fold_immediate? Don't we want to do deferred checking of
immediate invocations in discarded branches?
+ && immediate_escalating_function_p (decl))
+ vec_safe_push (deferred_escalating_exprs, stmt);
+ break;
+ }
/* Expand immediate invocations. */
case CALL_EXPR:
@@ -1094,33 +1352,54 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees,
void *data_)
if (tree fn = cp_get_callee (stmt))
if (TREE_CODE (fn) != ADDR_EXPR || ADDR_EXPR_DENOTES_CALL_P (fn))
if (tree fndecl = cp_get_fndecl_from_callee (fn, /*fold*/false))
- if (DECL_IMMEDIATE_FUNCTION_P (fndecl))
- {
- stmt = cxx_constant_value (stmt, complain);
- if (stmt == error_mark_node)
- {
- if (complain & tf_error)
- *stmt_p = error_mark_node;
- return error_mark_node;
- }
- *stmt_p = stmt;
- }
- break;
-
- case ADDR_EXPR:
- if (TREE_CODE (TREE_OPERAND (stmt, 0)) == FUNCTION_DECL
- && DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (stmt, 0))
- && !ADDR_EXPR_DENOTES_CALL_P (stmt))
- {
- if (complain & tf_error)
{
- error_at (EXPR_LOCATION (stmt),
- "taking address of an immediate function %qD",
- TREE_OPERAND (stmt, 0));
- *stmt_p = build_zero_cst (TREE_TYPE (stmt));
+ tree eval = NULL_TREE;
+ if (data->flags & ff_escalating)
+ maybe_escalate_decl_and_cfun (fndecl, stmt, &eval);
+
+ /* [expr.const]p16 "An expression or conversion is
+ immediate-escalating if it is not initially in an immediate
+ function context and it is either
+ -- an immediate invocation that is not a constant expression
+ and is not a subexpression of an immediate invocation."
+
+ If we are in an immediate-escalating function, the
+ immediate-escalating expression or conversion makes it an
+ immediate function. So STMT does not need to produce
+ a constant expression. */
+ if (immediate_invocation_p (fndecl))
+ {
+ tree e = eval ? eval : cxx_constant_value (stmt, tf_none);
+ if (e == error_mark_node)
+ {
+ if (maybe_promote_function_to_consteval
+ (current_function_decl))
+ break;
+ if (complain & tf_error)
+ {
+ auto_diagnostic_group d;
+ location_t loc = cp_expr_loc_or_input_loc (stmt);
+ error_at (loc, "call to consteval function %qE is "
+ "not a constant expression", stmt);
+ /* Explain why it's not a constant expression. */
+ *stmt_p = cxx_constant_value (stmt, complain);
+ maybe_explain_promoted_consteval (loc, fndecl);
+ /* Don't return error_mark_node, it would stop our
+ tree walk. */
+ break;
+ }
+ return error_mark_node;
+ }
+ /* We've evaluated the consteval function call. */
+ *stmt_p = e;
+ }
+ /* We've encountered a function call that may turn out to be
+ consteval later. Store its caller so that we can ensure
+ that the call is a constant expression. */
+ else if (!(data->flags & ff_fold_immediate)
Likewise.
It also seems odd that the ADDR_EXPR case calls vec_safe_push
(deferred_escalating_exprs, while the CALL_EXPR case calls
maybe_store_cfun_for_late_checking, why the different handling?
+ && immediate_escalating_function_p (fndecl))
+ maybe_store_cfun_for_late_checking ();
}
- return error_mark_node;
- }
break;
default:
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index e3fb2299d93..02c50ac6a11 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -5685,6 +5685,8 @@ trees_out::lang_decl_bools (tree t)
WB (lang->u.fn.has_dependent_explicit_spec_p);
WB (lang->u.fn.immediate_fn_p);
WB (lang->u.fn.maybe_deleted);
+ /* We do not stream lang->u.implicit_constexpr nor
+ lang->u.escalated_p. */
Won't this mean not getting maybe_explain_promoted_consteval for
imported functions?
Jason
