https://gcc.gnu.org/g:85f15ea65a97686ad39af0c14b7dd9a9372e3a19
commit r15-964-g85f15ea65a97686ad39af0c14b7dd9a9372e3a19
Author: Nathaniel Shead <nathanielosh...@gmail.com>
Date: Sat Jun 1 01:14:44 2024 +1000
c++/modules: Fix revealing with using-decls [PR114867]
This patch fixes a couple issues with the current handling of revealing
declarations with using-decls.
Firstly, doing 'remove_node' when handling function overload sets is not
safe, because it not only mutates the OVERLOAD we're walking over but
potentially any other references to this OVERLOAD that are cached from
phase-1 template lookup. This causes the attached using-17 testcase to
fail because the overload set in 'X::test()' no longer contains the
'ns::f(T)' template once instantiated at the end of the file.
This patch works around this by simply not removing the old declaration.
This does make the overload list potentially longer than it otherwise
would have been, but only when re-exporting the same set of functions in
a using-decl. Additionally, because 'ovl_insert' always prepends these
newly inserted overloads, repeated exported using-decls won't continue
to add declarations, as the first exported using-decl will be found
before the original (unexported) declaration.
Another, related, issue is that using-decls of GMF entities currently
doesn't mark them as reachable unless they are also exported, and thus
they may not be available in e.g. module implementation units. We solve
this with a new flag on OVERLOADs set when they are declared within the
module purview. This starts to run into the more general issue of
handling using-decls of non-functions (see e.g. PR114863) but by just
marking such GMF entities as purview we can work around this for now.
This also allows us to get rid of the special-casing of exported
using-decls in 'add_binding_entity', which was incorrect anyway: a
non-exported using-decl still needs to be emitted anyway if it lives in
the module purview, even if referring to a non-purview item.
PR c++/114867
gcc/cp/ChangeLog:
* cp-tree.h (OVL_PURVIEW_P): New.
(ovl_iterator::purview_p): New.
* module.cc (depset::hash::add_binding_entity): Only ignore
entities not within module purview. Set OVL_PURVIEW_P on new
OVERLOADs for emitted declarations.
(module_state::read_cluster): Imported using-decls are always
in purview, mark as OVL_PURVIEW_P.
* name-lookup.h (enum WMB_Flags): New WMB_Purview flag.
* name-lookup.cc (walk_module_binding): Set WMB_Purview as
needed.
(do_nonmember_using_decl): Don't remove from existing OVERLOADs.
Also reveal non-exported decls. Also reveal 'extern "C"' decls.
Add workaround to reveal non-function decls.
* tree.cc (ovl_insert): Adjust to also set OVL_PURVIEW_P when
needed.
gcc/testsuite/ChangeLog:
* g++.dg/modules/using-17_a.C: New test.
* g++.dg/modules/using-17_b.C: New test.
* g++.dg/modules/using-18_a.C: New test.
* g++.dg/modules/using-18_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
Diff:
---
gcc/cp/cp-tree.h | 7 ++
gcc/cp/module.cc | 8 ++-
gcc/cp/name-lookup.cc | 106 ++++++++++++++++--------------
gcc/cp/name-lookup.h | 1 +
gcc/cp/tree.cc | 12 ++--
gcc/testsuite/g++.dg/modules/using-17_a.C | 31 +++++++++
gcc/testsuite/g++.dg/modules/using-17_b.C | 13 ++++
gcc/testsuite/g++.dg/modules/using-18_a.C | 29 ++++++++
gcc/testsuite/g++.dg/modules/using-18_b.C | 11 ++++
9 files changed, 161 insertions(+), 57 deletions(-)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 6206482c602..565e4a9290e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -813,6 +813,8 @@ typedef struct ptrmem_cst * ptrmem_cst_t;
#define OVL_LOOKUP_P(NODE) TREE_LANG_FLAG_4 (OVERLOAD_CHECK (NODE))
/* If set, this OVL_USING_P overload is exported. */
#define OVL_EXPORT_P(NODE) TREE_LANG_FLAG_5 (OVERLOAD_CHECK (NODE))
+/* If set, this OVL_USING_P overload is in the module purview. */
+#define OVL_PURVIEW_P(NODE) (OVERLOAD_CHECK (NODE)->base.public_flag)
/* If set, this overload includes name-independent declarations. */
#define OVL_NAME_INDEPENDENT_DECL_P(NODE) \
TREE_LANG_FLAG_6 (OVERLOAD_CHECK (NODE))
@@ -887,6 +889,11 @@ class ovl_iterator {
return (TREE_CODE (ovl) == USING_DECL
|| (TREE_CODE (ovl) == OVERLOAD && OVL_USING_P (ovl)));
}
+ /* Whether this using is in the module purview. */
+ bool purview_p () const
+ {
+ return OVL_PURVIEW_P (get_using ());
+ }
/* Whether this using is being exported. */
bool exporting_p () const
{
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 3f8f84bb9fd..ed24814b601 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -13137,9 +13137,8 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags
flags, void *data_)
inner = DECL_TEMPLATE_RESULT (inner);
if ((!DECL_LANG_SPECIFIC (inner) || !DECL_MODULE_PURVIEW_P (inner))
- && !((flags & WMB_Using) && (flags & WMB_Export)))
- /* Ignore global module fragment entities unless explicitly
- exported with a using declaration. */
+ && !((flags & WMB_Using) && (flags & WMB_Purview)))
+ /* Ignore entities not within the module purview. */
return false;
if (VAR_OR_FUNCTION_DECL_P (inner)
@@ -13189,6 +13188,7 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags
flags, void *data_)
{
if (!(flags & WMB_Hidden))
d->clear_hidden_binding ();
+ OVL_PURVIEW_P (d->get_entity ()) = true;
if (flags & WMB_Export)
OVL_EXPORT_P (d->get_entity ()) = true;
return bool (flags & WMB_Export);
@@ -13230,6 +13230,7 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags
flags, void *data_)
decl = ovl_make (decl, NULL_TREE);
if (!unscoped_enum_const_p)
OVL_USING_P (decl) = true;
+ OVL_PURVIEW_P (decl) = true;
if (flags & WMB_Export)
OVL_EXPORT_P (decl) = true;
}
@@ -15311,6 +15312,7 @@ module_state::read_cluster (unsigned snum)
{
dedup = true;
OVL_USING_P (decls) = true;
+ OVL_PURVIEW_P (decls) = true;
if (flags & cbf_export)
OVL_EXPORT_P (decls) = true;
}
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index f1f8c19feb1..3d3e20f48cb 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -4234,6 +4234,8 @@ walk_module_binding (tree binding, bitmap partitions,
if (iter.using_p ())
{
flags = WMB_Flags (flags | WMB_Using);
+ if (iter.purview_p ())
+ flags = WMB_Flags (flags | WMB_Purview);
if (iter.exporting_p ())
flags = WMB_Flags (flags | WMB_Export);
}
@@ -4307,6 +4309,8 @@ walk_module_binding (tree binding, bitmap partitions,
if (iter.using_p ())
{
flags = WMB_Flags (flags | WMB_Using);
+ if (iter.purview_p ())
+ flags = WMB_Flags (flags | WMB_Purview);
if (iter.exporting_p ())
flags = WMB_Flags (flags | WMB_Export);
}
@@ -5210,9 +5214,10 @@ do_nonmember_using_decl (name_lookup &lookup, bool
fn_scope_p,
for (lkp_iterator usings (lookup.value); usings; ++usings)
{
tree new_fn = *usings;
- bool exporting = revealing_p && module_exporting_p ();
- if (exporting)
- exporting = check_can_export_using_decl (new_fn);
+ tree inner = STRIP_TEMPLATE (new_fn);
+ bool exporting_p = revealing_p && module_exporting_p ();
+ if (exporting_p)
+ exporting_p = check_can_export_using_decl (new_fn);
/* [namespace.udecl]
@@ -5239,18 +5244,18 @@ do_nonmember_using_decl (name_lookup &lookup, bool
fn_scope_p,
;
else if (old.using_p ())
{
- if (exporting)
- /* Update in place. 'tis ok. */
+ /* Update in place. 'tis ok. */
+ OVL_PURVIEW_P (old.get_using ()) = true;
+ if (exporting_p)
OVL_EXPORT_P (old.get_using ()) = true;
- ;
- }
- else if (DECL_MODULE_EXPORT_P (new_fn))
- ;
- else
- {
- value = old.remove_node (value);
- found = false;
}
+ else if (!DECL_LANG_SPECIFIC (inner)
+ || !DECL_MODULE_PURVIEW_P (inner))
+ /* We need to re-insert this function as a revealed
+ (possibly exported) declaration. We can't remove
+ the existing decl because that will change any
+ overloads cached in template functions. */
+ found = false;
break;
}
else if (old.using_p ())
@@ -5261,8 +5266,14 @@ do_nonmember_using_decl (name_lookup &lookup, bool
fn_scope_p,
continue; /* Parameters do not match. */
else if (decls_match (new_fn, old_fn))
{
- /* Extern "C" in different namespaces. */
+ /* Extern "C" in different namespaces. But similarly
+ to above, if revealing a not-revealed thing we may
+ need to reinsert. */
found = true;
+ if (revealing_p
+ && (!DECL_LANG_SPECIFIC (inner)
+ || !DECL_MODULE_PURVIEW_P (inner)))
+ found = false;
break;
}
else
@@ -5278,7 +5289,7 @@ do_nonmember_using_decl (name_lookup &lookup, bool
fn_scope_p,
/* Unlike the decl-pushing case we don't drop anticipated
builtins here. They don't cause a problem, and we'd
like to match them with a future declaration. */
- value = ovl_insert (new_fn, value, 1 + exporting);
+ value = ovl_insert (new_fn, value, 1 + revealing_p + exporting_p);
}
}
else if (value
@@ -5291,32 +5302,32 @@ do_nonmember_using_decl (name_lookup &lookup, bool
fn_scope_p,
}
else if (insert_p)
{
- if (revealing_p
- && module_exporting_p ()
- && check_can_export_using_decl (lookup.value)
- && lookup.value == value
- && !DECL_MODULE_EXPORT_P (value))
+ /* FIXME: Handle exporting declarations from a different scope
+ without also marking those declarations as exported.
+ This will require not just binding directly to the underlying
+ value; see c++/114863 and c++/114865. We allow this for purview
+ declarations for now as this doesn't (currently) cause ICEs
+ later down the line, but this should be revisited. */
+ if (revealing_p)
{
- /* We're redeclaring the same value, but this time as
- newly exported: make sure to mark it as such. */
- if (TREE_CODE (value) == TEMPLATE_DECL)
- {
- DECL_MODULE_EXPORT_P (value) = true;
-
- tree result = DECL_TEMPLATE_RESULT (value);
- retrofit_lang_decl (result);
- DECL_MODULE_PURVIEW_P (result) = true;
- DECL_MODULE_EXPORT_P (result) = true;
- }
- else
+ if (module_exporting_p ()
+ && check_can_export_using_decl (lookup.value)
+ && lookup.value == value
+ && !DECL_MODULE_EXPORT_P (lookup.value))
{
- retrofit_lang_decl (value);
- DECL_MODULE_PURVIEW_P (value) = true;
- DECL_MODULE_EXPORT_P (value) = true;
+ /* We're redeclaring the same value, but this time as
+ newly exported: make sure to mark it as such. */
+ if (TREE_CODE (lookup.value) == TEMPLATE_DECL)
+ DECL_MODULE_EXPORT_P (DECL_TEMPLATE_RESULT (lookup.value)) =
true;
+ DECL_MODULE_EXPORT_P (lookup.value) = true;
}
+ /* We're also revealing this declaration with a using-decl,
+ so mark it as purview to ensure it's emitted. */
+ tree inner = STRIP_TEMPLATE (lookup.value);
+ retrofit_lang_decl (inner);
+ DECL_MODULE_PURVIEW_P (inner) = true;
}
- else
- value = lookup.value;
+ value = lookup.value;
}
/* Now the type binding. */
@@ -5329,20 +5340,17 @@ do_nonmember_using_decl (name_lookup &lookup, bool
fn_scope_p,
}
else if (insert_p)
{
- if (revealing_p
- && module_exporting_p ()
- && check_can_export_using_decl (lookup.type)
- && lookup.type == type
- && !DECL_MODULE_EXPORT_P (type))
+ if (revealing_p)
{
- /* We're redeclaring the same type, but this time as
- newly exported: make sure to mark it as such. */
- retrofit_lang_decl (type);
- DECL_MODULE_PURVIEW_P (type) = true;
- DECL_MODULE_EXPORT_P (type) = true;
+ /* As with revealing value bindings. */
+ if (module_exporting_p ()
+ && check_can_export_using_decl (lookup.type)
+ && lookup.type == type)
+ DECL_MODULE_EXPORT_P (lookup.type) = true;
+ retrofit_lang_decl (lookup.type);
+ DECL_MODULE_PURVIEW_P (lookup.type) = true;
}
- else
- type = lookup.type;
+ type = lookup.type;
}
}
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index d2371323337..5cf6ae6374a 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -495,6 +495,7 @@ enum WMB_Flags
WMB_Export = 1 << 1,
WMB_Using = 1 << 2,
WMB_Hidden = 1 << 3,
+ WMB_Purview = 1 << 4,
};
extern unsigned walk_module_binding (tree binding, bitmap partitions,
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 72dd46e1bd1..d2a8f79ffab 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -2353,11 +2353,11 @@ ovl_make (tree fn, tree next)
return result;
}
-/* Add FN to the (potentially NULL) overload set OVL. USING_OR_HIDDEN is >
- zero if this is a using-decl. It is > 1 if we're exporting the
- using decl. USING_OR_HIDDEN is < 0, if FN is hidden. (A decl
- cannot be both using and hidden.) We keep the hidden decls first,
- but remaining ones are unordered. */
+/* Add FN to the (potentially NULL) overload set OVL. USING_OR_HIDDEN is > 0
+ if this is a using-decl. It is > 1 if we're revealing the using decl.
+ It is > 2 if we're also exporting it. USING_OR_HIDDEN is < 0, if FN is
+ hidden. (A decl cannot be both using and hidden.) We keep the hidden
+ decls first, but remaining ones are unordered. */
tree
ovl_insert (tree fn, tree maybe_ovl, int using_or_hidden)
@@ -2384,6 +2384,8 @@ ovl_insert (tree fn, tree maybe_ovl, int using_or_hidden)
{
OVL_DEDUP_P (maybe_ovl) = OVL_USING_P (maybe_ovl) = true;
if (using_or_hidden > 1)
+ OVL_PURVIEW_P (maybe_ovl) = true;
+ if (using_or_hidden > 2)
OVL_EXPORT_P (maybe_ovl) = true;
}
}
diff --git a/gcc/testsuite/g++.dg/modules/using-17_a.C
b/gcc/testsuite/g++.dg/modules/using-17_a.C
new file mode 100644
index 00000000000..de601ea2be0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-17_a.C
@@ -0,0 +1,31 @@
+// PR c++/114867
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi M }
+
+module;
+
+namespace ns {
+ template <typename T> void f(T);
+
+ namespace inner {
+ class E {};
+ int f(E);
+ }
+ using inner::f;
+}
+
+export module M;
+
+template <typename T>
+struct X {
+ void test() { ns::f(T{}); }
+};
+template struct X<int>;
+
+export namespace ns {
+ using ns::f;
+}
+
+export auto get_e() {
+ return ns::inner::E{};
+}
diff --git a/gcc/testsuite/g++.dg/modules/using-17_b.C
b/gcc/testsuite/g++.dg/modules/using-17_b.C
new file mode 100644
index 00000000000..e31582110e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-17_b.C
@@ -0,0 +1,13 @@
+// PR c++/114867
+// { dg-additional-options "-fmodules-ts" }
+
+import M;
+
+int main() {
+ int a = 0;
+ ns::f(a);
+
+ // Should also still find the inner::f overload
+ auto e = get_e();
+ int r = ns::f(e);
+}
diff --git a/gcc/testsuite/g++.dg/modules/using-18_a.C
b/gcc/testsuite/g++.dg/modules/using-18_a.C
new file mode 100644
index 00000000000..72c2bf8116d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-18_a.C
@@ -0,0 +1,29 @@
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi M }
+// Test revealing non-exported declarations still prevents
+// needed GMF declarations from being discarded
+
+module;
+
+struct A {};
+int f();
+
+namespace ns {
+ struct B {};
+ int g();
+}
+
+extern "C" int h();
+namespace ns {
+ extern "C" int h();
+}
+
+export module M;
+
+using ::A;
+using ::f;
+
+using ns::B;
+using ns::g;
+
+using ns::h;
diff --git a/gcc/testsuite/g++.dg/modules/using-18_b.C
b/gcc/testsuite/g++.dg/modules/using-18_b.C
new file mode 100644
index 00000000000..f5f608da964
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-18_b.C
@@ -0,0 +1,11 @@
+// { dg-additional-options "-fmodules-ts" }
+
+module M;
+
+void test() {
+ A a;
+ B b;
+ int x = f();
+ int y = g();
+ int z = h();
+}
