https://gcc.gnu.org/g:c592310d5275e09977504c136419686bd2277af0
commit r15-2807-gc592310d5275e09977504c136419686bd2277af0
Author: Nathaniel Shead <nathanielosh...@gmail.com>
Date: Mon Aug 5 22:37:57 2024 +1000
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 <nathanielosh...@gmail.com>
Reviewed-by: Jason Merrill <ja...@redhat.com>
Diff:
---
gcc/cp/module.cc | 52 +++++++++++++--------
gcc/cp/name-lookup.cc | 65 +++++++++++++-------------
gcc/cp/name-lookup.h | 2 +-
gcc/testsuite/g++.dg/modules/ambig-2_a.C | 7 +++
gcc/testsuite/g++.dg/modules/ambig-2_b.C | 10 ++++
gcc/testsuite/g++.dg/modules/part-9_a.C | 10 ++++
gcc/testsuite/g++.dg/modules/part-9_b.C | 10 ++++
gcc/testsuite/g++.dg/modules/part-9_c.C | 8 ++++
gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C | 4 +-
gcc/testsuite/g++.dg/modules/tpl-friend-15.h | 11 +++++
gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C | 8 ++++
gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C | 8 ++++
gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C | 7 +++
13 files changed, 148 insertions(+), 54 deletions(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 0f3e1d97c53b..58ad8cbdb614 100644
--- 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 8823ab71c600..872f1af0b2e3 100644
--- 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 5cf6ae6374af..7c4193444dd8 100644
--- 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
index 000000000000..d5dcc93584a7
--- /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
index 000000000000..b94416aabbf2
--- /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
index 000000000000..dc033d301eeb
--- /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
index 000000000000..7339da22d97d
--- /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
index 000000000000..26ac6777b7a3
--- /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 afbd0a39c238..b32fd98b756a 100644
--- 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
index 000000000000..e4d3fff44450
--- /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
index 000000000000..04c800875f48
--- /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
index 000000000000..781882f97bc5
--- /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
index 000000000000..ced7e87d993b
--- /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;
