c++/modules: Fix merging of GM entities in partitions [PR114950]

Currently name lookup generally seems to assume that all entities
declared within a named module (partition) are attached to said module,
which is not true for GM entities (e.g. via extern "C++"), and causes
issues with deduplication.

This patch fixes the issue by ensuring that module attachment of a
declaration is consistently used to handling merging.  Handling this
exposes some issues with deduplicating temploid friends; to resolve this
we always create the BINDING_SLOT_PARTITION slot so that we have
somewhere to place attached names (from any module).

This doesn't yet completely handle issues with allowing otherwise
conflicting temploid friends from different modules to co-exist in the
same module if neither are reachable from the other via name lookup.

	PR c++/114950

gcc/cp/ChangeLog:

	* module.cc (trees_out::decl_value): Stream bit indicating
	imported temploid friends early.
	(trees_in::decl_value): Use this bit with key_mergeable.
	(trees_in::key_mergeable): Allow merging attached declarations
	if they're imported temploid friends (which must be namespace
	scope).
	(module_state::read_cluster): Check for GM entities that may
	require merging even when importing from partitions.
	* name-lookup.cc (enum binding_slots): Adjust comment.
	(get_fixed_binding_slot): Always create partition slot.
	(name_lookup::search_namespace_only): Support binding vectors
	with both partition and GM entities to dedup.
	(walk_module_binding): Likewise.
	(name_lookup::adl_namespace_fns): Likewise.
	(set_module_binding): Likewise.
	(check_module_override): Use attachment of the decl when
	checking overrides rather than named_module_p.
	(lookup_imported_hidden_friend): Use partition slot for finding
	mergeable template bindings.
	* name-lookup.h (set_module_binding): Split mod_glob_flag
	parameter into separate global_p and partition_p params.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/tpl-friend-13_e.C: Adjust error message.
	* g++.dg/modules/ambig-2_a.C: New test.
	* g++.dg/modules/ambig-2_b.C: New test.
	* g++.dg/modules/part-9_a.C: New test.
	* g++.dg/modules/part-9_b.C: New test.
	* g++.dg/modules/part-9_c.C: New test.
	* g++.dg/modules/tpl-friend-15.h: New test.
	* g++.dg/modules/tpl-friend-15_a.C: New test.
	* g++.dg/modules/tpl-friend-15_b.C: New test.
	* g++.dg/modules/tpl-friend-15_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 0f3e1d97c53baa2b50cbdb59156162168d803a66..58ad8cbdb614b550ff21ec65bb8e759a6f5515da 100644 (file)
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2958,7 +2958,8 @@ private:
 public:
   tree decl_container ();
   tree key_mergeable (int tag, merge_kind, tree decl, tree inner, tree type,
-                     tree container, bool is_attached);
+                     tree container, bool is_attached,
+                     bool is_imported_temploid_friend);
   unsigned binfo_mergeable (tree *);
 
 private:
@@ -7806,6 +7807,7 @@ trees_out::decl_value (tree decl, depset *dep)
                       || !TYPE_PTRMEMFUNC_P (TREE_TYPE (decl)));
 
   merge_kind mk = get_merge_kind (decl, dep);
+  bool is_imported_temploid_friend = imported_temploid_friends->get (decl);
 
   if (CHECKING_P)
     {
@@ -7841,13 +7843,11 @@ trees_out::decl_value (tree decl, depset *dep)
                  && DECL_MODULE_ATTACH_P (not_tmpl))
                is_attached = true;
 
-             /* But don't consider imported temploid friends as attached,
-                since importers will need to merge this decl even if it was
-                attached to a different module.  */
-             if (imported_temploid_friends->get (decl))
-               is_attached = false;
-
              bits.b (is_attached);
+
+             /* Also tell the importer whether this is an imported temploid
+                friend, which has implications for merging.  */
+             bits.b (is_imported_temploid_friend);
            }
          bits.b (dep && dep->has_defn ());
        }
@@ -8024,13 +8024,12 @@ trees_out::decl_value (tree decl, depset *dep)
        }
     }
 
