On 9/6/25 7:27 AM, Nathaniel Shead wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK.

-- >8 --

Currently we represent exported using-directives as a list of indices
into the namespace array that we stream.  However this list of
namespaces doesn't include any namespaces that we don't expose in this
module's purview, and so we ICE.

This patch reworks the handling to instead use the existing depset
tracking for namespaces directly.  This means that we don't need to
build up a second lookup map when streaming, and we can reuse the logic
in {read,write}_namespace.  We do need to make sure that we create a
depset for namespaces only referenced by a using-directive, though.

I don't expect to be exporting large numbers of using-directives from a
namespace, so for simplicity we stream the names as {parent, target}
pairs.

This also adjusts read handling so that we load the using-directives for
any import (including indirect) if it's in the import list for the
current TU.  Otherwise we run into issues if the using-directive is in
a namespace that is otherwise never referenced in the 'export import'ing
module, because we never walk this namespace and so never know that we
need to emit it.  To do this the patch ensures that we calculate the
import list before read_language is called.

As a drive-by fix, I noticed that with modules 'add_using_namespace'
will add duplicate using-directives because we compare usings against
the target namespace, but we then push a wrapping USING_DECL instead.
This reworks so that the contents of the structure is equivalent between
modules and non-modules code.

        PR c++/121702

gcc/cp/ChangeLog:

        * module.cc (enum module_state_counts): New counter.
        (depset::hash::add_namespace_entities): Seed using-directive
        targets for later streaming.
        (module_state::write_namespaces): Don't handle using-directives
        here.
        (module_state::read_namespaces): Likewise.
        (module_state::write_using_directives): New function.
        (module_state::read_using_directives): New function.
        (module_state::write_counts): Log using-directives.
        (module_state::read_counts): Likewise.
        (module_state::write_begin): Stream using-directives.
        (module_state::read_language): Read using-directives if
        directly importing.
        (module_state::direct_import): Update current TU import list
        before calling read_language.
        * name-lookup.cc (add_using_namespace): Fix lookup of previous
        using-directives.
        * parser.cc (cp_parser_import_declaration): Don't set
        MK_EXPORTING when performing import_module.

gcc/testsuite/ChangeLog:

        * g++.dg/modules/namespace-10_c.C: Add check for log dump.
        * g++.dg/modules/namespace-13_a.C: New test.
        * g++.dg/modules/namespace-13_b.C: New test.
        * g++.dg/modules/namespace-13_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
  gcc/cp/module.cc                              | 176 ++++++++++--------
  gcc/cp/name-lookup.cc                         |  32 +++-
  gcc/cp/parser.cc                              |   6 -
  gcc/testsuite/g++.dg/modules/namespace-10_c.C |   4 +-
  gcc/testsuite/g++.dg/modules/namespace-13_a.C |  16 ++
  gcc/testsuite/g++.dg/modules/namespace-13_b.C |  32 ++++
  gcc/testsuite/g++.dg/modules/namespace-13_c.C |  17 ++
  7 files changed, 193 insertions(+), 90 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/modules/namespace-13_a.C
  create mode 100644 gcc/testsuite/g++.dg/modules/namespace-13_b.C
  create mode 100644 gcc/testsuite/g++.dg/modules/namespace-13_c.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index ee76dd863d7..d9426936111 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3633,6 +3633,7 @@ enum module_state_counts
    MSC_pendings,
    MSC_entities,
    MSC_namespaces,
+  MSC_using_directives,
    MSC_bindings,
    MSC_macros,
    MSC_inits,
@@ -3854,6 +3855,10 @@ class GTY((chain_next ("%h.parent"), for_user)) 
module_state {
                         unsigned, unsigned *crc_ptr);
    bool read_namespaces (unsigned);
+ unsigned write_using_directives (elf_out *to, depset::hash &,
+                                  vec<depset *> spaces, unsigned *crc_ptr);
+  bool read_using_directives (unsigned);
+
    void intercluster_seed (trees_out &sec, unsigned index, depset *dep);
    unsigned write_cluster (elf_out *to, depset *depsets[], unsigned size,
                          depset::hash &, unsigned *counts, unsigned *crc_ptr);
