diff mbox series

c++ modules: variable template partial spec fixes [PR107033]

Message ID 20220926143620.24037-1-ppalka@redhat.com
State New
Headers show
Series c++ modules: variable template partial spec fixes [PR107033] | expand

Commit Message

Patrick Palka Sept. 26, 2022, 2:36 p.m. UTC
In r13-2775-g32d8123cd6ce87 I overlooked that we need to adjust the
call to add_mergeable_specialization in the MK_partial case to correctly
handle variable template partial specializations (it currently assumes
we're always dealing with one for a class template).  This fixes an ICE
when converting the testcase from that commit to use an importable
header instead of a named module.

Tested on x86_64-pc-linux-gnu, does this look OK for trunk?

	PR c++/107033

gcc/cp/ChangeLog:

	* module.cc (trees_in::decl_value): In the MK_partial case
	for a variable template partial specialization, pass decl_p=true
	to add_mergeable_specialization and set spec to the VAR_DECL
	not the TEMPLATE_DECL.
	* pt.cc (add_mergeable_specialization): For a variable template
	partial specialization, set the TREE_TYPE of the new
	DECL_TEMPLATE_SPECIALIZATIONS node to the TREE_TYPE of the
	VAR_DECL not the VAR_DECL itself.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/partial-2.cc, g++.dg/modules/partial-2.h: New
	files, factored out from ...
	* g++.dg/modules/partial-2_a.C, g++.dg/modules/partial-2_b.C: ...
	here.
	* g++.dg/modules/partial-2_c.H: New test.
	* g++.dg/modules/partial-2_d.C: New test.
---
 gcc/cp/module.cc                           | 17 ++++++----
 gcc/cp/pt.cc                               |  2 +-
 gcc/testsuite/g++.dg/modules/partial-2.cc  | 17 ++++++++++
 gcc/testsuite/g++.dg/modules/partial-2.h   | 38 +++++++++++++++++++++
 gcc/testsuite/g++.dg/modules/partial-2_a.C | 39 +---------------------
 gcc/testsuite/g++.dg/modules/partial-2_b.C | 18 +---------
 gcc/testsuite/g++.dg/modules/partial-2_c.H |  5 +++
 gcc/testsuite/g++.dg/modules/partial-2_d.C |  8 +++++
 8 files changed, 82 insertions(+), 62 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/partial-2.cc
 create mode 100644 gcc/testsuite/g++.dg/modules/partial-2.h
 create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_c.H
 create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_d.C

Comments

Nathan Sidwell Sept. 26, 2022, 2:47 p.m. UTC | #1
On 9/26/22 10:36, Patrick Palka wrote:
> In r13-2775-g32d8123cd6ce87 I overlooked that we need to adjust the
> call to add_mergeable_specialization in the MK_partial case to correctly
> handle variable template partial specializations (it currently assumes
> we're always dealing with one for a class template).  This fixes an ICE
> when converting the testcase from that commit to use an importable
> header instead of a named module.


looks good, thanks

> 
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> 
> 	PR c++/107033
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (trees_in::decl_value): In the MK_partial case
> 	for a variable template partial specialization, pass decl_p=true
> 	to add_mergeable_specialization and set spec to the VAR_DECL
> 	not the TEMPLATE_DECL.
> 	* pt.cc (add_mergeable_specialization): For a variable template
> 	partial specialization, set the TREE_TYPE of the new
> 	DECL_TEMPLATE_SPECIALIZATIONS node to the TREE_TYPE of the
> 	VAR_DECL not the VAR_DECL itself.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/partial-2.cc, g++.dg/modules/partial-2.h: New
> 	files, factored out from ...
> 	* g++.dg/modules/partial-2_a.C, g++.dg/modules/partial-2_b.C: ...
> 	here.
> 	* g++.dg/modules/partial-2_c.H: New test.
> 	* g++.dg/modules/partial-2_d.C: New test.
> ---
>   gcc/cp/module.cc                           | 17 ++++++----
>   gcc/cp/pt.cc                               |  2 +-
>   gcc/testsuite/g++.dg/modules/partial-2.cc  | 17 ++++++++++
>   gcc/testsuite/g++.dg/modules/partial-2.h   | 38 +++++++++++++++++++++
>   gcc/testsuite/g++.dg/modules/partial-2_a.C | 39 +---------------------
>   gcc/testsuite/g++.dg/modules/partial-2_b.C | 18 +---------
>   gcc/testsuite/g++.dg/modules/partial-2_c.H |  5 +++
>   gcc/testsuite/g++.dg/modules/partial-2_d.C |  8 +++++
>   8 files changed, 82 insertions(+), 62 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/partial-2.cc
>   create mode 100644 gcc/testsuite/g++.dg/modules/partial-2.h
>   create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_c.H
>   create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_d.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index f23832cb56a..7496df5e843 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -8185,13 +8185,18 @@ trees_in::decl_value ()
>   	/* Set the TEMPLATE_DECL's type.  */
>   	TREE_TYPE (decl) = TREE_TYPE (inner);
>   
> -      if (mk & MK_template_mask
> -	  || mk == MK_partial)
> +      /* Add to specialization tables now that constraints etc are
> +	 added.  */
> +      if (mk == MK_partial)
>   	{
> -	  /* Add to specialization tables now that constraints etc are
> -	     added.  */
> -	  bool is_type = mk == MK_partial || !(mk & MK_tmpl_decl_mask);
> -
> +	  bool is_type = TREE_CODE (inner) == TYPE_DECL;
> +	  spec.spec = is_type ? type : inner;
> +	  add_mergeable_specialization (!is_type, false,
> +					&spec, decl, spec_flags);
> +	}
> +      else if (mk & MK_template_mask)
> +	{
> +	  bool is_type = !(mk & MK_tmpl_decl_mask);
>   	  spec.spec = is_type ? type : mk & MK_tmpl_tmpl_mask ? inner : decl;
>   	  add_mergeable_specialization (!is_type,
>   					!is_type && mk & MK_tmpl_alias_mask,
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index db4e808adec..1f088fe281e 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -31010,7 +31010,7 @@ add_mergeable_specialization (bool decl_p, bool alias_p, spec_entry *elt,
>         /* A partial specialization.  */
>         tree cons = tree_cons (elt->args, decl,
>   			     DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl));
> -      TREE_TYPE (cons) = elt->spec;
> +      TREE_TYPE (cons) = decl_p ? TREE_TYPE (elt->spec) : elt->spec;
>         DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl) = cons;
>       }
>   }
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2.cc b/gcc/testsuite/g++.dg/modules/partial-2.cc
> new file mode 100644
> index 00000000000..1316bf5e1c5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-2.cc
> @@ -0,0 +1,17 @@
> +static_assert(is_reference_v<int&>);
> +static_assert(is_reference_v<int&&>);
> +static_assert(!is_reference_v<int>);
> +
> +static_assert(A::is_reference_v<long&>);
> +static_assert(A::is_reference_v<long&&>);
> +static_assert(!A::is_reference_v<long>);
> +
> +#if __cpp_concepts
> +static_assert(concepts::is_reference_v<char&>);
> +static_assert(concepts::is_reference_v<char&&>);
> +static_assert(!concepts::is_reference_v<char>);
> +
> +static_assert(concepts::A::is_reference_v<bool&>);
> +static_assert(concepts::A::is_reference_v<bool&&>);
> +static_assert(!concepts::A::is_reference_v<bool>);
> +#endif
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2.h b/gcc/testsuite/g++.dg/modules/partial-2.h
> new file mode 100644
> index 00000000000..afcfce791b3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-2.h
> @@ -0,0 +1,38 @@
> +template<class T> constexpr bool is_reference_v = false;
> +template<class T> constexpr bool is_reference_v<T&> = true;
> +template<class T> constexpr bool is_reference_v<T&&> = true;
> +
> +struct A {
> +  template<class T> static constexpr bool is_reference_v = false;
> +};
> +
> +template<class T> constexpr bool A::is_reference_v<T&> = true;
> +template<class T> constexpr bool A::is_reference_v<T&&> = true;
> +
> +#if __cpp_concepts
> +namespace concepts {
> +  template<class T> bool is_reference_v;
> +
> +  template<class T> requires __is_same(T, T&)
> +  constexpr bool is_reference_v<T> = true;
> +
> +  template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
> +  constexpr bool is_reference_v<T> = true;
> +
> +  template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
> +  constexpr bool is_reference_v<T> = false;
> +
> +  struct A {
> +    template<class T> static bool is_reference_v;
> +  };
> +
> +  template<class T> requires __is_same(T, T&)
> +  constexpr bool A::is_reference_v<T> = true;
> +
> +  template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
> +  constexpr bool A::is_reference_v<T> = true;
> +
> +  template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
> +  constexpr bool A::is_reference_v<T> = false;
> +}
> +#endif
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2_a.C b/gcc/testsuite/g++.dg/modules/partial-2_a.C
> index 2681bb59ce8..1582f56f2d4 100644
> --- a/gcc/testsuite/g++.dg/modules/partial-2_a.C
> +++ b/gcc/testsuite/g++.dg/modules/partial-2_a.C
> @@ -3,41 +3,4 @@
>   // { dg-module-cmi pr106826 }
>   export module pr106826;
>   
> -template<class T> constexpr bool is_reference_v = false;
> -template<class T> constexpr bool is_reference_v<T&> = true;
> -template<class T> constexpr bool is_reference_v<T&&> = true;
> -
> -struct A {
> -  template<class T> static constexpr bool is_reference_v = false;
> -};
> -
> -template<class T> constexpr bool A::is_reference_v<T&> = true;
> -template<class T> constexpr bool A::is_reference_v<T&&> = true;
> -
> -#if __cpp_concepts
> -namespace concepts {
> -  template<class T> bool is_reference_v;
> -
> -  template<class T> requires __is_same(T, T&)
> -  constexpr bool is_reference_v<T> = true;
> -
> -  template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
> -  constexpr bool is_reference_v<T> = true;
> -
> -  template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
> -  constexpr bool is_reference_v<T> = false;
> -
> -  struct A {
> -    template<class T> static bool is_reference_v;
> -  };
> -
> -  template<class T> requires __is_same(T, T&)
> -  constexpr bool A::is_reference_v<T> = true;
> -
> -  template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
> -  constexpr bool A::is_reference_v<T> = true;
> -
> -  template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
> -  constexpr bool A::is_reference_v<T> = false;
> -}
> -#endif
> +#include "partial-2.h"
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2_b.C b/gcc/testsuite/g++.dg/modules/partial-2_b.C
> index 0af41ef5e5e..1b0c7a53e9f 100644
> --- a/gcc/testsuite/g++.dg/modules/partial-2_b.C
> +++ b/gcc/testsuite/g++.dg/modules/partial-2_b.C
> @@ -2,20 +2,4 @@
>   // { dg-additional-options -fmodules-ts }
>   module pr106826;
>   
> -static_assert(is_reference_v<int&>);
> -static_assert(is_reference_v<int&&>);
> -static_assert(!is_reference_v<int>);
> -
> -static_assert(A::is_reference_v<long&>);
> -static_assert(A::is_reference_v<long&&>);
> -static_assert(!A::is_reference_v<long>);
> -
> -#if __cpp_concepts
> -static_assert(concepts::is_reference_v<char&>);
> -static_assert(concepts::is_reference_v<char&&>);
> -static_assert(!concepts::is_reference_v<char>);
> -
> -static_assert(concepts::A::is_reference_v<bool&>);
> -static_assert(concepts::A::is_reference_v<bool&&>);
> -static_assert(!concepts::A::is_reference_v<bool>);
> -#endif
> +#include "partial-2.cc"
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2_c.H b/gcc/testsuite/g++.dg/modules/partial-2_c.H
> new file mode 100644
> index 00000000000..bd838529ce0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-2_c.H
> @@ -0,0 +1,5 @@
> +// PR c++/107033
> +// { dg-additional-options -fmodule-header }
> +// { dg-module-cmi {} }
> +
> +#include "partial-2.h"
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2_d.C b/gcc/testsuite/g++.dg/modules/partial-2_d.C
> new file mode 100644
> index 00000000000..ed54d3c2884
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-2_d.C
> @@ -0,0 +1,8 @@
> +// PR c++/107033
> +// { dg-additional-options -fmodules-ts }
> +// { dg-module-cmi pr107033 }
> +export module pr107033;
> +
> +import "partial-2_c.H";
> +
> +#include "partial-2.cc"
diff mbox series

Patch

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f23832cb56a..7496df5e843 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -8185,13 +8185,18 @@  trees_in::decl_value ()
 	/* Set the TEMPLATE_DECL's type.  */
 	TREE_TYPE (decl) = TREE_TYPE (inner);
 
-      if (mk & MK_template_mask
-	  || mk == MK_partial)
+      /* Add to specialization tables now that constraints etc are
+	 added.  */
+      if (mk == MK_partial)
 	{
-	  /* Add to specialization tables now that constraints etc are
-	     added.  */
-	  bool is_type = mk == MK_partial || !(mk & MK_tmpl_decl_mask);
-
+	  bool is_type = TREE_CODE (inner) == TYPE_DECL;
+	  spec.spec = is_type ? type : inner;
+	  add_mergeable_specialization (!is_type, false,
+					&spec, decl, spec_flags);
+	}
+      else if (mk & MK_template_mask)
+	{
+	  bool is_type = !(mk & MK_tmpl_decl_mask);
 	  spec.spec = is_type ? type : mk & MK_tmpl_tmpl_mask ? inner : decl;
 	  add_mergeable_specialization (!is_type,
 					!is_type && mk & MK_tmpl_alias_mask,
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index db4e808adec..1f088fe281e 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -31010,7 +31010,7 @@  add_mergeable_specialization (bool decl_p, bool alias_p, spec_entry *elt,
       /* A partial specialization.  */
       tree cons = tree_cons (elt->args, decl,
 			     DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl));
-      TREE_TYPE (cons) = elt->spec;
+      TREE_TYPE (cons) = decl_p ? TREE_TYPE (elt->spec) : elt->spec;
       DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl) = cons;
     }
 }
