diff mbox series

[v2,3/3] c++/modules: Prevent ICE when writing class-scope lambdas without mangling scope [PR116568]

Message ID 672f1d48.170a0220.4887.9d62@mx.google.com
State New
Headers show
Series [v2,1/3] c++: Fix mangling of otherwise unattached class-scope lambdas [PR107741] | expand

Commit Message

Nathaniel Shead Nov. 9, 2024, 8:28 a.m. UTC
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

Alternatively, after this I'll work on an update of my P1815 (TU-local
entities) patch series [1] which would also solve this ICE by erroring
early due to attempting to emit a TU-local entity.  As discussed in
patch #1 I believe this is incorrect but also seems out of scope of this
particular modification.  I still think this patch would be useful
however, if only for the clarification of the behaviour of streaming
lambdas without an extra mangling scope.

[1]: https://gcc.gnu.org/pipermail/gcc-patches/2024-October/665108.html

-- >8 --

If a lambda doesn't have an extra mangling scope despite being in class
scope (such as associated with a type alias, currently), we shouldn't
ICE; this behaviour is fine as long as we don't need to merge.

In the future such lambdas should be rejected as being TU-local, but for
now this at least clarifies the intended behaviour if this edge case
occurs again.

	PR c++/116568

gcc/cp/ChangeLog:

	* module.cc (trees_out::get_merge_kind): All lambdas without
	extra mangling scope now marked MK_unique.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/lambda-8.h: New test.
	* g++.dg/modules/lambda-8_a.H: New test.
	* g++.dg/modules/lambda-8_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                          | 30 ++++++++++++++---------
 gcc/testsuite/g++.dg/modules/lambda-8.h   |  7 ++++++
 gcc/testsuite/g++.dg/modules/lambda-8_a.H |  5 ++++
 gcc/testsuite/g++.dg/modules/lambda-8_b.C |  5 ++++
 4 files changed, 35 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/lambda-8.h
 create mode 100644 gcc/testsuite/g++.dg/modules/lambda-8_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/lambda-8_b.C

Comments

Jason Merrill Nov. 21, 2024, 6:52 p.m. UTC | #1
On 11/9/24 9:28 AM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK.

> Alternatively, after this I'll work on an update of my P1815 (TU-local
> entities) patch series [1] which would also solve this ICE by erroring
> early due to attempting to emit a TU-local entity.  As discussed in
> patch #1 I believe this is incorrect but also seems out of scope of this
> particular modification.  I still think this patch would be useful
> however, if only for the clarification of the behaviour of streaming
> lambdas without an extra mangling scope.
> 
> [1]: https://gcc.gnu.org/pipermail/gcc-patches/2024-October/665108.html
> 
> -- >8 --
> 
> If a lambda doesn't have an extra mangling scope despite being in class
> scope (such as associated with a type alias, currently), we shouldn't
> ICE; this behaviour is fine as long as we don't need to merge.
> 
> In the future such lambdas should be rejected as being TU-local, but for
> now this at least clarifies the intended behaviour if this edge case
> occurs again.
> 
> 	PR c++/116568
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (trees_out::get_merge_kind): All lambdas without
> 	extra mangling scope now marked MK_unique.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/lambda-8.h: New test.
> 	* g++.dg/modules/lambda-8_a.H: New test.
> 	* g++.dg/modules/lambda-8_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/module.cc                          | 30 ++++++++++++++---------
>   gcc/testsuite/g++.dg/modules/lambda-8.h   |  7 ++++++
>   gcc/testsuite/g++.dg/modules/lambda-8_a.H |  5 ++++
>   gcc/testsuite/g++.dg/modules/lambda-8_b.C |  5 ++++
>   4 files changed, 35 insertions(+), 12 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/lambda-8.h
>   create mode 100644 gcc/testsuite/g++.dg/modules/lambda-8_a.H
>   create mode 100644 gcc/testsuite/g++.dg/modules/lambda-8_b.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 4eefb2d3584..877e442bc41 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -10737,18 +10737,24 @@ trees_out::get_merge_kind (tree decl, depset *dep)
>   	       g++.dg/modules/lambda-6_a.C.  */
>   	    if (DECL_IMPLICIT_TYPEDEF_P (STRIP_TEMPLATE (decl))
>   		&& LAMBDA_TYPE_P (TREE_TYPE (decl)))
> -	      if (tree scope = LAMBDA_TYPE_EXTRA_SCOPE (TREE_TYPE (decl)))
> -		{
> -		  /* Lambdas attached to fields are keyed to its class.  */
> -		  if (TREE_CODE (scope) == FIELD_DECL)
> -		    scope = TYPE_NAME (DECL_CONTEXT (scope));
> -		  if (DECL_LANG_SPECIFIC (scope)
> -		      && DECL_MODULE_KEYED_DECLS_P (scope))
> -		    {
> -		      mk = MK_keyed;
> -		      break;
> -		    }
> -		}
> +	      {
> +		if (tree scope = LAMBDA_TYPE_EXTRA_SCOPE (TREE_TYPE (decl)))
> +		  {
> +		    /* Lambdas attached to fields are keyed to its class.  */
> +		    if (TREE_CODE (scope) == FIELD_DECL)
> +		      scope = TYPE_NAME (DECL_CONTEXT (scope));
> +		    if (DECL_LANG_SPECIFIC (scope)
> +			&& DECL_MODULE_KEYED_DECLS_P (scope))
> +		      {
> +			mk = MK_keyed;
> +			break;
> +		      }
> +		  }
> +		/* Lambdas without extra mangling scope are unique to the TU,
> +		   and cannot be merged; make them unique.  */
> +		mk = MK_unique;
> +		break;
> +	      }
>   
>   	    if (TREE_CODE (decl) == TEMPLATE_DECL
>   		&& DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl))
> diff --git a/gcc/testsuite/g++.dg/modules/lambda-8.h b/gcc/testsuite/g++.dg/modules/lambda-8.h
> new file mode 100644
> index 00000000000..0c66f053b20
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/lambda-8.h
> @@ -0,0 +1,7 @@
> +template <typename> struct S {
> +  template <typename> static constexpr auto x = []{};
> +  template <typename> using t = decltype([]{});
> +};
> +
> +inline auto x = S<int>::x<int>;
> +using t = S<int>::t<int>;
> diff --git a/gcc/testsuite/g++.dg/modules/lambda-8_a.H b/gcc/testsuite/g++.dg/modules/lambda-8_a.H
> new file mode 100644
> index 00000000000..d20958ee140
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/lambda-8_a.H
> @@ -0,0 +1,5 @@
> +// PR c++/116568
> +// { dg-additional-options "-fmodules-ts -std=c++20" }
> +// { dg-module-cmi {} }
> +
> +#include "lambda-8.h"
> diff --git a/gcc/testsuite/g++.dg/modules/lambda-8_b.C b/gcc/testsuite/g++.dg/modules/lambda-8_b.C
> new file mode 100644
> index 00000000000..05ea4afd8c1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/lambda-8_b.C
> @@ -0,0 +1,5 @@
> +// PR c++/116568
> +// { dg-additional-options "-fmodules-ts -std=c++20" }
> +
> +#include "lambda-8.h"
> +import "lambda-8_a.H";
diff mbox series

