[gcc r11-6954] c++: header unit template alias merging [PR 98770]

[gcc r11-6954] c++: header unit template alias merging [PR 98770]

[Nathan Sidwell]

https://gcc.gnu.org/g:af66f4f1b06f5e0c099dfced2fcf7b1b23fa53e7

commit r11-6954-gaf66f4f1b06f5e0c099dfced2fcf7b1b23fa53e7
Author: Nathan Sidwell <nathan@acm.org>
Date:   Thu Jan 28 04:48:33 2021 -0800

    c++: header unit template alias merging [PR 98770]
    
    Typedefs are streamed by streaming the underlying type, and then
    recreating the typedef.  But this breaks checking a duplicate is the
    same as the original when it is a template alias -- we end up checking
    a template alias (eg __void_t) against the underlying type (void).
    And those are not the same template alias.  This stops pretendig that
    the underlying type is the typedef for that checking and tells
    is_matching_decl 'you have a typedef', so it knows what to do.  (We do
    not want to recreate the typedef of the duplicate, because that whole
    set of nodes is going to go away.)
    
            PR c++/98770
            gcc/cp/
            * module.cc (trees_out::decl_value): Swap is_typedef & TYPE_NAME
            check order.
            (trees_in::decl_value): Do typedef frobbing only when installing
            a new typedef, adjust is_matching_decl call.  Swap is_typedef
            & TYPE_NAME check.
            (trees_in::is_matching_decl): Add is_typedef parm. Adjust variable
            names and deal with typedef checking.
            gcc/testsuite/
            * g++.dg/modules/pr98770_a.C: New.
            * g++.dg/modules/pr98770_b.C: New.

Diff:
---
 gcc/cp/module.cc                         | 67 +++++++++++++++++---------------
 gcc/testsuite/g++.dg/modules/pr98770_a.C | 10 +++++
 gcc/testsuite/g++.dg/modules/pr98770_b.C | 12 ++++++
 3 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 18f5de8724b..daf75b16007 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3029,7 +3029,7 @@ public:
   bool read_definition (tree decl);
   
 private:
-  bool is_matching_decl (tree existing, tree decl);
+  bool is_matching_decl (tree existing, tree decl, bool is_typedef);
   static bool install_implicit_member (tree decl);
   bool read_function_def (tree decl, tree maybe_template);
   bool read_var_def (tree decl, tree maybe_template);
@@ -7864,8 +7864,8 @@ trees_out::decl_value (tree decl, depset *dep)
 			 || !dep == (VAR_OR_FUNCTION_DECL_P (inner)
 				     && DECL_LOCAL_DECL_P (inner)));
   else if ((TREE_CODE (inner) == TYPE_DECL
-	    && TYPE_NAME (TREE_TYPE (inner)) == inner
-	    && !is_typedef)
+	    && !is_typedef
+	    && TYPE_NAME (TREE_TYPE (inner)) == inner)
 	   || TREE_CODE (inner) == FUNCTION_DECL)
     {
       bool write_defn = !dep && has_definition (decl);
@@ -8088,12 +8088,6 @@ trees_in::decl_value ()
 		     && TREE_CODE (inner) == TYPE_DECL
 		     && DECL_ORIGINAL_TYPE (inner)
 		     && !TREE_TYPE (inner));
-  if (is_typedef)
-    {
-      /* Frob it to be ready for cloning.  */
-      TREE_TYPE (inner) = DECL_ORIGINAL_TYPE (inner);
-      DECL_ORIGINAL_TYPE (inner) = NULL_TREE;
-    }
 
   existing = back_refs[~tag];
   bool installed = install_entity (existing);
@@ -8156,7 +8150,12 @@ trees_in::decl_value ()
 	}
 
       if (is_typedef)
-	set_underlying_type (inner);
+	{
+	  /* Frob it to be ready for cloning.  */
+	  TREE_TYPE (inner) = DECL_ORIGINAL_TYPE (inner);
+	  DECL_ORIGINAL_TYPE (inner) = NULL_TREE;
+	  set_underlying_type (inner);
+	}
 
       if (inner_tag)
 	/* Set the TEMPLATE_DECL's type.  */
@@ -8218,7 +8217,7 @@ trees_in::decl_value ()
 	/* Set the TEMPLATE_DECL's type.  */
 	TREE_TYPE (decl) = TREE_TYPE (inner);
 
-      if (!is_matching_decl (existing, decl))
+      if (!is_matching_decl (existing, decl, is_typedef))
 	unmatched_duplicate (existing);
 
       /* And our result is the existing node.  */
@@ -8257,8 +8256,8 @@ trees_in::decl_value ()
   if (inner
       && !NAMESPACE_SCOPE_P (inner)
       && ((TREE_CODE (inner) == TYPE_DECL
-	   && TYPE_NAME (TREE_TYPE (inner)) == inner
-	   && !is_typedef)
+	   && !is_typedef
+	   && TYPE_NAME (TREE_TYPE (inner)) == inner)
 	  || TREE_CODE (inner) == FUNCTION_DECL)
       && u ())
     read_definition (decl);
@@ -11088,7 +11087,7 @@ trees_in::binfo_mergeable (tree *type)
    decls_match because it can cause instantiations of constraints.  */
 
 bool
