On 9/8/23 14:24, Marek Polacek wrote:
On Thu, Sep 07, 2023 at 02:32:51PM -0400, Jason Merrill wrote:
On 9/7/23 11:23, Marek Polacek wrote:
On Tue, Sep 05, 2023 at 04:36:34PM -0400, Jason Merrill wrote:
On 9/5/23 15:59, Marek Polacek wrote:
On Tue, Sep 05, 2023 at 10:52:04AM -0400, Jason Merrill wrote:
On 9/1/23 13:23, Marek Polacek wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
In the review of P2564:
<https://gcc.gnu.org/pipermail/gcc-patches/2023-August/628747.html>
it turned out that in order to correctly handle an example in the paper,
we should stop doing immediate evaluation in build_over_call and
bot_replace, and instead do it in cp_fold_r. This patch does that.
Another benefit is that this is a pretty significant simplification, at
least in my opinion. Also, this fixes the c++/110997 ICE (but the test
doesn't compile yet).
The main drawback seems to be that cp_fold_r doesn't process as much
code as we did before: uninstantiated templates
That's acceptable, it's an optional diagnostic.
and things like "false ? foo () : 1".
This is a problem. Maybe we want cp_fold_r to recurse into the arms of a
COND_EXPR before folding them away? Maybe only if we know we've seen an
immediate function?
Unfortunately we had already thrown the dead branch away when we got to
cp_fold_r. I wonder if we have to adjust cxx_eval_conditional_expression
to call cp_fold_r on the dead branch too,
Hmm, I guess so.
perhaps with a new ff_ flag to skip the whole second switch in cp_fold_r?
Or factor out the immediate function handling to a separate walk function
that cp_fold_r also calls?
I did that.
But then it's possible that the in_immediate_context checks have to stay.
We can just not do the walk in immediate (or mce_true) context, like we
currently avoid calling cp_fold_function.
Right. Unfortunately I have to check even when mce_true, consider
consteval int bar (int i) { if (i != 1) throw 1; return 0; }
constexpr int a = 0 ? bar(3) : 3;
I disagree; the call is in a manifestly constant-evaluated expression, and
so is now considered an immediate function context, and we should accept
that example.
Ack. I was still living in pre-P2564 world.
For mce_unknown I guess we'd want
to set *non_constant_p instead of giving an error.
I did not do this because I haven't found a case where it would make
a difference.
I think it will given the above comment.
Correct. For instance, in:
consteval int bar (int i) { if (i != 1) throw 1; return 0; }
constexpr int
foo (bool b)
{
return b ? bar (3) : 2;
}
static_assert (foo (false) == 2);
we should complain only once. I've implemented your suggestion to set
*non_constant_p instead of giving an error for mce_unknown.
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 0ca4370deab..397d5c7ec3f 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2311,6 +2311,29 @@ cxx_dynamic_cast_fn_p (tree fndecl)
&& CP_DECL_CONTEXT (fndecl) == abi_node);
}
+/* Return true if we are in the body of a consteval function. > + This is in
addition to in_immediate_context because that
+ uses current_function_decl which may not be available. CTX is
+ the current constexpr context. */
+
+static bool
+in_immediate_context (const constexpr_ctx *ctx)
+{
+ if (in_immediate_context ())
+ return true;
Can't we check for mce_true here instead of looking at the call chain?
Yes.
+/* A wrapper around cp_fold_immediate_r. */
+
+void
+cp_fold_immediate (tree *tp)
+{
Maybe return early if consteval isn't supported in the active standard?
Absolutely.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
In the review of P2564:
<https://gcc.gnu.org/pipermail/gcc-patches/2023-August/628747.html>
it turned out that in order to correctly handle an example in the paper,
we should stop doing immediate evaluation in build_over_call and
bot_replace, and instead do it in cp_fold_r. This patch does that.
Another benefit is that this is a pretty significant simplification, at
least in my opinion. Also, this fixes the c++/110997 ICE (but the test
doesn't compile yet).
The main drawback seems to be that cp_fold_r doesn't process
uninstantiated templates. We still have to handle things like
"false ? foo () : 1". To that end, I've added cp_fold_immediate, called
on dead branches in cxx_eval_conditional_expression.
You'll see that I've reintroduced ADDR_EXPR_DENOTES_CALL_P here. This
is to detect
*(&foo)) ()
(s.*&S::foo) ()
which were deemed ill-formed.
gcc/cp/ChangeLog:
* call.cc (build_over_call): Set ADDR_EXPR_DENOTES_CALL_P. Don't handle
immediate_invocation_p here.
* constexpr.cc (in_immediate_context): New overload.
(cxx_eval_call_expression): Use mce_true for DECL_IMMEDIATE_FUNCTION_P.
(cxx_eval_conditional_expression): Call cp_fold_immediate.
* cp-gimplify.cc (maybe_replace_decl): Make static.
(cp_fold_r): Expand immediate invocations.
(cp_fold_immediate_r): New.
(cp_fold_immediate): New.
* cp-tree.h (ADDR_EXPR_DENOTES_CALL_P): Define.
(cp_fold_immediate): Declare.
* tree.cc (bot_replace): Don't handle immediate invocations here.
libstdc++-v3/ChangeLog:
* testsuite/20_util/allocator/105975.cc: Add dg-error.
gcc/testsuite/ChangeLog:
* g++.dg/cpp23/consteval-if2.C: Add xfail.
* g++.dg/cpp2a/consteval-memfn1.C: Adjust.
* g++.dg/cpp2a/consteval11.C: Remove dg-message.
* g++.dg/cpp2a/consteval3.C: Remove dg-message and dg-error.
* g++.dg/cpp2a/consteval9.C: Remove dg-message.
* g++.dg/cpp2a/consteval32.C: New test.
* g++.dg/cpp2a/consteval33.C: New test.
* g++.dg/cpp2a/consteval34.C: New test.
* g++.dg/cpp2a/consteval35.C: New test.
---
gcc/cp/call.cc | 40 +------
gcc/cp/constexpr.cc | 23 +++-
gcc/cp/cp-gimplify.cc | 108 ++++++++++++++----
gcc/cp/cp-tree.h | 42 ++++---
gcc/cp/tree.cc | 23 +---
gcc/testsuite/g++.dg/cpp23/consteval-if2.C | 3 +-
gcc/testsuite/g++.dg/cpp2a/consteval-memfn1.C | 7 ++
gcc/testsuite/g++.dg/cpp2a/consteval11.C | 33 +++---
gcc/testsuite/g++.dg/cpp2a/consteval3.C | 3 +-
gcc/testsuite/g++.dg/cpp2a/consteval32.C | 4 +
gcc/testsuite/g++.dg/cpp2a/consteval33.C | 34 ++++++
gcc/testsuite/g++.dg/cpp2a/consteval34.C | 18 +++
gcc/testsuite/g++.dg/cpp2a/consteval35.C | 10 ++
gcc/testsuite/g++.dg/cpp2a/consteval9.C | 3 +-
.../testsuite/20_util/allocator/105975.cc | 2 +-
15 files changed, 234 insertions(+), 119 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval32.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval33.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval34.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval35.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 0ca4370deab..8c077aa22fe 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3837,12 +3841,25 @@ cxx_eval_conditional_expression (const constexpr_ctx
*ctx, tree t,
boolean_type_node);
}
/* Don't VERIFY_CONSTANT the other operands. */
- if (integer_zerop (val))
+ const bool zero_p = integer_zerop (val);
+ if (zero_p)
val = TREE_OPERAND (t, 2);
else
val = TREE_OPERAND (t, 1);
if (TREE_CODE (t) == IF_STMT && !val)
val = void_node;
+
+ if (!in_immediate_context ()
+ /* P2564: a subexpression of a manifestly constant-evaluated expression
+ or conversion is an immediate function context. */
+ && ctx->manifestly_const_eval != mce_true
I might check this first as a tiny optimization.
+ && cp_fold_immediate (&TREE_OPERAND (t, zero_p ? 1 : 2),
+ ctx->manifestly_const_eval))
+ {
+ *non_constant_p = true;
+ return t;
+ }
+
/* A TARGET_EXPR may be nested inside another TARGET_EXPR, but still
serve as the initializer for the same object as the outer TARGET_EXPR,
as in
diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index 206e791fcfd..9901513461c 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1000,7 +1000,7 @@ cp_genericize_target_expr (tree *stmt_p)
replacement when cp_folding TARGET_EXPR to preserve the invariant that
AGGR_INIT_EXPR_SLOT agrees with the enclosing TARGET_EXPR_SLOT. */
-bool
+static bool
maybe_replace_decl (tree *tp, tree decl, tree replacement)
{
if (!*tp || !VOID_TYPE_P (TREE_TYPE (*tp)))
@@ -1029,44 +1029,73 @@ struct cp_genericize_data
bool handle_invisiref_parm_p;
};
-/* Perform any pre-gimplification folding of C++ front end trees to
- GENERIC.
- Note: The folding of non-omp cases is something to move into
- the middle-end. As for now we have most foldings only on GENERIC
- in fold-const, we need to perform this before transformation to
- GIMPLE-form. */
+/* A subroutine of cp_fold_r to handle immediate functions. */
static tree
-cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data_)
+cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees, void *data_)
{
- cp_fold_data *data = (cp_fold_data*)data_;
+ auto data = static_cast<cp_fold_data *>(data_);
tree stmt = *stmt_p;
- enum tree_code code = TREE_CODE (stmt);
+ const tsubst_flags_t complain = (data->flags == ff_none ? tf_none :
tf_error);
- switch (code)
+ /* No need to look into types or unevaluated operands.
+ NB: This affects cp_fold_r as well. */
+ if (TYPE_P (stmt) || unevaluated_p (TREE_CODE (stmt)))
{
+ *walk_subtrees = 0;
+ return NULL_TREE;
+ }
+
+ switch (TREE_CODE (stmt))
+ {
+ /* Unfortunately we must handle code like
+ false ? bar () : 42
+ where we have to check bar too. */
+ case COND_EXPR:
+ if (cp_fold_immediate_r (&TREE_OPERAND (stmt, 1), walk_subtrees, data))
+ return error_mark_node;
+ if (TREE_OPERAND (stmt, 2)
+ && cp_fold_immediate_r (&TREE_OPERAND (stmt, 2), walk_subtrees, data))
+ return error_mark_node;
Is this necessary? Doesn't walk_tree already walk into the arms of
COND_EXPR?
+ 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))
+ 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 = *stmt_p = build_zero_cst (TREE_TYPE (stmt));
It looks like this will overwrite *stmt_p even if we didn't give an error.
- break;
+ return error_mark_node;
}
break;
+ /* Expand immediate invocations. */
+ case CALL_EXPR:
+ case AGGR_INIT_EXPR:
+ 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_p = stmt = cxx_constant_value (stmt, complain);
Likewise.
+ if (stmt == error_mark_node)
+ return error_mark_node;
+ }
+ break;
+
case ADDR_EXPR:
if (TREE_CODE (TREE_OPERAND (stmt, 0)) == FUNCTION_DECL
&& DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (stmt, 0)))
{
- error_at (EXPR_LOCATION (stmt),
- "taking address of an immediate function %qD",
- TREE_OPERAND (stmt, 0));
+ if (complain & tf_error)
+ error_at (EXPR_LOCATION (stmt),
+ "taking address of an immediate function %qD",
+ TREE_OPERAND (stmt, 0));
stmt = *stmt_p = build_zero_cst (TREE_TYPE (stmt));
Likewise.
Jason