@@ -14640,17 +14645,22 @@ depset::hash::add_namespace_entities (tree ns, bitmap 
partitions)
    data.partitions = partitions;
    data.hash = this;
- hash_table<named_decl_hash>::iterator end
-    (DECL_NAMESPACE_BINDINGS (ns)->end ());
-  for (hash_table<named_decl_hash>::iterator iter
-        (DECL_NAMESPACE_BINDINGS (ns)->begin ()); iter != end; ++iter)
+  for (tree binding : *DECL_NAMESPACE_BINDINGS (ns))
      {
        data.binding = nullptr;
        data.met_namespace = false;
-      if (walk_module_binding (*iter, partitions, add_binding_entity, &data))
+      if (walk_module_binding (binding, partitions, add_binding_entity, &data))
        count++;
      }
+ /* Seed any using-directives so that we emit the relevant namespaces. */
+  for (tree udir : NAMESPACE_LEVEL (ns)->using_directives)
+    if (TREE_CODE (udir) == USING_DECL && DECL_MODULE_EXPORT_P (udir))
+      {
+       make_dependency (USING_DECL_DECLS (udir), depset::EK_NAMESPACE);
+       count++;
+      }
+
    if (count)
      dump () && dump ("Found %u entries", count);
    dump.outdent ();
@@ -17074,15 +17084,11 @@ 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);
@@ -17124,31 +17130,6 @@ 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 ();
  }
@@ -17167,8 +17148,6 @@ 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 ();
@@ -17230,8 +17209,6 @@ 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))
@@ -17247,41 +17224,79 @@ module_state::read_namespaces (unsigned num)
        }
      }
- /* Read the exported using-directives. */
-  while (unsigned ix = sec.u ())
+  dump.outdent ();
+  if (!sec.end (from ()))
+    return false;
+  return true;
+}
+
+unsigned
+module_state::write_using_directives (elf_out *to, depset::hash &table,
+                                     vec<depset *> spaces, unsigned *crc_p)
+{
+  dump () && dump ("Writing using-directives");
+  dump.indent ();
+
+  bytes_out sec (to);
+  sec.begin ();
+
+  unsigned num = 0;
+  auto emit_one_ns = [&](depset *parent_dep)
      {
-      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)
+      tree parent = parent_dep->get_entity ();
+      for (auto udir : NAMESPACE_LEVEL (parent)->using_directives)
        {
-         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);
+         if (TREE_CODE (udir) != USING_DECL || !DECL_MODULE_EXPORT_P (udir))
+           continue;
+         tree target = USING_DECL_DECLS (udir);
+         depset *target_dep = table.find_dependency (target);
+         gcc_checking_assert (target_dep);
+
+         dump () && dump ("Writing using-directive in %N for %N",
+                          parent, target);
+         write_namespace (sec, parent_dep);
+         write_namespace (sec, target_dep);
+         ++num;
        }
-      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);
+    };
+
+  emit_one_ns (table.find_dependency (global_namespace));
+  for (depset *parent_dep : spaces)
+    emit_one_ns (parent_dep);
+
+  sec.end (to, to->name (MOD_SNAME_PFX ".udi"), crc_p);
+  dump.outdent ();
+
+  return num;
+}
+
+bool
+module_state::read_using_directives (unsigned num)
+{
+  if (!bitmap_bit_p ((*modules)[0]->imports, mod))
+    {
+      dump () && dump ("Ignoring using-directives because module %M "
+                      "is not visible in this TU", this);
+      return true;
+    }
+
+  bytes_in sec;
+
+  if (!sec.begin (loc, from (), MOD_SNAME_PFX ".udi"))
+    return false;
+
+  dump () && dump ("Reading using-directives");
+  dump.indent ();
+
+  for (unsigned ix = 0; ix != num; ++ix)
+    {
+      tree parent = read_namespace (sec);
+      tree target = read_namespace (sec);
+      if (sec.get_overrun ())
+       break;
+
+      dump () && dump ("Read using-directive in %N for %N", parent, target);
+      add_using_namespace (parent, target);
      }
dump.outdent ();
@@ -19803,6 +19818,7 @@ module_state::write_counts (elf_out *to, unsigned 
counts[MSC_HWM],
        dump ("Pendings %u", counts[MSC_pendings]);
        dump ("Entities %u", counts[MSC_entities]);
        dump ("Namespaces %u", counts[MSC_namespaces]);
+      dump ("Using-directives %u", counts[MSC_using_directives]);
        dump ("Macros %u", counts[MSC_macros]);
        dump ("Initializers %u", counts[MSC_inits]);
      }
@@ -19829,6 +19845,7 @@ module_state::read_counts (unsigned counts[MSC_HWM])
        dump ("Pendings %u", counts[MSC_pendings]);
        dump ("Entities %u", counts[MSC_entities]);
        dump ("Namespaces %u", counts[MSC_namespaces]);
+      dump ("Using-directives %u", counts[MSC_using_directives]);
        dump ("Macros %u", counts[MSC_macros]);
        dump ("Initializers %u", counts[MSC_inits]);
      }