-trees_in::is_matching_decl (tree existing, tree decl)
+trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef)
 {
   // FIXME: We should probably do some duplicate decl-like stuff here
   // (beware, default parms should be the same?)  Can we just call
@@ -11099,35 +11098,36 @@ trees_in::is_matching_decl (tree existing, tree decl)
   // can elide some of the checking
   gcc_checking_assert (TREE_CODE (existing) == TREE_CODE (decl));
 
-  tree inner = decl;
+  tree d_inner = decl;
+  tree e_inner = existing;
   if (TREE_CODE (decl) == TEMPLATE_DECL)
     {
-      inner = DECL_TEMPLATE_RESULT (decl);
-      gcc_checking_assert (TREE_CODE (DECL_TEMPLATE_RESULT (existing))
-			   == TREE_CODE (inner));
+      d_inner = DECL_TEMPLATE_RESULT (d_inner);
+      e_inner = DECL_TEMPLATE_RESULT (e_inner);
+      gcc_checking_assert (TREE_CODE (e_inner) == TREE_CODE (d_inner));
     }
 
   gcc_checking_assert (!map_context_from);
   /* This mapping requres the new decl on the lhs and the existing
      entity on the rhs of the comparitors below.  */
-  map_context_from = inner;
-  map_context_to = STRIP_TEMPLATE (existing);
+  map_context_from = d_inner;
+  map_context_to = e_inner;
 
-  if (TREE_CODE (inner) == FUNCTION_DECL)
+  if (TREE_CODE (d_inner) == FUNCTION_DECL)
     {
       tree e_ret = fndecl_declared_return_type (existing);
       tree d_ret = fndecl_declared_return_type (decl);
 
-      if (decl != inner && DECL_NAME (inner) == fun_identifier
-	  && LAMBDA_TYPE_P (DECL_CONTEXT (inner)))
+      if (decl != d_inner && DECL_NAME (d_inner) == fun_identifier
+	  && LAMBDA_TYPE_P (DECL_CONTEXT (d_inner)))
 	/* This has a recursive type that will compare different.  */;
       else if (!same_type_p (d_ret, e_ret))
 	goto mismatch;
 
-      tree e_type = TREE_TYPE (existing);
-      tree d_type = TREE_TYPE (decl);
+      tree e_type = TREE_TYPE (e_inner);
+      tree d_type = TREE_TYPE (d_inner);
 
-      if (DECL_EXTERN_C_P (decl) != DECL_EXTERN_C_P (existing))
+      if (DECL_EXTERN_C_P (d_inner) != DECL_EXTERN_C_P (e_inner))
 	goto mismatch;
 
       for (tree e_args = TYPE_ARG_TYPES (e_type),
@@ -11176,6 +11176,13 @@ trees_in::is_matching_decl (tree existing, tree decl)
 	       && !comp_except_specs (d_spec, e_spec, ce_type))
 	goto mismatch;
     }
+  else if (is_typedef)
+    {
+      if (!DECL_ORIGINAL_TYPE (e_inner)
+	  || !same_type_p (DECL_ORIGINAL_TYPE (d_inner),
+			   DECL_ORIGINAL_TYPE (e_inner)))
+	goto mismatch;
+    }
   /* Using cp_tree_equal because we can meet TYPE_ARGUMENT_PACKs
      here. I suspect the entities that directly do that are things
      that shouldn't go to duplicate_decls (FIELD_DECLs etc).   */
@@ -11255,12 +11262,10 @@ trees_in::is_matching_decl (tree existing, tree decl)
     /* Don't instantiate again!  */
     DECL_TEMPLATE_INSTANTIATED (existing) = true;
 
-  tree e_inner = inner == decl ? existing : DECL_TEMPLATE_RESULT (existing);
-
-  if (TREE_CODE (inner) == FUNCTION_DECL
-      && DECL_DECLARED_INLINE_P (inner))
+  if (TREE_CODE (d_inner) == FUNCTION_DECL
+      && DECL_DECLARED_INLINE_P (d_inner))
     DECL_DECLARED_INLINE_P (e_inner) = true;
-  if (!DECL_EXTERNAL (inner))
+  if (!DECL_EXTERNAL (d_inner))
     DECL_EXTERNAL (e_inner) = false;
 
   // FIXME: Check default tmpl and fn parms here
diff --git a/gcc/testsuite/g++.dg/modules/pr98770_a.C b/gcc/testsuite/g++.dg/modules/pr98770_a.C
new file mode 100644
index 00000000000..668ff2891ca
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr98770_a.C
@@ -0,0 +1,10 @@
+// PR 98770 confused about duplicate template type aliases
+// { dg-additional-options "-fmodules-ts -Wno-pedantic" }
+
+module ;
+# 6 __FILE__ 1
+template<typename> using __void_t = void;
+# 8 "" 2
+export module Foo;
+
+export using B = __void_t<int>;
diff --git a/gcc/testsuite/g++.dg/modules/pr98770_b.C b/gcc/testsuite/g++.dg/modules/pr98770_b.C
new file mode 100644
index 00000000000..a4ab2376815
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr98770_b.C
@@ -0,0 +1,12 @@
+// PR 98770 confused about duplicate template type aliases
+// { dg-additional-options "-fmodules-ts -Wno-pedantic" }
+
+module ;
+# 6 __FILE__ 1
+template<typename> using __void_t = void;
+# 8 "" 2
+export module Bar;
+
+import Foo;
+
+export B *b;