On Thu, Dec 12, 2024 at 07:07:38PM +0000, Simon Martin wrote:
> We currently ICE upon the following valid (under -Wno-vla) code
>
> === cut here ===
> void f(int c) {
> constexpr int r = 4;
> [&](auto) { int t[r * c]; }(0);
> }
> === cut here ===
>
> The problem is that when parsing the lambda body, and more specifically
> the multiplication, we mark the lambda as LAMBDA_EXPR_CAPTURE_OPTIMIZED
> even though the replacement of r by 4 is "undone" by the call to
> build_min_non_dep in build_x_binary_op. This makes prune_lambda_captures
Ah yeah, because build_min_non_dep gets the original operands.
> remove the proxy declaration while it should not, and we trip on an
> assert at instantiation time.
>
> This patch fixes the ICE by making sure that lambdas are only marked as
> LAMBDA_EXPR_CAPTURE_OPTIMIZED when they're instantiated (I tried other
> strategies like not undoing constant folding in build_min_non_dep, but
> it is pretty intrusive and breaks lots of things).
I've tried that too and I also ran into a number of issues. I also tried
checking p_t_d in prune_lambda_capture since it already says "Don't bother
pruning in a template" but that doesn't work, either.
> The test I added also shows that we don't always optimize out captures
> to constants for lambdas that are not within a template (see ok_2 for
> example, or ok_3 that unlike ok_2 "regresses" a bit with my patch) - I'm
> curious if we consider it a problem or not? If so, I can try to fix this
> in a follow-up patch.
Since "P0588R1: Simplifying implicit lambda capture" they are captures,
though [expr.prim.lambda.capture] gives us license to optimize the
captures if not ODR-used. I couldn't find a test where this patch
would be an ABI change.
> Successfully tested on x86_64-pc-linux-gnu.
>
> PR c++/114292
>
> gcc/cp/ChangeLog:
>
> * cp-tree.h (mark_const_var_capture_optimized): Declare.
> * expr.cc (mark_use): Call mark_const_var_capture_optimized.
> * lambda.cc (mark_const_var_capture_optimized): New. Only set
> LAMBDA_EXPR_CAPTURE_OPTIMIZED at lambda instantiation time.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp1y/lambda-ice4.C: New test.
>
> ---
> gcc/cp/cp-tree.h | 1 +
> gcc/cp/expr.cc | 10 ++----
> gcc/cp/lambda.cc | 13 +++++++
> gcc/testsuite/g++.dg/cpp1y/lambda-ice4.C | 44 ++++++++++++++++++++++++
> 4 files changed, 60 insertions(+), 8 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-ice4.C
>
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index c5e0fc5c440..ce050032fdb 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -8058,6 +8058,7 @@ extern bool is_constant_capture_proxy (tree);
> extern void register_capture_members (tree);
> extern tree lambda_expr_this_capture (tree, int);
> extern void maybe_generic_this_capture (tree, tree);
> +extern void mark_const_var_capture_optimized (void);
> extern tree maybe_resolve_dummy (tree, bool);
> extern tree current_nonlambda_function (void);
> extern tree nonlambda_method_basetype (void);
> diff --git a/gcc/cp/expr.cc b/gcc/cp/expr.cc
> index de4991e616c..d6a2454c46e 100644
> --- a/gcc/cp/expr.cc
> +++ b/gcc/cp/expr.cc
> @@ -120,10 +120,7 @@ mark_use (tree expr, bool rvalue_p, bool read_p,
> {
> tree val = RECUR (cap);
> if (!is_capture_proxy (val))
> - {
> - tree l = current_lambda_expr ();
> - LAMBDA_EXPR_CAPTURE_OPTIMIZED (l) = true;
> - }
> + mark_const_var_capture_optimized ();
> return val;
> }
> }
> @@ -171,10 +168,7 @@ mark_use (tree expr, bool rvalue_p, bool read_p,
> {
> tree val = RECUR (cap);
> if (!is_capture_proxy (val))
> - {
> - tree l = current_lambda_expr ();
> - LAMBDA_EXPR_CAPTURE_OPTIMIZED (l) = true;
> - }
> + mark_const_var_capture_optimized ();
> return val;
> }
> }
> diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
> index d8a15d97d5d..4fd3b39c99b 100644
> --- a/gcc/cp/lambda.cc
> +++ b/gcc/cp/lambda.cc
> @@ -945,6 +945,19 @@ resolvable_dummy_lambda (tree object)
> return NULL_TREE;
> }
>
> +/* Called when optimizing out a capture to a const variable. */
> +
> +void
> +mark_const_var_capture_optimized ()
> +{
> + /* The actual optimizing out only occurs when instantiating the lambda. */
> + if (processing_template_decl)
> + return;
> +
> + tree l = current_lambda_expr ();
> + LAMBDA_EXPR_CAPTURE_OPTIMIZED (l) = true;
> +}
If we do this, perhaps the function can be a static function in expr.cc,
and maybe we can check processing_template_decl before is_capture_proxy.
Marek