-  if (TREE_CODE (inner) == FUNCTION_DECL
-      || TREE_CODE (inner) == TYPE_DECL)
+  if (is_imported_temploid_friend)
     {
       /* Write imported temploid friends so that importers can reconstruct
         this information on stream-in.  */
       tree* slot = imported_temploid_friends->get (decl);
-      tree_node (slot ? *slot : NULL_TREE);
+      tree_node (*slot);
     }
 
   bool is_typedef = false;
@@ -8109,6 +8108,7 @@ trees_in::decl_value ()
 {
   int tag = 0;
   bool is_attached = false;
+  bool is_imported_temploid_friend = false;
   bool has_defn = false;
   unsigned mk_u = u ();
   if (mk_u >= MK_hwm || !merge_kind_name[mk_u])
@@ -8129,7 +8129,10 @@ trees_in::decl_value ()
        {
          bits_in bits = stream_bits ();
          if (!(mk & MK_template_mask) && !state->is_header ())
-           is_attached = bits.b ();
+           {
+             is_attached = bits.b ();
+             is_imported_temploid_friend = bits.b ();
+           }
 
          has_defn = bits.b ();
        }
@@ -8234,7 +8237,7 @@ trees_in::decl_value ()
     parm_tag = fn_parms_init (inner);
 
   tree existing = key_mergeable (tag, mk, decl, inner, type, container,
-                                is_attached);
+                                is_attached, is_imported_temploid_friend);
   tree existing_inner = existing;
   if (existing)
     {
@@ -8339,8 +8342,7 @@ trees_in::decl_value ()
        }
     }
 
-  if (TREE_CODE (inner) == FUNCTION_DECL
-      || TREE_CODE (inner) == TYPE_DECL)
+  if (is_imported_temploid_friend)
     if (tree owner = tree_node ())
       if (is_new)
        imported_temploid_friends->put (decl, owner);
@@ -11177,7 +11179,8 @@ check_mergeable_decl (merge_kind mk, tree decl, tree ovl, merge_key const &key)
 
 tree
 trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