diff --git a/gcc/testsuite/g++.dg/modules/partial-2.cc b/gcc/testsuite/g++.dg/modules/partial-2.cc
new file mode 100644
index 00000000000..1316bf5e1c5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/partial-2.cc
@@ -0,0 +1,17 @@ 
+static_assert(is_reference_v<int&>);
+static_assert(is_reference_v<int&&>);
+static_assert(!is_reference_v<int>);
+
+static_assert(A::is_reference_v<long&>);
+static_assert(A::is_reference_v<long&&>);
+static_assert(!A::is_reference_v<long>);
+
+#if __cpp_concepts
+static_assert(concepts::is_reference_v<char&>);
+static_assert(concepts::is_reference_v<char&&>);
+static_assert(!concepts::is_reference_v<char>);
+
+static_assert(concepts::A::is_reference_v<bool&>);
+static_assert(concepts::A::is_reference_v<bool&&>);
+static_assert(!concepts::A::is_reference_v<bool>);
+#endif
diff --git a/gcc/testsuite/g++.dg/modules/partial-2.h b/gcc/testsuite/g++.dg/modules/partial-2.h
new file mode 100644
index 00000000000..afcfce791b3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/partial-2.h
@@ -0,0 +1,38 @@ 
+template<class T> constexpr bool is_reference_v = false;
+template<class T> constexpr bool is_reference_v<T&> = true;
+template<class T> constexpr bool is_reference_v<T&&> = true;
+
+struct A {
+  template<class T> static constexpr bool is_reference_v = false;
+};
+
+template<class T> constexpr bool A::is_reference_v<T&> = true;
+template<class T> constexpr bool A::is_reference_v<T&&> = true;
+
+#if __cpp_concepts
+namespace concepts {
+  template<class T> bool is_reference_v;
+
+  template<class T> requires __is_same(T, T&)
+  constexpr bool is_reference_v<T> = true;
+
+  template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
+  constexpr bool is_reference_v<T> = true;
+
+  template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
+  constexpr bool is_reference_v<T> = false;
+
+  struct A {
+    template<class T> static bool is_reference_v;
+  };
+
+  template<class T> requires __is_same(T, T&)
+  constexpr bool A::is_reference_v<T> = true;
+
+  template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
+  constexpr bool A::is_reference_v<T> = true;
+
+  template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
+  constexpr bool A::is_reference_v<T> = false;
+}
+#endif
diff --git a/gcc/testsuite/g++.dg/modules/partial-2_a.C b/gcc/testsuite/g++.dg/modules/partial-2_a.C
index 2681bb59ce8..1582f56f2d4 100644
--- a/gcc/testsuite/g++.dg/modules/partial-2_a.C
+++ b/gcc/testsuite/g++.dg/modules/partial-2_a.C
@@ -3,41 +3,4 @@ 
 // { dg-module-cmi pr106826 }
 export module pr106826;
 
