diff mbox series

c++/modules: Don't treat template parameters as TU-local [PR118846]

Message ID 67ac8ee9.a70a0220.97748.3eeb@mx.google.com
State Committed
Commit 9f1f4efc06f43b1ba8c1cf5a31d5b73d6a2bb12d
Headers
Series c++/modules: Don't treat template parameters as TU-local [PR118846] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed

Commit Message

Nathaniel Shead Feb. 12, 2025, 12:07 p.m. UTC 
  Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

-- >8 --

There are two separate issues making various template parameters behave
as if they were TU-local.

Firstly, the TU-local detection code uses WILDCARD_TYPE_P to check for
types that are not yet concrete; for some reason UNBOUND_CLASS_TEMPLATE
is not on that list.  I don't see any particular reason why it shouldn't
be, so this patch adds it; this may solve other latent issues as well.

Secondly, the TEMPLATE_DECL for a type with expressions involving
template template parameters is currently always constrained to internal
linkage, because the result does not have TREE_PUBLIC set.  Rather than
messing with TREE_PUBLIC here, I think rather we just should ensure that
we only attempt to constrain visiblity of templates of type, variable,
or function decls.

	PR c++/118846

gcc/cp/ChangeLog:

	* cp-tree.h (WILDCARD_TYPE_P): Include UNBOUND_CLASS_TEMPLATE.
	* decl2.cc (min_vis_expr_r): Don't rely on TREE_PUBLIC always
	being accurate for the DECL_TEMPLATE_RESULT of a template.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/pr118846_a.C: New test.
	* g++.dg/modules/pr118846_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/cp-tree.h                          |  1 +
 gcc/cp/decl2.cc                           | 22 ++++++++++++++--------
 gcc/testsuite/g++.dg/modules/pr118846_a.C | 18 ++++++++++++++++++
 gcc/testsuite/g++.dg/modules/pr118846_b.C | 10 ++++++++++
 4 files changed, 43 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/pr118846_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/pr118846_b.C

Comments

Jason Merrill Feb. 14, 2025, 10:09 p.m. UTC | #1 
On 2/12/25 1:07 PM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK.