-                        tree type, tree container, bool is_attached)
+                        tree type, tree container, bool is_attached,
+                        bool is_imported_temploid_friend)
 {
   const char *kind = "new";
   tree existing = NULL_TREE;
@@ -11319,6 +11322,7 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
 
          case NAMESPACE_DECL:
            if (is_attached
+               && !is_imported_temploid_friend
                && !(state->is_module () || state->is_partition ()))
              kind = "unique";
            else
@@ -11350,6 +11354,7 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
            break;
 
          case TYPE_DECL:
+           gcc_checking_assert (!is_imported_temploid_friend);
            if (is_attached && !(state->is_module () || state->is_partition ())
                /* Implicit member functions can come from
                   anywhere.  */
@@ -15356,6 +15361,7 @@ module_state::read_cluster (unsigned snum)
            tree visible = NULL_TREE;
            tree type = NULL_TREE;
            bool dedup = false;
+           bool global_p = false;
 
            /* We rely on the bindings being in the reverse order of
               the resulting overload set.  */
@@ -15373,6 +15379,16 @@ module_state::read_cluster (unsigned snum)
                if (sec.get_overrun ())
                  break;
 
+               if (!global_p)
+                 {
+                   /* Check if the decl could require GM merging.  */
+                   tree orig = get_originating_module_decl (decl);
+                   tree inner = STRIP_TEMPLATE (orig);
+                   if (!DECL_LANG_SPECIFIC (inner)
+                       || !DECL_MODULE_ATTACH_P (inner))
+                     global_p = true;
+                 }
+
                if (decls && TREE_CODE (decl) == TYPE_DECL)
                  {
                    /* Stat hack.  */
@@ -15459,10 +15475,8 @@ module_state::read_cluster (unsigned snum)
              break; /* Bail.  */
 
            dump () && dump ("Binding of %P", ns, name);
-           if (!set_module_binding (ns, name, mod,
-                                    is_header () ? -1
-                                    : is_module () || is_partition () ? 1
-                                    : 0,
+           if (!set_module_binding (ns, name, mod, global_p,
+                                    is_module () || is_partition (),
                                     decls, type, visible))
              sec.set_overrun ();
          }

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 8823ab71c6005b834a7fc58327b7e516e7eb62da..872f1af0b2e3716e45a8a005fb1c8a8545b5e7b9 100644 (file)
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -51,8 +51,8 @@ enum binding_slots
 {
  BINDING_SLOT_CURRENT, /* Slot for current TU.  */
  BINDING_SLOT_GLOBAL,  /* Slot for merged global module. */
- BINDING_SLOT_PARTITION, /* Slot for merged partition entities
-                           (optional).  */
+ BINDING_SLOT_PARTITION, /* Slot for merged partition entities or
+                           imported friends.  */
 
  /* Number of always-allocated slots.  */
  BINDING_SLOTS_FIXED = BINDING_SLOT_GLOBAL + 1
@@ -248,9 +248,10 @@ get_fixed_binding_slot (tree *slot, tree name, unsigned ix, int create)
       if (!create)
        return NULL;
 
-      /* The partition slot is only needed when we're a named
-        module.  */
-      bool partition_slot = named_module_p ();
+      /* The partition slot is always needed, in case we have imported
+        temploid friends with attachment different from the module we
+        imported them from.  */
+      bool partition_slot = true;
       unsigned want = ((BINDING_SLOTS_FIXED + partition_slot + (create < 0)
                        + BINDING_VECTOR_SLOTS_PER_CLUSTER - 1)
                       / BINDING_VECTOR_SLOTS_PER_CLUSTER);
@@ -937,7 +938,6 @@ name_lookup::search_namespace_only (tree scope)
                   stat_hack, then everything was exported.  */
                tree type = NULL_TREE;
 
-
                /* If STAT_HACK_P is false, everything is visible, and
                   there's no duplication possibilities.  */
                if (STAT_HACK_P (bind))
@@ -947,9 +947,9 @@ name_lookup::search_namespace_only (tree scope)
                        /* Do we need to engage deduplication?  */
                        int dup = 0;
                        if (MODULE_BINDING_GLOBAL_P (bind))
-                         dup = 1;
-                       else if (MODULE_BINDING_PARTITION_P (bind))
-                         dup = 2;
+                         dup |= 1;
+                       if (MODULE_BINDING_PARTITION_P (bind))
+                         dup |= 2;
                        if (unsigned hit = dup_detect & dup)
                          {
                            if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
@@ -1275,9 +1275,9 @@ name_lookup::adl_namespace_fns (tree scope, bitmap imports)
                        /* Do we need to engage deduplication?  */
                        int dup = 0;
                        if (MODULE_BINDING_GLOBAL_P (bind))
-                         dup = 1;
-                       else if (MODULE_BINDING_PARTITION_P (bind))
-                         dup = 2;
+                         dup |= 1;
+                       if (MODULE_BINDING_PARTITION_P (bind))
+                         dup |= 2;
                        if (unsigned hit = dup_detect & dup)
                          if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
                              || (hit & 2
@@ -3758,6 +3758,9 @@ check_module_override (tree decl, tree mvec, bool hiding,
   binding_cluster *cluster = BINDING_VECTOR_CLUSTER_BASE (mvec);
   unsigned ix = BINDING_VECTOR_NUM_CLUSTERS (mvec);
 
+  tree nontmpl = STRIP_TEMPLATE (decl);
+  bool attached = DECL_LANG_SPECIFIC (nontmpl) && DECL_MODULE_ATTACH_P (nontmpl);
+
   if (BINDING_VECTOR_SLOTS_PER_CLUSTER == BINDING_SLOTS_FIXED)
     {
       cluster++;
@@ -3819,7 +3822,7 @@ check_module_override (tree decl, tree mvec, bool hiding,
     {
       /* Look in the appropriate mergeable decl slot.  */
       tree mergeable = NULL_TREE;
-      if (named_module_p ())
+      if (attached)
        mergeable = BINDING_VECTOR_CLUSTER (mvec, BINDING_SLOT_PARTITION
                                           / BINDING_VECTOR_SLOTS_PER_CLUSTER)
          .slots[BINDING_SLOT_PARTITION % BINDING_VECTOR_SLOTS_PER_CLUSTER];
@@ -3839,15 +3842,13 @@ check_module_override (tree decl, tree mvec, bool hiding,
  matched:
   if (match != error_mark_node)
     {
-      if (named_module_p ())
+      if (attached)
        BINDING_VECTOR_PARTITION_DUPS_P (mvec) = true;
       else
        BINDING_VECTOR_GLOBAL_DUPS_P (mvec) = true;
     }
 
   return match;
-
-  
 }
 
 /* Record DECL as belonging to the current lexical scope.  Check for
@@ -4300,7 +4301,9 @@ walk_module_binding (tree binding, bitmap partitions,
          cluster++;
        }
 
-      bool maybe_dups = BINDING_VECTOR_PARTITION_DUPS_P (binding);
+      /* There could be duplicate module or GMF entries.  */
+      bool maybe_dups = (BINDING_VECTOR_PARTITION_DUPS_P (binding)
+                        || BINDING_VECTOR_GLOBAL_DUPS_P (binding));
 
       for (; ix--; cluster++)
        for (unsigned jx = 0; jx != BINDING_VECTOR_SLOTS_PER_CLUSTER; jx++)
@@ -4394,14 +4397,14 @@ import_module_binding  (tree ns, tree name, unsigned mod, unsigned snum)
 }
 
 /* An import of MODULE is binding NS::NAME.  There should be no
-   existing binding for >= MODULE.  MOD_GLOB indicates whether MODULE
-   is a header_unit (-1) or part of the current module (+1).  VALUE
-   and TYPE are the value and type bindings. VISIBLE are the value
-   bindings being exported.  */
+   existing binding for >= MODULE.  GLOBAL_P indicates whether the
+   bindings include global module entities.  PARTITION_P is true if
+   it is part of the current module. VALUE and TYPE are the value
+   and type bindings. VISIBLE are the value bindings being exported.  */
 
 bool
-set_module_binding (tree ns, tree name, unsigned mod, int mod_glob,
-                   tree value, tree type, tree visible)
+set_module_binding (tree ns, tree name, unsigned mod, bool global_p,
+                   bool partition_p, tree value, tree type, tree visible)
 {
   if (!value)
     /* Bogus BMIs could give rise to nothing to bind.  */
@@ -4419,19 +4422,19 @@ set_module_binding (tree ns, tree name, unsigned mod, int mod_glob,
     return false;
 
   tree bind = value;
-  if (type || visible != bind || mod_glob)
+  if (type || visible != bind || partition_p || global_p)
     {
       bind = stat_hack (bind, type);
       STAT_VISIBLE (bind) = visible;
-      if ((mod_glob > 0 && TREE_PUBLIC (ns))
+      if ((partition_p && TREE_PUBLIC (ns))
          || (type && DECL_MODULE_EXPORT_P (type)))
        STAT_TYPE_VISIBLE_P (bind) = true;
     }
 
-  /* Note if this is this-module or global binding.  */
-  if (mod_glob > 0)
+  /* Note if this is this-module and/or global binding.  */
+  if (partition_p)
     MODULE_BINDING_PARTITION_P (bind) = true;
-  else if (mod_glob < 0)
+  if (global_p)
     MODULE_BINDING_GLOBAL_P (bind) = true;
 
   *mslot = bind;
@@ -4540,10 +4543,8 @@ lookup_imported_hidden_friend (tree friend_tmpl)
       || !DECL_MODULE_IMPORT_P (inner))
     return NULL_TREE;
 
-  /* Imported temploid friends are not considered as attached to this
-     module for merging purposes.  */
-  tree bind = get_mergeable_namespace_binding (current_namespace,
-                                              DECL_NAME (inner), false);
+  tree bind = get_mergeable_namespace_binding
+    (current_namespace, DECL_NAME (inner), DECL_MODULE_ATTACH_P (inner));
   if (!bind)
     return NULL_TREE;
 

diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index 5cf6ae6374af6a4fa3f03986aa2e1466984bf592..7c4193444dd81f79206e6da38a1922692e677d17 100644 (file)
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -484,7 +484,7 @@ extern tree lookup_class_binding (tree ctx, tree name);
 extern bool import_module_binding (tree ctx, tree name, unsigned mod,
                                   unsigned snum);
 extern bool set_module_binding (tree ctx, tree name, unsigned mod,
-                               int mod_glob_flag,
+                               bool global_p, bool partition_p,
                                tree value, tree type, tree visible);
 extern void add_module_namespace_decl (tree ns, tree decl);
 

diff --git a/gcc/testsuite/g++.dg/modules/ambig-2_a.C b/gcc/testsuite/g++.dg/modules/ambig-2_a.C
new file mode 100644 (file)
index 0000000..d5dcc93
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/ambig-2_a.C
@@ -0,0 +1,7 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi A }
+
+export module A;
+
+extern "C++" int foo ();
+extern "C++" char bar ();

diff --git a/gcc/testsuite/g++.dg/modules/ambig-2_b.C b/gcc/testsuite/g++.dg/modules/ambig-2_b.C
new file mode 100644 (file)
index 0000000..b94416a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/ambig-2_b.C
@@ -0,0 +1,10 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi !B }
+
+export module B;
+import A;
+
+extern "C++" int foo ();
+extern "C++" int bar ();  // { dg-error "ambiguating new declaration" }
+
+// { dg-prune-output "not writing module" }

diff --git a/gcc/testsuite/g++.dg/modules/part-9_a.C b/gcc/testsuite/g++.dg/modules/part-9_a.C
new file mode 100644 (file)
index 0000000..dc033d3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-9_a.C
@@ -0,0 +1,10 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:a }
+
+module M:a;
+
+struct A {};
+extern "C++" struct B {};
+void f(int) {}
+extern "C++" void f(double) {}

diff --git a/gcc/testsuite/g++.dg/modules/part-9_b.C b/gcc/testsuite/g++.dg/modules/part-9_b.C
new file mode 100644 (file)
index 0000000..7339da2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-9_b.C
@@ -0,0 +1,10 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:b }
+
+module M:b;
+
+struct A {};
+extern "C++" struct B {};
+void f(int) {}
+extern "C++" void f(double) {}

diff --git a/gcc/testsuite/g++.dg/modules/part-9_c.C b/gcc/testsuite/g++.dg/modules/part-9_c.C
new file mode 100644 (file)
index 0000000..26ac677
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-9_c.C
@@ -0,0 +1,8 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M }
+// Handle merging definitions of extern "C++" decls across partitions
+
+export module M;
+import :a;
+import :b;

diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
index afbd0a39c238fe04295dfb011cfdb7c2039c4a15..b32fd98b756accad14bed5a2d7254a78c0faca1c 100644 (file)
--- a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
@@ -1,8 +1,8 @@
 // { dg-additional-options "-fmodules-ts" }
 
 // 'import X' does not correctly notice that S has already been declared.
-struct S {};  // { dg-message "previously declared" "" { xfail *-*-* } }
-template <typename> struct T {};  // { dg-message "previously declared" }
+struct S {};  // { dg-message "previous declaration" "" { xfail *-*-* } }
+template <typename> struct T {};  // { dg-message "previous declaration" }
 void f() {}  // { dg-message "previously declared" }
 template <typename T> void g() {}  // { dg-message "previously declared" }
 

diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15.h b/gcc/testsuite/g++.dg/modules/tpl-friend-15.h
new file mode 100644 (file)
index 0000000..e4d3fff
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15.h
@@ -0,0 +1,11 @@
+// PR c++/114950
+
+template <typename T>
+struct A {
+  friend void x();
+};
+template <typename T>
+struct B {
+  virtual void f() { A<T> r; }
+};
+template struct B<int>;

diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
new file mode 100644 (file)
index 0000000..04c8008
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
@@ -0,0 +1,8 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:a }
+
+module M:a;
+extern "C++" {
+  #include "tpl-friend-15.h"
+}

diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
new file mode 100644 (file)
index 0000000..781882f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
@@ -0,0 +1,8 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:b }
+
+module M:b;
+extern "C++" {
+  #include "tpl-friend-15.h"
+}

diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
new file mode 100644 (file)
index 0000000..ced7e87
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
@@ -0,0 +1,7 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M }
+
+export module M;
+import :a;
+import :b;