[PATCH] c++/modules: ICE with lambda initializing local var [PR105322]

[PATCH] c++/modules: ICE with lambda initializing local var [PR105322]

[Patrick Palka]

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

-- >8 --

For a local variable initialized by a lambda:

  auto f = []{};

The corresponding BLOCK_VARS contains the variable declaration first,
followed by the closure type declaration, consistent with the
syntactical order.  This however means that a use of the closure type
appears (in the variable type/initializer) before the declaration of the
type.  This ends up causing an ICE when streaming the BLOCK_VARS of f1
below because we stream (by value) the CONSTRUCTOR initializer of g1 --
which contains components of the closure type -- before we've streamed
the declaration defining the closure type.  The following comment in
module.cc seems relevant:

  /* We want to stream the type of a expression-like nodes /after/
     we've streamed the operands.  The type often contains (bits
     of the) types of the operands, and with things like decltype
     and noexcept in play, we really want to stream the decls
     defining the type before we try and stream the type on its
     own.  Otherwise we can find ourselves trying to read in a
     decl, when we're already partially reading in a component of
     its type.  And that's bad.  */

This patch narrowly fixes this issue by special casing closure type
declarations in add_decl_to_level.  (A loop is needed since there could
be multiple variable declarations with an unprocessed initializer in
light of structured bindings.)

	PR c++/105322

gcc/cp/ChangeLog:

	* name-lookup.cc (add_decl_to_level): When adding a closure
	type declaration to a block scope, add it before rather than
	after any variable declarations whose initializer we're still
	processing.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/lambda-5_a.C: New test.
	* g++.dg/modules/lambda-5_b.C: New test.
