On Tue, Apr 15, 2025 at 09:16:46AM -0400, Jason Merrill wrote:
> On 4/15/25 2:56 AM, Nathaniel Shead wrote:
> > On Mon, Apr 14, 2025 at 05:33:05PM -0400, Jason Merrill wrote:
> > > On 4/13/25 6:32 AM, Nathaniel Shead wrote:
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > > >
> > > > -- >8 --
> > > >
> > > > Currently, pruned lambda captures are still leftover in the function's
> > > > BLOCK and topmost BIND_EXPR; this doesn't cause any issues for normal
> > > > compilation, but does break modules streaming as we try to reconstruct a
> > > > FIELD_DECL that no longer exists on the type itself.
> > > >
> > > > PR c++/119755
> > > >
> > > > gcc/cp/ChangeLog:
> > > >
> > > > * lambda.cc (prune_lambda_captures): Remove pruned capture from
> > > > function's BLOCK_VARS and BIND_EXPR_VARS.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > * g++.dg/modules/lambda-10_a.H: New test.
> > > > * g++.dg/modules/lambda-10_b.C: New test.
> > > >
> > > > Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> > > > ---
> > > > gcc/cp/lambda.cc | 22
> > > > ++++++++++++++++++++++
> > > > gcc/testsuite/g++.dg/modules/lambda-10_a.H | 17 +++++++++++++++++
> > > > gcc/testsuite/g++.dg/modules/lambda-10_b.C | 7 +++++++
> > > > 3 files changed, 46 insertions(+)
> > > > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-10_a.H
> > > > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-10_b.C
> > > >
> > > > diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
> > > > index f0a54b60275..d01bb04cd32 100644
> > > > --- a/gcc/cp/lambda.cc
> > > > +++ b/gcc/cp/lambda.cc
> > > > @@ -1858,6 +1858,14 @@ prune_lambda_captures (tree body)
> > > > cp_walk_tree_without_duplicates (&body, mark_const_cap_r,
> > > > &const_vars);
> > > > + tree bind_expr = expr_single (DECL_SAVED_TREE (lambda_function
> > > > (lam)));
> > > > + gcc_assert (bind_expr
> > > > + && (TREE_CODE (bind_expr) == BIND_EXPR
> > > > + /* FIXME: In a noexcept lambda we never prune captures
> > > > + (PR119764); when we do we need to handle this case
> > > > + for modules streaming. */
> > >
> > > The attached patch seems to fix that, with the result that your patch
> > > crashes.
> > >
> >
> > Thanks. And yup, crashing was deliberate here as I wasn't 100% sure
> > what the tree would look like for this case after an appropriate fix.
> >
> > One quick question about your patch, since it could in theory affect ABI
> > (the size of the lambdas change) should the pruning of such lambdas be
> > dependent on an ABI version check?
>
> Indeed, perhaps this is too late in the 15 cycle for such a change.
>
> > Otherwise here's an updated patch that relies on your patch.
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk along
> > with yours? (Or if the potential ABI concerns mean that your change
> > isn't appropriate for GCC15, would the old version of my patch still be
> > OK for GCC15 to get 'import std' working again for C++26?)
>
> For 15 please adjust this patch to be more fault-tolerant:
>
> > -- >8 --
> >
> > Currently, pruned lambda captures are still leftover in the function's
> > BLOCK and topmost BIND_EXPR; this doesn't cause any issues for normal
> > compilation, but does break modules streaming as we try to reconstruct a
> > FIELD_DECL that no longer exists on the type itself.
> >
> > PR c++/119755
> >
> > gcc/cp/ChangeLog:
> >
> > * lambda.cc (prune_lambda_captures): Remove pruned capture from
> > function's BLOCK_VARS and BIND_EXPR_VARS.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/lambda-10_a.H: New test.
> > * g++.dg/modules/lambda-10_b.C: New test.
> >
> > Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> > Reviewed-by: Jason Merrill <ja...@redhat.com>
> > ---
> > gcc/cp/lambda.cc | 19 +++++++++++++++++++
> > gcc/testsuite/g++.dg/modules/lambda-10_a.H | 17 +++++++++++++++++
> > gcc/testsuite/g++.dg/modules/lambda-10_b.C | 7 +++++++
> > 3 files changed, 43 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-10_a.H
> > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-10_b.C
> >
> > diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
> > index c6308b941d3..7bb88a900d5 100644
> > --- a/gcc/cp/lambda.cc
> > +++ b/gcc/cp/lambda.cc
> > @@ -1862,6 +1862,11 @@ prune_lambda_captures (tree body)
> > cp_walk_tree_without_duplicates (&body, mark_const_cap_r, &const_vars);
> > + tree bind_expr = expr_single (DECL_SAVED_TREE (lambda_function (lam)));
> > + if (bind_expr && TREE_CODE (bind_expr) == MUST_NOT_THROW_EXPR)
> > + bind_expr = expr_single (TREE_OPERAND (bind_expr, 0));
> > + gcc_assert (bind_expr && TREE_CODE (bind_expr) == BIND_EXPR);
>
> i.e. here clear bind_expr if it isn't a BIND_EXPR...
>
> > tree *fieldp = &TYPE_FIELDS (LAMBDA_EXPR_CLOSURE (lam));
> > for (tree *capp = &LAMBDA_EXPR_CAPTURE_LIST (lam); *capp; )
> > {
> > @@ -1883,6 +1888,20 @@ prune_lambda_captures (tree body)
> > fieldp = &DECL_CHAIN (*fieldp);
> > *fieldp = DECL_CHAIN (*fieldp);
> > + /* And out of the bindings for the function. */
> > + tree *blockp = &BLOCK_VARS (current_binding_level->blocks);
> > + while (*blockp != DECL_EXPR_DECL (**use))
> > + blockp = &DECL_CHAIN (*blockp);
> > + *blockp = DECL_CHAIN (*blockp);
> > +
> > + /* And maybe out of the vars declared in the containing
> > + BIND_EXPR, if it's listed there. */
> > + tree *bindp = &BIND_EXPR_VARS (bind_expr);
> > + while (*bindp && *bindp != DECL_EXPR_DECL (**use))
> > + bindp = &DECL_CHAIN (*bindp);
> > + if (*bindp)
> > + *bindp = DECL_CHAIN (*bindp);
>
> ...and only do this if bind_expr is set.
>
> Then it should work with or without my patch, right?
Right; here's a slight variation on that (taking advantage of the fact
that the bug means that 'expr_single' will return NULL_TREE if the
variables got incorrectly pushed into that statement list). Tested so
far on dg.exp=*lambda* modules.exp=*lambda*, OK for trunk if full
bootstrap+regtest passes?
-- >8 --
Currently, pruned lambda captures are still leftover in the function's
BLOCK and topmost BIND_EXPR; this doesn't cause any issues for normal
compilation, but does break modules streaming as we try to reconstruct a
FIELD_DECL that no longer exists on the type itself.
PR c++/119755
gcc/cp/ChangeLog:
* lambda.cc (prune_lambda_captures): Remove pruned capture from
function's BLOCK_VARS and BIND_EXPR_VARS.
gcc/testsuite/ChangeLog:
* g++.dg/modules/lambda-10_a.H: New test.
* g++.dg/modules/lambda-10_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
Reviewed-by: Jason Merrill <ja...@redhat.com>
---
gcc/cp/lambda.cc | 24 ++++++++++++++++++++++
gcc/testsuite/g++.dg/modules/lambda-10_a.H | 17 +++++++++++++++
gcc/testsuite/g++.dg/modules/lambda-10_b.C | 7 +++++++
3 files changed, 48 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/lambda-10_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/lambda-10_b.C
diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
index f0a54b60275..b2e0ecdd67e 100644
--- a/gcc/cp/lambda.cc
+++ b/gcc/cp/lambda.cc
@@ -1858,6 +1858,13 @@ prune_lambda_captures (tree body)
cp_walk_tree_without_duplicates (&body, mark_const_cap_r, &const_vars);
+ tree bind_expr = expr_single (DECL_SAVED_TREE (lambda_function (lam)));
+ if (bind_expr && TREE_CODE (bind_expr) == MUST_NOT_THROW_EXPR)
+ bind_expr = expr_single (TREE_OPERAND (bind_expr, 0));
+ /* FIXME: We don't currently handle noexcept lambda captures correctly,
+ so bind_expr may not be set; see PR c++/119764. */
+ gcc_assert (!bind_expr || TREE_CODE (bind_expr) == BIND_EXPR);
+
tree *fieldp = &TYPE_FIELDS (LAMBDA_EXPR_CLOSURE (lam));
for (tree *capp = &LAMBDA_EXPR_CAPTURE_LIST (lam); *capp; )
{
@@ -1879,6 +1886,23 @@ prune_lambda_captures (tree body)
fieldp = &DECL_CHAIN (*fieldp);
*fieldp = DECL_CHAIN (*fieldp);
+ /* And out of the bindings for the function. */
+ tree *blockp = &BLOCK_VARS (current_binding_level->blocks);
+ while (*blockp != DECL_EXPR_DECL (**use))
+ blockp = &DECL_CHAIN (*blockp);
+ *blockp = DECL_CHAIN (*blockp);
+
+ /* And maybe out of the vars declared in the containing
+ BIND_EXPR, if it's listed there. */
+ if (bind_expr)
+ {
+ tree *bindp = &BIND_EXPR_VARS (bind_expr);
+ while (*bindp && *bindp != DECL_EXPR_DECL (**use))
+ bindp = &DECL_CHAIN (*bindp);
+ if (*bindp)
+ *bindp = DECL_CHAIN (*bindp);
+ }
+
/* And remove the capture proxy declaration. */
**use = void_node;
continue;
diff --git a/gcc/testsuite/g++.dg/modules/lambda-10_a.H
b/gcc/testsuite/g++.dg/modules/lambda-10_a.H
new file mode 100644
index 00000000000..1ad1a808c07
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-10_a.H
@@ -0,0 +1,17 @@
+// PR c++/119755
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+template <typename _Out> void format(_Out) {
+ constexpr int __term = 1;
+ [&] { __term; };
+ [&] { const int outer = __term; { __term; } };
+ [&]() noexcept { __term; };
+ [&]() noexcept { const int outer = __term; { __term; } };
+ [&](auto) { int n[__term]; }(0);
+ [&](auto) noexcept { int n[__term]; }(0);
+}
+
+inline void vformat() {
+ format(0);
+}
diff --git a/gcc/testsuite/g++.dg/modules/lambda-10_b.C
b/gcc/testsuite/g++.dg/modules/lambda-10_b.C
new file mode 100644
index 00000000000..3556bcee02c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-10_b.C
@@ -0,0 +1,7 @@
+// PR c++/119755
+// { dg-additional-options "-fmodules" }
+
+import "lambda-10_a.H";
+int main() {
+ vformat();
+}
--
2.47.0