> -- >8 --
> 
> There are two separate issues making various template parameters behave
> as if they were TU-local.
> 
> Firstly, the TU-local detection code uses WILDCARD_TYPE_P to check for
> types that are not yet concrete; for some reason UNBOUND_CLASS_TEMPLATE
> is not on that list.  I don't see any particular reason why it shouldn't
> be, so this patch adds it; this may solve other latent issues as well.
> 
> Secondly, the TEMPLATE_DECL for a type with expressions involving
> template template parameters is currently always constrained to internal
> linkage, because the result does not have TREE_PUBLIC set.  Rather than
> messing with TREE_PUBLIC here, I think rather we just should ensure that
> we only attempt to constrain visiblity of templates of type, variable,
> or function decls.
> 
> 	PR c++/118846
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (WILDCARD_TYPE_P): Include UNBOUND_CLASS_TEMPLATE.
> 	* decl2.cc (min_vis_expr_r): Don't rely on TREE_PUBLIC always
> 	being accurate for the DECL_TEMPLATE_RESULT of a template.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/pr118846_a.C: New test.
> 	* g++.dg/modules/pr118846_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/cp-tree.h                          |  1 +
>   gcc/cp/decl2.cc                           | 22 ++++++++++++++--------
>   gcc/testsuite/g++.dg/modules/pr118846_a.C | 18 ++++++++++++++++++
>   gcc/testsuite/g++.dg/modules/pr118846_b.C | 10 ++++++++++
>   4 files changed, 43 insertions(+), 8 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/pr118846_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/pr118846_b.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index ec976928f5f..3d0349f66a3 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -2325,6 +2325,7 @@ enum languages { lang_c, lang_cplusplus };
>      || TREE_CODE (T) == TYPENAME_TYPE			\
>      || TREE_CODE (T) == TYPEOF_TYPE			\
>      || TREE_CODE (T) == BOUND_TEMPLATE_TEMPLATE_PARM	\
> +   || TREE_CODE (T) == UNBOUND_CLASS_TEMPLATE		\
>      || TREE_CODE (T) == DECLTYPE_TYPE			\
>      || TREE_CODE (T) == TRAIT_TYPE			\
>      || TREE_CODE (T) == DEPENDENT_OPERATOR_TYPE		\
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index 593dcaa4e2d..4415cea93e0 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -2786,6 +2786,19 @@ min_vis_expr_r (tree *tp, int */*walk_subtrees*/, void *data)
>     tree t = *tp;
>     if (TREE_CODE (t) == PTRMEM_CST)
>       t = PTRMEM_CST_MEMBER (t);
> +
> +  if (TREE_CODE (t) == TEMPLATE_DECL)
> +    {
> +      if (DECL_ALIAS_TEMPLATE_P (t) || concept_definition_p (t))
> +	/* FIXME: We don't maintain TREE_PUBLIC / DECL_VISIBILITY for
> +	   alias templates so we can't trust it here (PR107906).  Ditto
> +	   for concepts.  */
> +	return NULL_TREE;
> +      t = DECL_TEMPLATE_RESULT (t);
> +      if (!t)
> +	return NULL_TREE;
> +    }
> +
>     switch (TREE_CODE (t))
>       {
>       case CAST_EXPR:
> @@ -2797,17 +2810,10 @@ min_vis_expr_r (tree *tp, int */*walk_subtrees*/, void *data)
>       case NEW_EXPR:
>       case CONSTRUCTOR:
>       case LAMBDA_EXPR:
> +    case TYPE_DECL:
>         tpvis = type_visibility (TREE_TYPE (t));
>         break;
>   
> -    case TEMPLATE_DECL:
> -      if (DECL_ALIAS_TEMPLATE_P (t) || concept_definition_p (t))
> -	/* FIXME: We don't maintain TREE_PUBLIC / DECL_VISIBILITY for
> -	   alias templates so we can't trust it here (PR107906).  Ditto
> -	   for concepts.  */
> -	break;
> -      t = DECL_TEMPLATE_RESULT (t);
> -      /* Fall through.  */
>       case VAR_DECL:
>       case FUNCTION_DECL:
>         if (decl_constant_var_p (t))
> diff --git a/gcc/testsuite/g++.dg/modules/pr118846_a.C b/gcc/testsuite/g++.dg/modules/pr118846_a.C
> new file mode 100644
> index 00000000000..bbbdde78457
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr118846_a.C
> @@ -0,0 +1,18 @@
> +// PR c++/118846
> +// { dg-additional-options "-fmodules" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +
> +template <int N> struct integral_constant { static constexpr int value = N; };
> +template <template <class> class> constexpr int cx_count_if() { return 0; }
> +template <template <class> class P> struct mp_count_if_impl {
> +  using type = integral_constant<cx_count_if<P>()>;
> +};
> +
> +template <template <class> class> struct consume {
> +  static constexpr bool value = true;
> +};
> +template <class P> struct use {
> +  using type = consume<P::template fn>;
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/pr118846_b.C b/gcc/testsuite/g++.dg/modules/pr118846_b.C
> new file mode 100644
> index 00000000000..a2f28894630
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr118846_b.C
> @@ -0,0 +1,10 @@
> +// PR c++/118846
> +// { dg-additional-options "-fmodules" }
> +
> +module M;
> +
> +template <typename> struct S {
> +  template <typename> struct fn {};
> +};
> +static_assert(mp_count_if_impl<S>::type::value == 0);
> +static_assert(use<S<int>>::type::value);
diff mbox series

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index ec976928f5f..3d0349f66a3 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -2325,6 +2325,7 @@  enum languages { lang_c, lang_cplusplus };
    || TREE_CODE (T) == TYPENAME_TYPE			\
    || TREE_CODE (T) == TYPEOF_TYPE			\
    || TREE_CODE (T) == BOUND_TEMPLATE_TEMPLATE_PARM	\
+   || TREE_CODE (T) == UNBOUND_CLASS_TEMPLATE		\
    || TREE_CODE (T) == DECLTYPE_TYPE			\
    || TREE_CODE (T) == TRAIT_TYPE			\
    || TREE_CODE (T) == DEPENDENT_OPERATOR_TYPE		\
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 593dcaa4e2d..4415cea93e0 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -2786,6 +2786,19 @@  min_vis_expr_r (tree *tp, int */*walk_subtrees*/, void *data)
   tree t = *tp;
   if (TREE_CODE (t) == PTRMEM_CST)
     t = PTRMEM_CST_MEMBER (t);
+
+  if (TREE_CODE (t) == TEMPLATE_DECL)
+    {
+      if (DECL_ALIAS_TEMPLATE_P (t) || concept_definition_p (t))
+	/* FIXME: We don't maintain TREE_PUBLIC / DECL_VISIBILITY for
+	   alias templates so we can't trust it here (PR107906).  Ditto
+	   for concepts.  */
+	return NULL_TREE;
+      t = DECL_TEMPLATE_RESULT (t);
+      if (!t)
+	return NULL_TREE;
+    }
+
   switch (TREE_CODE (t))
     {
     case CAST_EXPR:
@@ -2797,17 +2810,10 @@  min_vis_expr_r (tree *tp, int */*walk_subtrees*/, void *data)
     case NEW_EXPR:
     case CONSTRUCTOR:
     case LAMBDA_EXPR:
+    case TYPE_DECL:
       tpvis = type_visibility (TREE_TYPE (t));
       break;
 
-    case TEMPLATE_DECL:
-      if (DECL_ALIAS_TEMPLATE_P (t) || concept_definition_p (t))
-	/* FIXME: We don't maintain TREE_PUBLIC / DECL_VISIBILITY for
-	   alias templates so we can't trust it here (PR107906).  Ditto
-	   for concepts.  */
-	break;
-      t = DECL_TEMPLATE_RESULT (t);
-      /* Fall through.  */
     case VAR_DECL:
     case FUNCTION_DECL:
       if (decl_constant_var_p (t))
diff --git a/gcc/testsuite/g++.dg/modules/pr118846_a.C b/gcc/testsuite/g++.dg/modules/pr118846_a.C
new file mode 100644
index 00000000000..bbbdde78457
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr118846_a.C
@@ -0,0 +1,18 @@ 
+// PR c++/118846
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi M }
+
+export module M;
+
+template <int N> struct integral_constant { static constexpr int value = N; };
+template <template <class> class> constexpr int cx_count_if() { return 0; }
+template <template <class> class P> struct mp_count_if_impl {
+  using type = integral_constant<cx_count_if<P>()>;
+};
+
+template <template <class> class> struct consume {
+  static constexpr bool value = true;
+};
+template <class P> struct use {
+  using type = consume<P::template fn>;
+};
diff --git a/gcc/testsuite/g++.dg/modules/pr118846_b.C b/gcc/testsuite/g++.dg/modules/pr118846_b.C
new file mode 100644
index 00000000000..a2f28894630
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr118846_b.C
@@ -0,0 +1,10 @@ 
+// PR c++/118846
+// { dg-additional-options "-fmodules" }
+
+module M;
+
+template <typename> struct S {
+  template <typename> struct fn {};
+};
+static_assert(mp_count_if_impl<S>::type::value == 0);
+static_assert(use<S<int>>::type::value);