---
 gcc/cp/name-lookup.cc                     | 19 ++++++++++++++++---
 gcc/testsuite/g++.dg/modules/lambda-5_a.C | 23 +++++++++++++++++++++++
 gcc/testsuite/g++.dg/modules/lambda-5_b.C | 10 ++++++++++
 3 files changed, 49 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_b.C

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index a8b9229b29e..bb00baaf9f4 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -391,9 +391,22 @@ add_decl_to_level (cp_binding_level *b, tree decl)
   gcc_assert (b->names != decl);
 
   /* We build up the list in reverse order, and reverse it later if
-     necessary.  */
-  TREE_CHAIN (decl) = b->names;
-  b->names = decl;
+     necessary.  If we're adding a lambda closure type to a block
+     scope as part of a local variable initializer, then make sure
+     we declare the type before the variable; modules expects that
+     we see a type declaration before a use of the type.  */
+  tree *prev = &b->names;
+  if (b->kind == sk_block
+      && !processing_template_decl
+      && TREE_CODE (decl) == TYPE_DECL
+      && LAMBDA_TYPE_P (TREE_TYPE (decl)))
+    while (*prev && VAR_P (*prev)
+	   && !DECL_EXTERNAL (*prev)
+	   && !DECL_INITIALIZED_P (*prev))
+      prev = &TREE_CHAIN (*prev);
+
+  TREE_CHAIN (decl) = *prev;
+  *prev = decl;
 
   /* If appropriate, add decl to separate list of statics.  We include
      extern variables because they might turn out to be static later.
diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
new file mode 100644
index 00000000000..6b54c8e3173
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
@@ -0,0 +1,23 @@
+// PR c++/105322
+// { dg-additional-options -fmodules-ts }
+// { dg-module-cmi pr105322 }
+
+export module pr105322;
+
+struct A { };
+
+export
+inline void f1() {
+  A a;
+  auto g1 = [a] { }; // used to ICE here during stream out
+}
+
+export
+template<class...>
+void f2() {
+  A a;
+  auto g2 = [a] { };
+}
+
+export
+inline auto g3 = [a=A{}] { };
diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_b.C b/gcc/testsuite/g++.dg/modules/lambda-5_b.C
new file mode 100644
index 00000000000..e25a913b726
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-5_b.C
@@ -0,0 +1,10 @@
+// PR c++/105322
+// { dg-additional-options -fmodules-ts }
+
+import pr105322;
+
+int main() {
+  f1();
+  f2();
+  g3();
+}
-- 
2.42.0.398.ga9ecda2788

Re: [PATCH] c++/modules: ICE with lambda initializing local var [PR105322]

[Patrick Palka]

On Wed, 18 Oct 2023, Patrick Palka wrote:

> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?

Note that this doesn't fix the other testcase in the PR, which doesn't use any
lambdas and which ICEs in the same way:

    export module pr105322;

    auto f() {
      struct A { int m; };
      return A{};
    }

    export
    inline void g() {
      auto r = decltype(f()){0};
    }

Here when streaming the CONSTRUCTOR initializer of r, we end up streaming
components of f()::A before ever streaming the declaration/definition of
f()::A.  I suspect a separate fix might be needed for this testcase?
The narrow fix for the lambda testcase still seems useful nonetheless.

> 
> -- >8 --
> 
> For a local variable initialized by a lambda:
> 
>   auto f = []{};
> 
> The corresponding BLOCK_VARS contains the variable declaration first,
> followed by the closure type declaration, consistent with the
> syntactical order.  This however means that a use of the closure type
> appears (in the variable type/initializer) before the declaration of the
> type.  This ends up causing an ICE when streaming the BLOCK_VARS of f1
> below because we stream (by value) the CONSTRUCTOR initializer of g1 --
> which contains components of the closure type -- before we've streamed
> the declaration defining the closure type.  The following comment in
> module.cc seems relevant:
> 
>   /* We want to stream the type of a expression-like nodes /after/
>      we've streamed the operands.  The type often contains (bits
>      of the) types of the operands, and with things like decltype
>      and noexcept in play, we really want to stream the decls
>      defining the type before we try and stream the type on its
>      own.  Otherwise we can find ourselves trying to read in a
>      decl, when we're already partially reading in a component of
>      its type.  And that's bad.  */
> 
> This patch narrowly fixes this issue by special casing closure type
> declarations in add_decl_to_level.  (A loop is needed since there could
> be multiple variable declarations with an unprocessed initializer in
> light of structured bindings.)
> 
> 	PR c++/105322
> 
> gcc/cp/ChangeLog:
> 
> 	* name-lookup.cc (add_decl_to_level): When adding a closure
> 	type declaration to a block scope, add it before rather than
> 	after any variable declarations whose initializer we're still
> 	processing.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/lambda-5_a.C: New test.
> 	* g++.dg/modules/lambda-5_b.C: New test.
> ---
>  gcc/cp/name-lookup.cc                     | 19 ++++++++++++++++---
>  gcc/testsuite/g++.dg/modules/lambda-5_a.C | 23 +++++++++++++++++++++++
>  gcc/testsuite/g++.dg/modules/lambda-5_b.C | 10 ++++++++++
>  3 files changed, 49 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_b.C
> 
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index a8b9229b29e..bb00baaf9f4 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -391,9 +391,22 @@ add_decl_to_level (cp_binding_level *b, tree decl)
>    gcc_assert (b->names != decl);
>  
>    /* We build up the list in reverse order, and reverse it later if
> -     necessary.  */
> -  TREE_CHAIN (decl) = b->names;
> -  b->names = decl;
> +     necessary.  If we're adding a lambda closure type to a block
> +     scope as part of a local variable initializer, then make sure
> +     we declare the type before the variable; modules expects that
> +     we see a type declaration before a use of the type.  */
> +  tree *prev = &b->names;
> +  if (b->kind == sk_block
> +      && !processing_template_decl
> +      && TREE_CODE (decl) == TYPE_DECL
> +      && LAMBDA_TYPE_P (TREE_TYPE (decl)))
> +    while (*prev && VAR_P (*prev)
> +	   && !DECL_EXTERNAL (*prev)
> +	   && !DECL_INITIALIZED_P (*prev))
> +      prev = &TREE_CHAIN (*prev);
> +
> +  TREE_CHAIN (decl) = *prev;
> +  *prev = decl;
>  
>    /* If appropriate, add decl to separate list of statics.  We include
>       extern variables because they might turn out to be static later.
> diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> new file mode 100644
> index 00000000000..6b54c8e3173
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> @@ -0,0 +1,23 @@
> +// PR c++/105322
> +// { dg-additional-options -fmodules-ts }
> +// { dg-module-cmi pr105322 }
> +
> +export module pr105322;
> +
> +struct A { };
> +
> +export
> +inline void f1() {
> +  A a;
> +  auto g1 = [a] { }; // used to ICE here during stream out
> +}
> +
> +export
> +template<class...>
> +void f2() {
> +  A a;
> +  auto g2 = [a] { };
> +}
> +
> +export
> +inline auto g3 = [a=A{}] { };
> diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_b.C b/gcc/testsuite/g++.dg/modules/lambda-5_b.C
> new file mode 100644
> index 00000000000..e25a913b726
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/lambda-5_b.C
> @@ -0,0 +1,10 @@
> +// PR c++/105322
> +// { dg-additional-options -fmodules-ts }
> +
> +import pr105322;
> +
> +int main() {
> +  f1();
> +  f2();
> +  g3();
> +}
> -- 
> 2.42.0.398.ga9ecda2788
> 
> 

Re: [PATCH] c++/modules: ICE with lambda initializing local var [PR105322]

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 5078 bytes --]

Thanks for looking at this, but your patch is essentially papering over the problem.

It took me a while to figure out, but the clue was that things like 
'decltype(f()).m' worked, but 'decltype(f()){0}' did not.  The CONSTRUCTOR node 
is the exception to the rule that required an expression node's type to be 
streamed after the node's operands.  We want the opposite for CTORS.

I'll commit this once bootstrapped.

nathan

On 10/18/23 12:28, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> -- >8 --
> 
> For a local variable initialized by a lambda:
> 
>    auto f = []{};
> 
> The corresponding BLOCK_VARS contains the variable declaration first,
> followed by the closure type declaration, consistent with the
> syntactical order.  This however means that a use of the closure type
> appears (in the variable type/initializer) before the declaration of the
> type.  This ends up causing an ICE when streaming the BLOCK_VARS of f1
> below because we stream (by value) the CONSTRUCTOR initializer of g1 --
> which contains components of the closure type -- before we've streamed
> the declaration defining the closure type.  The following comment in
> module.cc seems relevant:
> 
>    /* We want to stream the type of a expression-like nodes /after/
>       we've streamed the operands.  The type often contains (bits
>       of the) types of the operands, and with things like decltype
>       and noexcept in play, we really want to stream the decls
>       defining the type before we try and stream the type on its
>       own.  Otherwise we can find ourselves trying to read in a
>       decl, when we're already partially reading in a component of
>       its type.  And that's bad.  */
> 
> This patch narrowly fixes this issue by special casing closure type
> declarations in add_decl_to_level.  (A loop is needed since there could
> be multiple variable declarations with an unprocessed initializer in
> light of structured bindings.)
> 
> 	PR c++/105322
> 
> gcc/cp/ChangeLog:
> 
> 	* name-lookup.cc (add_decl_to_level): When adding a closure
> 	type declaration to a block scope, add it before rather than
> 	after any variable declarations whose initializer we're still
> 	processing.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/lambda-5_a.C: New test.
> 	* g++.dg/modules/lambda-5_b.C: New test.
> ---
>   gcc/cp/name-lookup.cc                     | 19 ++++++++++++++++---
>   gcc/testsuite/g++.dg/modules/lambda-5_a.C | 23 +++++++++++++++++++++++
>   gcc/testsuite/g++.dg/modules/lambda-5_b.C | 10 ++++++++++
>   3 files changed, 49 insertions(+), 3 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_b.C
> 
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index a8b9229b29e..bb00baaf9f4 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -391,9 +391,22 @@ add_decl_to_level (cp_binding_level *b, tree decl)
>     gcc_assert (b->names != decl);
>   
>     /* We build up the list in reverse order, and reverse it later if
> -     necessary.  */
> -  TREE_CHAIN (decl) = b->names;
> -  b->names = decl;
> +     necessary.  If we're adding a lambda closure type to a block
> +     scope as part of a local variable initializer, then make sure
> +     we declare the type before the variable; modules expects that
> +     we see a type declaration before a use of the type.  */
> +  tree *prev = &b->names;
> +  if (b->kind == sk_block
> +      && !processing_template_decl
> +      && TREE_CODE (decl) == TYPE_DECL
> +      && LAMBDA_TYPE_P (TREE_TYPE (decl)))
> +    while (*prev && VAR_P (*prev)
> +	   && !DECL_EXTERNAL (*prev)
> +	   && !DECL_INITIALIZED_P (*prev))
> +      prev = &TREE_CHAIN (*prev);
> +
> +  TREE_CHAIN (decl) = *prev;
> +  *prev = decl;
>   
>     /* If appropriate, add decl to separate list of statics.  We include
>        extern variables because they might turn out to be static later.
> diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> new file mode 100644
> index 00000000000..6b54c8e3173
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> @@ -0,0 +1,23 @@
> +// PR c++/105322
> +// { dg-additional-options -fmodules-ts }
> +// { dg-module-cmi pr105322 }
> +
> +export module pr105322;
> +
> +struct A { };
> +
> +export
> +inline void f1() {
> +  A a;
> +  auto g1 = [a] { }; // used to ICE here during stream out
> +}
> +
> +export
> +template<class...>
> +void f2() {
> +  A a;
> +  auto g2 = [a] { };
> +}
> +
> +export
> +inline auto g3 = [a=A{}] { };
> diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_b.C b/gcc/testsuite/g++.dg/modules/lambda-5_b.C
> new file mode 100644
> index 00000000000..e25a913b726
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/lambda-5_b.C
> @@ -0,0 +1,10 @@
> +// PR c++/105322
> +// { dg-additional-options -fmodules-ts }
> +
> +import pr105322;
> +
> +int main() {
> +  f1();
> +  f2();
> +  g3();
> +}

-- 
Nathan Sidwell

[-- Attachment #2: 0001-c-Constructor-streaming-PR105322.patch --]
[-- Type: text/x-patch, Size: 5504 bytes --]

From fb65e961231943ecc68988f38a9ec78b0c93e5df Mon Sep 17 00:00:00 2001
From: Nathan Sidwell <nathan@acm.org>
Date: Fri, 20 Oct 2023 12:20:37 -0400
Subject: [PATCH] c++: Constructor streaming [PR105322]

An expresion node's type is streamed after the expression's operands,
because the type can come from some aspect of an operand (for instance
decltype and noexcept). There's a comment in the code explaining that.

But that doesn't work for constructors, which can directly reference
components of their type (eg FIELD_DECLS). If this is a
type-introducing CONSTRUCTOR, we need to ensure the type has been
streamed first. So move CONSTRUCTOR stream to after the type streaming.

The reason things like COMPONENT_REF work is that they stream their
first operand first, and that introduces the type that their second
operand looks up a field in.
---
 gcc/cp/module.cc                            | 58 ++++++++++++---------
 gcc/testsuite/g++.dg/modules/decltype-1_a.C | 28 ++++++++++
 gcc/testsuite/g++.dg/modules/decltype-1_b.C | 10 ++++
 gcc/testsuite/g++.dg/modules/lambda-5_a.C   | 24 +++++++++
 gcc/testsuite/g++.dg/modules/lambda-5_b.C   | 10 ++++
 5 files changed, 105 insertions(+), 25 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/decltype-1_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/decltype-1_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_b.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index bbb1e20b55b..539518d7923 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -6212,19 +6212,9 @@ trees_out::core_vals (tree t)
       break;
 
     case CONSTRUCTOR:
-      {
-	unsigned len = vec_safe_length (t->constructor.elts);
-	if (streaming_p ())
-	  WU (len);
-	if (len)
-	  for (unsigned ix = 0; ix != len; ix++)
-	    {
-	      const constructor_elt &elt = (*t->constructor.elts)[ix];
-
-	      WT (elt.index);
-	      WT (elt.value);
-	    }
-      }
+      // This must be streamed /after/ we've streamed the type,
+      // because it can directly refer to elements of the type. Eg,
+      // FIELD_DECLs of a RECORD_TYPE.
       break;
 
     case OMP_CLAUSE:
@@ -6458,6 +6448,21 @@ trees_out::core_vals (tree t)
 	WU (prec);
     }
 
+  if (TREE_CODE (t) == CONSTRUCTOR)
+    {
+      unsigned len = vec_safe_length (t->constructor.elts);
+      if (streaming_p ())
+	WU (len);
+      if (len)
+	for (unsigned ix = 0; ix != len; ix++)
+	  {
+	    const constructor_elt &elt = (*t->constructor.elts)[ix];
+
+	    WT (elt.index);
+	    WT (elt.value);
+	  }
+    }
+
 #undef WT
 #undef WU
 }
@@ -6717,18 +6722,7 @@ trees_in::core_vals (tree t)
       break;
 
     case CONSTRUCTOR:
-      if (unsigned len = u ())
-	{
-	  vec_alloc (t->constructor.elts, len);
-	  for (unsigned ix = 0; ix != len; ix++)
-	    {
-	      constructor_elt elt;
-
-	      RT (elt.index);
-	      RTU (elt.value);
-	      t->constructor.elts->quick_push (elt);
-	    }
-	}
+      // Streamed after the node's type.
       break;
 
     case OMP_CLAUSE:
@@ -6901,6 +6895,20 @@ trees_in::core_vals (tree t)
 	t->typed.type = type;
     }
 
+  if (TREE_CODE (t) == CONSTRUCTOR)
+    if (unsigned len = u ())
+      {
+	vec_alloc (t->constructor.elts, len);
+	for (unsigned ix = 0; ix != len; ix++)
+	  {
+	    constructor_elt elt;
+
+	    RT (elt.index);
+	    RTU (elt.value);
+	    t->constructor.elts->quick_push (elt);
+	  }
+      }
+
 #undef RT
 #undef RM
 #undef RU
diff --git a/gcc/testsuite/g++.dg/modules/decltype-1_a.C b/gcc/testsuite/g++.dg/modules/decltype-1_a.C
new file mode 100644
index 00000000000..ca66e8b598a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/decltype-1_a.C
@@ -0,0 +1,28 @@
+// PR c++/105322
+// { dg-module-do link
+// { dg-additional-options -fmodules-ts }
+// { dg-module-cmi pr105322.Decltype }
+
+export module pr105322.Decltype;
+
+auto f() {
+  struct A { int m;
+    int get () { return m; }
+  };
+  return A{};
+}
+
+export
+inline void g1() {
+  auto r = decltype(f()){0};
+}
+
+export
+inline void g2() {
+  auto r = f().m;
+}
+
+export
+inline void g3() {
+  auto r = f().get();
+}
diff --git a/gcc/testsuite/g++.dg/modules/decltype-1_b.C b/gcc/testsuite/g++.dg/modules/decltype-1_b.C
new file mode 100644
index 00000000000..6bebe13604f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/decltype-1_b.C
@@ -0,0 +1,10 @@
+// PR c++/105322
+// { dg-additional-options -fmodules-ts }
+
+import pr105322.Decltype;
+
+int main() {
+  g1();
+  g2();
+  g3();
+}
diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
new file mode 100644
index 00000000000..6b589d4965c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
@@ -0,0 +1,24 @@
+// PR c++/105322
+// { dg-module-do link
+// { dg-additional-options -fmodules-ts }
+// { dg-module-cmi pr105322.Lambda }
+
+export module pr105322.Lambda;
+
+struct A { };
+
+export
+inline void f1() {
+  A a;
+  auto g1 = [a] { }; // used to ICE here during stream out
+}
+
+export
+template<class...>
+void f2() {
+  A a;
+  auto g2 = [a] { };
+}
+
+export
+inline auto g3 = [a=A{}] { };
diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_b.C b/gcc/testsuite/g++.dg/modules/lambda-5_b.C
new file mode 100644
index 00000000000..a7ce7098835
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-5_b.C
@@ -0,0 +1,10 @@
+// PR c++/105322
+// { dg-additional-options -fmodules-ts }
+
+import pr105322.Lambda;
+
+int main() {
+  f1();
+  f2();
+  g3();
+}
-- 
2.41.0

Re: [PATCH] c++/modules: ICE with lambda initializing local var [PR105322]

[Patrick Palka]

On Fri, 20 Oct 2023, Nathan Sidwell wrote:

> Thanks for looking at this, but your patch is essentially papering over the
> problem.
> 
> It took me a while to figure out, but the clue was that things like
> 'decltype(f()).m' worked, but 'decltype(f()){0}' did not.  The CONSTRUCTOR
> node is the exception to the rule that required an expression node's type to
> be streamed after the node's operands.  We want the opposite for CTORS.
> 
> I'll commit this once bootstrapped.

Very interesting, thanks a lot!  It's awesome that both testcases could
be fixed simultaneously after all :)

> 
> nathan
> 
> On 10/18/23 12:28, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk?
> > 
> > -- >8 --
> > 
> > For a local variable initialized by a lambda:
> > 
> >    auto f = []{};
> > 
> > The corresponding BLOCK_VARS contains the variable declaration first,
> > followed by the closure type declaration, consistent with the
> > syntactical order.  This however means that a use of the closure type
> > appears (in the variable type/initializer) before the declaration of the
> > type.  This ends up causing an ICE when streaming the BLOCK_VARS of f1
> > below because we stream (by value) the CONSTRUCTOR initializer of g1 --
> > which contains components of the closure type -- before we've streamed
> > the declaration defining the closure type.  The following comment in
> > module.cc seems relevant:
> > 
> >    /* We want to stream the type of a expression-like nodes /after/
> >       we've streamed the operands.  The type often contains (bits
> >       of the) types of the operands, and with things like decltype
> >       and noexcept in play, we really want to stream the decls
> >       defining the type before we try and stream the type on its
> >       own.  Otherwise we can find ourselves trying to read in a
> >       decl, when we're already partially reading in a component of
> >       its type.  And that's bad.  */
> > 
> > This patch narrowly fixes this issue by special casing closure type
> > declarations in add_decl_to_level.  (A loop is needed since there could
> > be multiple variable declarations with an unprocessed initializer in
> > light of structured bindings.)
> > 
> > 	PR c++/105322
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* name-lookup.cc (add_decl_to_level): When adding a closure
> > 	type declaration to a block scope, add it before rather than
> > 	after any variable declarations whose initializer we're still
> > 	processing.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/modules/lambda-5_a.C: New test.
> > 	* g++.dg/modules/lambda-5_b.C: New test.
> > ---
> >   gcc/cp/name-lookup.cc                     | 19 ++++++++++++++++---
> >   gcc/testsuite/g++.dg/modules/lambda-5_a.C | 23 +++++++++++++++++++++++
> >   gcc/testsuite/g++.dg/modules/lambda-5_b.C | 10 ++++++++++
> >   3 files changed, 49 insertions(+), 3 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_a.C
> >   create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_b.C
> > 
> > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> > index a8b9229b29e..bb00baaf9f4 100644
> > --- a/gcc/cp/name-lookup.cc
> > +++ b/gcc/cp/name-lookup.cc
> > @@ -391,9 +391,22 @@ add_decl_to_level (cp_binding_level *b, tree decl)
> >     gcc_assert (b->names != decl);
> >       /* We build up the list in reverse order, and reverse it later if
> > -     necessary.  */
> > -  TREE_CHAIN (decl) = b->names;
> > -  b->names = decl;
> > +     necessary.  If we're adding a lambda closure type to a block
> > +     scope as part of a local variable initializer, then make sure
> > +     we declare the type before the variable; modules expects that
> > +     we see a type declaration before a use of the type.  */
> > +  tree *prev = &b->names;
> > +  if (b->kind == sk_block
> > +      && !processing_template_decl
> > +      && TREE_CODE (decl) == TYPE_DECL
> > +      && LAMBDA_TYPE_P (TREE_TYPE (decl)))
> > +    while (*prev && VAR_P (*prev)
> > +	   && !DECL_EXTERNAL (*prev)
> > +	   && !DECL_INITIALIZED_P (*prev))
> > +      prev = &TREE_CHAIN (*prev);
> > +
> > +  TREE_CHAIN (decl) = *prev;
> > +  *prev = decl;
> >       /* If appropriate, add decl to separate list of statics.  We include
> >        extern variables because they might turn out to be static later.
> > diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> > b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> > new file mode 100644
> > index 00000000000..6b54c8e3173
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> > @@ -0,0 +1,23 @@
> > +// PR c++/105322
> > +// { dg-additional-options -fmodules-ts }
> > +// { dg-module-cmi pr105322 }
> > +
> > +export module pr105322;
> > +
> > +struct A { };
> > +
> > +export
> > +inline void f1() {
> > +  A a;
> > +  auto g1 = [a] { }; // used to ICE here during stream out
> > +}
> > +
> > +export
> > +template<class...>
> > +void f2() {
> > +  A a;
> > +  auto g2 = [a] { };
> > +}
> > +
> > +export
> > +inline auto g3 = [a=A{}] { };
> > diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_b.C
> > b/gcc/testsuite/g++.dg/modules/lambda-5_b.C
> > new file mode 100644
> > index 00000000000..e25a913b726
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/lambda-5_b.C
> > @@ -0,0 +1,10 @@
> > +// PR c++/105322
> > +// { dg-additional-options -fmodules-ts }
> > +
> > +import pr105322;
> > +
> > +int main() {
> > +  f1();
> > +  f2();
> > +  g3();
> > +}
> 
> -- 
> Nathan Sidwell
>