Patch

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 4eefb2d3584..877e442bc41 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -10737,18 +10737,24 @@  trees_out::get_merge_kind (tree decl, depset *dep)
 	       g++.dg/modules/lambda-6_a.C.  */
 	    if (DECL_IMPLICIT_TYPEDEF_P (STRIP_TEMPLATE (decl))
 		&& LAMBDA_TYPE_P (TREE_TYPE (decl)))
-	      if (tree scope = LAMBDA_TYPE_EXTRA_SCOPE (TREE_TYPE (decl)))
-		{
-		  /* Lambdas attached to fields are keyed to its class.  */
-		  if (TREE_CODE (scope) == FIELD_DECL)
-		    scope = TYPE_NAME (DECL_CONTEXT (scope));
-		  if (DECL_LANG_SPECIFIC (scope)
-		      && DECL_MODULE_KEYED_DECLS_P (scope))
-		    {
-		      mk = MK_keyed;
-		      break;
-		    }
-		}
+	      {
+		if (tree scope = LAMBDA_TYPE_EXTRA_SCOPE (TREE_TYPE (decl)))
+		  {
+		    /* Lambdas attached to fields are keyed to its class.  */
+		    if (TREE_CODE (scope) == FIELD_DECL)
+		      scope = TYPE_NAME (DECL_CONTEXT (scope));
+		    if (DECL_LANG_SPECIFIC (scope)
+			&& DECL_MODULE_KEYED_DECLS_P (scope))
+		      {
+			mk = MK_keyed;
+			break;
+		      }
+		  }
+		/* Lambdas without extra mangling scope are unique to the TU,
+		   and cannot be merged; make them unique.  */
+		mk = MK_unique;
+		break;
+	      }
 
 	    if (TREE_CODE (decl) == TEMPLATE_DECL
 		&& DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl))
diff --git a/gcc/testsuite/g++.dg/modules/lambda-8.h b/gcc/testsuite/g++.dg/modules/lambda-8.h
new file mode 100644
index 00000000000..0c66f053b20
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-8.h
@@ -0,0 +1,7 @@ 
+template <typename> struct S {
+  template <typename> static constexpr auto x = []{};
+  template <typename> using t = decltype([]{});
+};
+
+inline auto x = S<int>::x<int>;
+using t = S<int>::t<int>;
diff --git a/gcc/testsuite/g++.dg/modules/lambda-8_a.H b/gcc/testsuite/g++.dg/modules/lambda-8_a.H
new file mode 100644
index 00000000000..d20958ee140
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-8_a.H
@@ -0,0 +1,5 @@ 
+// PR c++/116568
+// { dg-additional-options "-fmodules-ts -std=c++20" }
+// { dg-module-cmi {} }
+
+#include "lambda-8.h"
diff --git a/gcc/testsuite/g++.dg/modules/lambda-8_b.C b/gcc/testsuite/g++.dg/modules/lambda-8_b.C
new file mode 100644
index 00000000000..05ea4afd8c1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/lambda-8_b.C
@@ -0,0 +1,5 @@ 
+// PR c++/116568
+// { dg-additional-options "-fmodules-ts -std=c++20" }
+
+#include "lambda-8.h"
+import "lambda-8_a.H";