On 11/27/24 11:17 AM, Jason Merrill wrote:
On 11/27/24 1:43 AM, Nathaniel Shead wrote:
On Wed, Nov 27, 2024 at 12:03:23AM -0500, Jason Merrill wrote:
Tested x86_64-pc-linux-gnu.
Does this approach make sense to you? Any other ideas?
-- 8< --
We weren't representing 'using namespace' at all in modules, which broke
some of the <chrono> literals tests.
I experimented with various approaches to representing them, and
ended up
with emitting them as a pseudo-binding for "using", which as a
keyword can't
have any real bindings. Then reading this pseudo-binding adds it to
using_directives instead of the usual handling.
+ /* ??? should we try to distinguish whether the using-directive
+ is purview/exported? */
+ add_binding_entity (used, WMB_Flags(WMB_Using|WMB_Purview), &data);
I don't think the standard is entirely clear about how using-directives
should interact with modules; they don't declare names, and before P2615
were in fact forbidden from being explicitly exported, which implies to
me that the intention was for them to not be considered outside of the
declaring module.
P2615 is certainly clear about allowing them. Given that, I think the
general rules of [module.interface] apply, so it should be found by name
lookup in an importing TU.
That said, if we were to do this I would think the logic should match
what we do for any other name, in terms of requiring it to be explicitly
exported/purview as required; in particular, I would hope that something
like this doesn't happen:
// m.cpp
export module M;
using namespace std;
// test.cpp
#include <iostream>
import M;
int main() {
cout << "hello\n"; // using-directive "inherited" from M?
}
Good point, I have more work to do.
I think that since ADL doesn't consider using-directives, we only need
to represent the exported ones?
name_lookup::search_namespace_only (tree scope)
{
bool found = false;
+ if (modules_p () && name && !id_equal (name, "using"))
+ {
+ name_lookup u (get_identifier ("using"));
+ u.search_namespace_only (scope);
+ }
Could we just add to the list of using-directives within read_namespaces
perhaps? Probably as a second pass after all namespaces have been
created so that we don't run into issues with circular directives.
That would mean we wouldn't need to do this in every lookup.
That was my first thought, but I had trouble figuring out how. Perhaps
I'll try again.
Done thus. Any thoughts on this version?
From e4711055f683faa2ae747507dfe8b1d51fe26760 Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Wed, 20 Nov 2024 23:46:54 +0100
Subject: [PATCH] c++: modules and using-directives
To: gcc-patches@gcc.gnu.org
We weren't representing 'using namespace' at all in modules, which broke
some of the <chrono> literals tests.
This only represents exported using-declarations; others should be
irrelevant to importers, as any name lookup in the imported module that
would have cared about them was done while compiling the header unit.
I experimented with various approaches to representing them; this patch
handles them in read/write_namespaces, after the namespaces themselves. I
spent a while pondering how to deal with the depset code in order to connect
them, but then realized it would be simpler to refer to them based on their
index in the array of namespaces.
Any using-directives from an indirect import are ignored, so in an export
import, any imported using-directives are exported again.
gcc/cp/ChangeLog:
* module.cc (module_state::write_namespaces): Write
using-directives.
(module_state::read_namespaces): And read them.
* name-lookup.cc (add_using_namespace): Add overload. Build a
USING_DECL for modules.
(name_lookup::search_usings, name_lookup::queue_usings)
(using_directives_contain_std_p): Strip the USING_DECL.
* name-lookup.h: Declare it.
* parser.cc (cp_parser_import_declaration): Set MK_EXPORTING
for export import.
gcc/testsuite/ChangeLog:
* g++.dg/modules/namespace-8_a.C: New test.
* g++.dg/modules/namespace-8_b.C: New test.
* g++.dg/modules/namespace-9_a.C: New test.
* g++.dg/modules/namespace-9_b.C: New test.
* g++.dg/modules/namespace-10_a.C: New test.
* g++.dg/modules/namespace-10_b.C: New test.
* g++.dg/modules/namespace-10_c.C: New test.
* g++.dg/modules/namespace-11_a.C: New test.
* g++.dg/modules/namespace-11_b.C: New test.
* g++.dg/modules/namespace-11_c.C: New test.
---
gcc/cp/name-lookup.h | 1 +
gcc/cp/module.cc | 71 +++++++++++++++++++
gcc/cp/name-lookup.cc | 24 ++++++-
gcc/cp/parser.cc | 6 ++
gcc/testsuite/g++.dg/modules/namespace-10_a.C | 11 +++
gcc/testsuite/g++.dg/modules/namespace-10_b.C | 9 +++
gcc/testsuite/g++.dg/modules/namespace-10_c.C | 6 ++
gcc/testsuite/g++.dg/modules/namespace-11_a.C | 11 +++
gcc/testsuite/g++.dg/modules/namespace-11_b.C | 9 +++
gcc/testsuite/g++.dg/modules/namespace-11_c.C | 6 ++
gcc/testsuite/g++.dg/modules/namespace-8_a.C | 12 ++++
gcc/testsuite/g++.dg/modules/namespace-8_b.C | 8 +++
gcc/testsuite/g++.dg/modules/namespace-9_a.C | 11 +++
gcc/testsuite/g++.dg/modules/namespace-9_b.C | 6 ++
14 files changed, 188 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/namespace-10_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/namespace-10_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/namespace-10_c.C
create mode 100644 gcc/testsuite/g++.dg/modules/namespace-11_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/namespace-11_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/namespace-11_c.C
create mode 100644 gcc/testsuite/g++.dg/modules/namespace-8_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/namespace-8_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/namespace-9_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/namespace-9_b.C
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index f1596c60c7a..4216a515afa 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -462,6 +462,7 @@ extern cxx_binding *outer_binding (tree, cxx_binding *, bool);
extern void cp_emit_debug_info_for_using (tree, tree);
extern void finish_nonmember_using_decl (tree scope, tree name);
+extern void add_using_namespace (tree, tree);
extern void finish_using_directive (tree target, tree attribs);
void push_local_extern_decl_alias (tree decl);
extern tree pushdecl (tree, bool hiding = false);
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 17c040d26b0..ad2acaf559f 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -16865,11 +16865,15 @@ module_state::write_namespaces (elf_out *to, vec<depset *> spaces,
bytes_out sec (to);
sec.begin ();
+ hash_map<tree, unsigned> ns_map;
+
for (unsigned ix = 0; ix != num; ix++)
{
depset *b = spaces[ix];
tree ns = b->get_entity ();
+ ns_map.put (ns, ix);
+
/* This could be an anonymous namespace even for a named module,
since we can still emit no-linkage decls. */
gcc_checking_assert (TREE_CODE (ns) == NAMESPACE_DECL);
@@ -16911,6 +16915,31 @@ module_state::write_namespaces (elf_out *to, vec<depset *> spaces,
}
}
+ /* Now write exported using-directives, as a sequence of 1-origin indices in
+ the spaces array (not entity indices): First the using namespace, then the
+ used namespaces. And then a zero terminating the list. :: is
+ represented as index -1. */
+ auto emit_one_ns = [&](unsigned ix, tree ns) {
+ for (auto udir: NAMESPACE_LEVEL (ns)->using_directives)
+ {
+ if (TREE_CODE (udir) != USING_DECL || !DECL_MODULE_EXPORT_P (udir))
+ continue;
+ tree ns2 = USING_DECL_DECLS (udir);
+ dump() && dump ("Writing using-directive in %N for %N",
+ ns, ns2);
+ sec.u (ix);
+ sec.u (*ns_map.get (ns2) + 1);
+ }
+ };
+ emit_one_ns (-1, global_namespace);
+ for (unsigned ix = 0; ix != num; ix++)
+ {
+ depset *b = spaces[ix];
+ tree ns = b->get_entity ();
+ emit_one_ns (ix + 1, ns);
+ }
+ sec.u (0);
+
sec.end (to, to->name (MOD_SNAME_PFX ".nms"), crc_p);
dump.outdent ();
}
@@ -16929,6 +16958,8 @@ module_state::read_namespaces (unsigned num)
dump () && dump ("Reading namespaces");
dump.indent ();
+ tree *ns_map = XALLOCAVEC (tree, num);
+
for (unsigned ix = 0; ix != num; ix++)
{
unsigned entity_index = sec.u ();
@@ -16990,6 +17021,8 @@ module_state::read_namespaces (unsigned num)
DECL_ATTRIBUTES (inner)
= tree_cons (get_identifier ("abi_tag"), tags, DECL_ATTRIBUTES (inner));
+ ns_map[ix] = inner;
+
/* Install the namespace. */
(*entity_ary)[entity_lwm + entity_index] = inner;
if (DECL_MODULE_IMPORT_P (inner))
@@ -17004,6 +17037,44 @@ module_state::read_namespaces (unsigned num)
*slot = entity_lwm + entity_index;
}
}
+
+ /* Read the exported using-directives. */
+ while (unsigned ix = sec.u ())
+ {
+ tree ns;
+ if (ix == (unsigned)-1)
+ ns = global_namespace;
+ else
+ {
+ if (--ix >= num)
+ {
+ sec.set_overrun ();
+ break;
+ }
+ ns = ns_map [ix];
+ }
+ unsigned ix2 = sec.u ();
+ if (--ix2 >= num)
+ {
+ sec.set_overrun ();
+ break;
+ }
+ tree ns2 = ns_map [ix2];
+ if (directness)
+ {
+ dump() && dump ("Reading using-directive in %N for %N",
+ ns, ns2);
+ /* In an export import this will mark the using-directive as
+ exported, so it will be emitted again. */
+ add_using_namespace (ns, ns2);
+ }
+ else
+ /* Ignore using-directives from indirect imports, we only want them
+ from our own imports. */
+ dump() && dump ("Ignoring using-directive in %N for %N",
+ ns, ns2);
+ }
+
dump.outdent ();
if (!sec.end (from ()))
return false;
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 84b5e673a6d..9aa7c16f64b 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -1049,7 +1049,7 @@ name_lookup::search_usings (tree scope)
bool found = false;
if (vec<tree, va_gc> *usings = NAMESPACE_LEVEL (scope)->using_directives)
for (unsigned ix = usings->length (); ix--;)
- found |= search_qualified ((*usings)[ix], true);
+ found |= search_qualified (strip_using_decl ((*usings)[ix]), true);
/* Look in its inline children. */
if (vec<tree, va_gc> *inlinees = DECL_NAMESPACE_INLINEES (scope))
@@ -1121,7 +1121,7 @@ name_lookup::queue_usings (using_queue& queue, int depth, vec<tree, va_gc> *usin
{
if (usings)
for (unsigned ix = usings->length (); ix--;)
- queue_namespace (queue, depth, (*usings)[ix]);
+ queue_namespace (queue, depth, strip_using_decl ((*usings)[ix]));
}
/* Unqualified namespace lookup in SCOPE.
@@ -6868,7 +6868,7 @@ using_directives_contain_std_p (vec<tree, va_gc> *usings)
return false;
for (unsigned ix = usings->length (); ix--;)
- if ((*usings)[ix] == std_node)
+ if (strip_using_decl ((*usings)[ix]) == std_node)
return true;
return false;
@@ -8943,9 +8943,27 @@ add_using_namespace (vec<tree, va_gc> *&usings, tree target)
if ((*usings)[ix] == target)
return;
+ if (modules_p ())
+ {
+ tree u = build_lang_decl (USING_DECL, NULL_TREE, NULL_TREE);
+ USING_DECL_DECLS (u) = target;
+ DECL_MODULE_EXPORT_P (u) = module_exporting_p ();
+ DECL_MODULE_PURVIEW_P (u) = module_purview_p ();
+ target = u;
+ }
vec_safe_push (usings, target);
}
+/* Convenience overload for the above, taking the user as its first
+ parameter. */
+
+void
+add_using_namespace (tree ns, tree target)
+{
+ add_using_namespace (NAMESPACE_LEVEL (ns)->using_directives,
+ ORIGINAL_NAMESPACE (target));
+}
+
/* Tell the debug system of a using directive. */
static void
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 1fb9e7fd872..d9fc5f76bc3 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -15827,7 +15827,13 @@ cp_parser_import_declaration (cp_parser *parser, module_parse mp_state,
" must not be from header inclusion");
}
+ auto mk = module_kind;
+ if (exporting)
+ module_kind |= MK_EXPORTING;
+
import_module (mod, token->location, exporting, attrs, parse_in);
+
+ module_kind = mk;
}
}
diff --git a/gcc/testsuite/g++.dg/modules/namespace-10_a.C b/gcc/testsuite/g++.dg/modules/namespace-10_a.C
new file mode 100644
index 00000000000..c67c93a1d34
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-10_a.C
@@ -0,0 +1,11 @@
+// { dg-additional-options -fmodules }
+
+export module M1;
+
+namespace A
+{
+ export using AT = int;
+}
+
+export using namespace A;
+inline AT var;
diff --git a/gcc/testsuite/g++.dg/modules/namespace-10_b.C b/gcc/testsuite/g++.dg/modules/namespace-10_b.C
new file mode 100644
index 00000000000..5a3558da342
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-10_b.C
@@ -0,0 +1,9 @@
+// { dg-additional-options "-fmodules" }
+
+export module M2;
+import M1;
+
+namespace A
+{
+ export using A::AT;
+}
diff --git a/gcc/testsuite/g++.dg/modules/namespace-10_c.C b/gcc/testsuite/g++.dg/modules/namespace-10_c.C
new file mode 100644
index 00000000000..aee00bdc438
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-10_c.C
@@ -0,0 +1,6 @@
+// { dg-additional-options -fmodules }
+
+import M2;
+
+A::AT var1;
+AT var2; // { dg-error "AT" }
diff --git a/gcc/testsuite/g++.dg/modules/namespace-11_a.C b/gcc/testsuite/g++.dg/modules/namespace-11_a.C
new file mode 100644
index 00000000000..c67c93a1d34
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-11_a.C
@@ -0,0 +1,11 @@
+// { dg-additional-options -fmodules }
+
+export module M1;
+
+namespace A
+{
+ export using AT = int;
+}
+
+export using namespace A;
+inline AT var;
diff --git a/gcc/testsuite/g++.dg/modules/namespace-11_b.C b/gcc/testsuite/g++.dg/modules/namespace-11_b.C
new file mode 100644
index 00000000000..c0e0b9e6c70
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-11_b.C
@@ -0,0 +1,9 @@
+// { dg-additional-options "-fmodules" }
+
+export module M2;
+export import M1;
+
+namespace A
+{
+ export using A::AT;
+}
diff --git a/gcc/testsuite/g++.dg/modules/namespace-11_c.C b/gcc/testsuite/g++.dg/modules/namespace-11_c.C
new file mode 100644
index 00000000000..e12a8aea4be
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-11_c.C
@@ -0,0 +1,6 @@
+// { dg-additional-options -fmodules }
+
+import M2;
+
+A::AT var1;
+AT var2;
diff --git a/gcc/testsuite/g++.dg/modules/namespace-8_a.C b/gcc/testsuite/g++.dg/modules/namespace-8_a.C
new file mode 100644
index 00000000000..67ffc6a8bfa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-8_a.C
@@ -0,0 +1,12 @@
+// { dg-additional-options "-fmodules" }
+
+export module M;
+
+export namespace B
+{
+ int i;
+}
+export namespace C
+{
+ using namespace B;
+}
diff --git a/gcc/testsuite/g++.dg/modules/namespace-8_b.C b/gcc/testsuite/g++.dg/modules/namespace-8_b.C
new file mode 100644
index 00000000000..7db35bf955e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-8_b.C
@@ -0,0 +1,8 @@
+// { dg-additional-options "-fmodules" }
+
+import M;
+
+int main()
+{
+ C::i = 42;
+}
diff --git a/gcc/testsuite/g++.dg/modules/namespace-9_a.C b/gcc/testsuite/g++.dg/modules/namespace-9_a.C
new file mode 100644
index 00000000000..4cc3ab0c315
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-9_a.C
@@ -0,0 +1,11 @@
+// { dg-additional-options -fmodules }
+
+export module M;
+
+namespace A
+{
+ export using AT = int;
+}
+
+using namespace A;
+inline AT var;
diff --git a/gcc/testsuite/g++.dg/modules/namespace-9_b.C b/gcc/testsuite/g++.dg/modules/namespace-9_b.C
new file mode 100644
index 00000000000..05bb300893e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-9_b.C
@@ -0,0 +1,6 @@
+// { dg-additional-options -fmodules }
+
+import M;
+
+A::AT var1;
+AT var2; // { dg-error "AT" }
--
2.49.0
