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
>
>
