Bootstrapped and regtested (so far just modules.exp) on
x86_64-pc-linux-gnu, OK for trunk if full regtest passes?
-- >8 --
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).
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.
(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>
---
gcc/cp/module.cc | 55 ++++++++++------
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 +++
.../g++.dg/modules/tpl-friend-13_e.C | 4 +-
gcc/testsuite/g++.dg/modules/tpl-friend-15.h | 11 ++++
.../g++.dg/modules/tpl-friend-15_a.C | 8 +++
.../g++.dg/modules/tpl-friend-15_b.C | 8 +++
.../g++.dg/modules/tpl-friend-15_c.C | 7 ++
13 files changed, 150 insertions(+), 55 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/ambig-2_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/ambig-2_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/part-9_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/part-9_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/part-9_c.C
create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15.h
create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index d1607a06757..e6b569ebca5 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2955,7 +2955,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:
@@ -7803,6 +7804,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)
{
@@ -7838,13 +7840,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 ());
}
@@ -8021,13 +8021,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;
@@ -8106,6 +8105,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])
@@ -8126,7 +8126,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 ();
}
@@ -8231,7 +8234,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)
{
@@ -8336,8 +8339,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);
@@ -11174,7 +11176,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;
@@ -11316,6 +11319,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
@@ -11347,7 +11351,9 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree
decl, tree inner,
break;
case TYPE_DECL:
- if (is_attached && !(state->is_module () || state->is_partition ())
+ if (is_attached
+ && !is_imported_temploid_friend
+ && !(state->is_module () || state->is_partition ())
/* Implicit member functions can come from
anywhere. */
&& !(DECL_ARTIFICIAL (decl)
@@ -15291,6 +15297,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. */
@@ -15308,6 +15315,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. */
@@ -15394,10 +15411,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 8823ab71c60..872f1af0b2e 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 5cf6ae6374a..7c4193444dd 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 00000000000..d5dcc93584a
--- /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 00000000000..b94416aabbf
--- /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 00000000000..dc033d301ee
--- /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 00000000000..7339da22d97
--- /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 00000000000..26ac6777b7a
--- /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 afbd0a39c23..b32fd98b756 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 00000000000..e4d3fff4445
--- /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 00000000000..04c800875f4
--- /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 00000000000..781882f97bc
--- /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 00000000000..ced7e87d993
--- /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;
--
2.43.2