-template<class T> constexpr bool is_reference_v = false;
-template<class T> constexpr bool is_reference_v<T&> = true;
-template<class T> constexpr bool is_reference_v<T&&> = true;
-
-struct A {
-  template<class T> static constexpr bool is_reference_v = false;
-};
-
-template<class T> constexpr bool A::is_reference_v<T&> = true;
-template<class T> constexpr bool A::is_reference_v<T&&> = true;
-
-#if __cpp_concepts
-namespace concepts {
-  template<class T> bool is_reference_v;
-
-  template<class T> requires __is_same(T, T&)
-  constexpr bool is_reference_v<T> = true;
-
-  template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
-  constexpr bool is_reference_v<T> = true;
-
-  template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
-  constexpr bool is_reference_v<T> = false;
-
-  struct A {
-    template<class T> static bool is_reference_v;
-  };
-
-  template<class T> requires __is_same(T, T&)
-  constexpr bool A::is_reference_v<T> = true;
-
-  template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
-  constexpr bool A::is_reference_v<T> = true;
-
-  template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
-  constexpr bool A::is_reference_v<T> = false;
-}
-#endif
+#include "partial-2.h"
diff --git a/gcc/testsuite/g++.dg/modules/partial-2_b.C b/gcc/testsuite/g++.dg/modules/partial-2_b.C
index 0af41ef5e5e..1b0c7a53e9f 100644
--- a/gcc/testsuite/g++.dg/modules/partial-2_b.C
+++ b/gcc/testsuite/g++.dg/modules/partial-2_b.C
@@ -2,20 +2,4 @@ 
 // { dg-additional-options -fmodules-ts }
 module pr106826;
 