@@ -20111,7 +20128,8 @@ ool_cmp (const void *a_, const void *b_)
       qualified-names      : binding value(s)
       MOD_SNAME_PFX.README   : human readable, strings
       MOD_SNAME_PFX.ENV      : environment strings, strings
-     MOD_SNAME_PFX.nms             : namespace hierarchy
+     MOD_SNAME_PFX.nms      : namespace hierarchy
+     MOD_SNAME_PFX.udi      : namespace using-directives
       MOD_SNAME_PFX.bnd      : binding table
       MOD_SNAME_PFX.spc      : specialization table
       MOD_SNAME_PFX.imp      : import table
@@ -20357,6 +20375,11 @@ module_state::write_begin (elf_out *to, cpp_reader 
*reader,
    if (counts[MSC_namespaces])
      write_namespaces (to, spaces, counts[MSC_namespaces], &crc);
+ /* Write any using-directives. */
+  if (counts[MSC_namespaces])
+    counts[MSC_using_directives]
+      = write_using_directives (to, table, spaces, &crc);
+
    /* Write the bindings themselves.  */
    counts[MSC_bindings] = write_bindings (to, sccs, &crc);
@@ -20608,11 +20631,16 @@ module_state::read_language (bool outermost)
                         counts[MSC_sec_lwm], counts[MSC_sec_hwm]))
      ok = false;
- /* Read the namespace hierarchy. */
+  /* Read the namespace hierarchy.  */
    if (ok && counts[MSC_namespaces]
        && !read_namespaces (counts[MSC_namespaces]))
      ok = false;
+ /* Read any using-directives. */
+  if (ok && counts[MSC_using_directives]
+      && !read_using_directives (counts[MSC_using_directives]))
+    ok = false;
+
    if (ok && !read_bindings (counts[MSC_bindings],
                            counts[MSC_sec_lwm], counts[MSC_sec_hwm]))
      ok = false;
@@ -21699,6 +21727,8 @@ direct_import (module_state *import, cpp_reader *reader)
      if (!import->do_import (reader, true))
        gcc_unreachable ();
+ (*modules)[0]->set_import (import, import->exported_p);
+
    if (import->loadedness < ML_LANGUAGE)
      {
        if (!keyed_table)
@@ -21706,8 +21736,6 @@ direct_import (module_state *import, cpp_reader *reader)
        import->read_language (true);
      }
- (*modules)[0]->set_import (import, import->exported_p);
-
    dump.pop (n);
    timevar_stop (TV_MODULE_IMPORT);
  }
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 9a2d0747ba4..43b95bffecf 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -8957,20 +8957,34 @@ pop_nested_namespace (tree ns)
  static void
  add_using_namespace (vec<tree, va_gc> *&usings, tree target)
  {
+  /* Find if this using already exists.  */
+  tree old = NULL_TREE;
    if (usings)
-    for (unsigned ix = usings->length (); ix--;)
-      if ((*usings)[ix] == target)
-       return;
+    for (tree t : *usings)
+      if (USING_DECL_DECLS (t) == target)
+       {
+         old = t;
+         break;
+       }
+ tree decl = old;
+  if (!decl)
+    {
+      decl = build_lang_decl (USING_DECL, NULL_TREE, NULL_TREE);
+      USING_DECL_DECLS (decl) = target;
+    }
+
+  /* Update purviewness and exportedness in case that has changed.  */
    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;
+      if (module_purview_p ())
+       DECL_MODULE_PURVIEW_P (decl) = true;
+      if (module_exporting_p ())
+       DECL_MODULE_EXPORT_P (decl) = true;
      }
-  vec_safe_push (usings, target);
+
+  if (!old)
+    vec_safe_push (usings, decl);
  }
/* Convenience overload for the above, taking the user as its first
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 9f9bd565004..8d1187f8cda 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -16267,13 +16267,7 @@ 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_c.C b/gcc/testsuite/g++.dg/modules/namespace-10_c.C
index aee00bdc438..7e03c19d246 100644
--- a/gcc/testsuite/g++.dg/modules/namespace-10_c.C
+++ b/gcc/testsuite/g++.dg/modules/namespace-10_c.C
@@ -1,6 +1,8 @@
-// { dg-additional-options -fmodules }
+// { dg-additional-options "-fmodules -fdump-lang-module" }
import M2; A::AT var1;
  AT var2;                      // { dg-error "AT" }
+
+// { dg-final { scan-lang-dump {Ignoring using-directives because module M1 is 
not visible} module } }
diff --git a/gcc/testsuite/g++.dg/modules/namespace-13_a.C 
b/gcc/testsuite/g++.dg/modules/namespace-13_a.C
new file mode 100644
index 00000000000..09255fd8bbb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-13_a.C
@@ -0,0 +1,16 @@
+// PR c++/121702
+// { dg-additional-options "-fmodules -fdump-lang-module" }
+// { dg-module-cmi a }
+
+export module a;
+export namespace a {
+  constexpr int f() { return 42; }
+}
+
+namespace x {}
+namespace y {
+  export using namespace x;
+}
+
+// { dg-final { scan-lang-dump {Using-directives 1} module } }
+// { dg-final { scan-lang-dump {Writing using-directive in '::y' for '::x'} 
module } }
diff --git a/gcc/testsuite/g++.dg/modules/namespace-13_b.C 
b/gcc/testsuite/g++.dg/modules/namespace-13_b.C
new file mode 100644
index 00000000000..1b309617814
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-13_b.C
@@ -0,0 +1,32 @@
+// PR c++/121702
+// { dg-additional-options "-fmodules -Wno-global-module -fdump-lang-module" }
+// { dg-module-cmi b }
+
+module;
+
+namespace gmf::blah {}
+namespace gmf::other {}
+
+export module b;
+export import a;
+// { dg-final { scan-lang-dump {Read using-directive in '::y' for '::x'} 
module } }
+
+export namespace b {
+  using namespace a;
+  using namespace gmf::blah;
+}
+namespace c {
+  using namespace gmf;
+  export using namespace gmf;
+  using namespace a;
+}
+
+// { dg-final { scan-lang-dump {Using-directives 3} module } }
+
+// { dg-final { scan-lang-dump {Writing using-directive in '::b' for '::a'} 
module } }
+// { dg-final { scan-lang-dump {Writing using-directive in '::b' for 
'::gmf::blah'} module } }
+// { dg-final { scan-lang-dump {Writing using-directive in '::c' for '::gmf'} 
module } }
+// { dg-final { scan-lang-dump-not {Writing using-directive in '::y' for 
'::x'} module } }
+
+// { dg-final { scan-lang-dump {Writing namespace:[0-9]* '::gmf::blah', 
public} module } }
+// { dg-final { scan-lang-dump-not {Writing namespace:[0-9]* '::gmf::other'} 
module } }
diff --git a/gcc/testsuite/g++.dg/modules/namespace-13_c.C 
b/gcc/testsuite/g++.dg/modules/namespace-13_c.C
new file mode 100644
index 00000000000..d04ef37cdbf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/namespace-13_c.C
@@ -0,0 +1,17 @@
+// PR c++/121702
+// { dg-additional-options "-fmodules" }
+
+import b;
+namespace gmf::blah {
+  constexpr int g() { return 123; }
+}
+namespace gmf::other {
+  constexpr int h() { return 99; }
+}
+namespace x {
+  constexpr int i() { return 5; }
+}
+static_assert(b::f() == 42);
+static_assert(b::g() == 123);
+static_assert(c::other::h() == 99);
+static_assert(y::i() == 5);

Reply via email to