-static_assert(is_reference_v<int&>);
-static_assert(is_reference_v<int&&>);
-static_assert(!is_reference_v<int>);
-
-static_assert(A::is_reference_v<long&>);
-static_assert(A::is_reference_v<long&&>);
-static_assert(!A::is_reference_v<long>);
-
-#if __cpp_concepts
-static_assert(concepts::is_reference_v<char&>);
-static_assert(concepts::is_reference_v<char&&>);
-static_assert(!concepts::is_reference_v<char>);
-
-static_assert(concepts::A::is_reference_v<bool&>);
-static_assert(concepts::A::is_reference_v<bool&&>);
-static_assert(!concepts::A::is_reference_v<bool>);
-#endif
+#include "partial-2.cc"
diff --git a/gcc/testsuite/g++.dg/modules/partial-2_c.H b/gcc/testsuite/g++.dg/modules/partial-2_c.H
new file mode 100644
index 00000000000..bd838529ce0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/partial-2_c.H
@@ -0,0 +1,5 @@ 
+// PR c++/107033
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+#include "partial-2.h"
diff --git a/gcc/testsuite/g++.dg/modules/partial-2_d.C b/gcc/testsuite/g++.dg/modules/partial-2_d.C
new file mode 100644
index 00000000000..ed54d3c2884
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/partial-2_d.C
@@ -0,0 +1,8 @@ 
+// PR c++/107033
+// { dg-additional-options -fmodules-ts }
+// { dg-module-cmi pr107033 }
+export module pr107033;
+
+import "partial-2_c.H";
+
+#include "partial-2.cc"