On 9/9/25 4:49 PM, Nathaniel Shead wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
(If you prefer I could also split up each fix into separate commits.)

-- >8 --

This patch fixes some issues with import handling.

The main functional change is to allow importing a module's interface
file into its implementation file indirectly.  [module.import] p9
forbids an explicit 'import M;' declaration, but there's no provision
against having 'import X;' where module X itself imports M, so this
patch splits up the detection of circular imports to handle this better.
I also updated the errors to be closer to the standard wording.

A second issue I found while testing this is that we don't properly
handle name visibility when a partition implementation unit imports its
primary module interface (g++.dg/modules/part-10).  This is resolved by
setting 'module_p' on the primary interface when it gets imported.

Solving this I incidentally removed the assertion that PR121808 was
failing on, which was never valid: we can enter import_module for a
module previously seen as a module-declaration if parsing bails before
declare_module is called.  I experimented with guaranteeing that
declare_module always gets called for a module_state generated during
preprocessing if at all possible, but the resulting errors didn't seem a
lot better so I've left it as-is for now.

I did make a small adjustment so that a direct import of a module
doesn't overwrite the location of a module-declaration from earlier in
the file; this is important because preprocessing (and thus the
assigning of these locations) seems to happen for the whole file before
parsing begins, which can otherwise cause confusing locations referring
to later in the file than parsing would otherwise indicate.

        PR c++/99682
        PR c++/121808

gcc/cp/ChangeLog:

        * module.cc (class module_state): Rename check_not_purview.
        (module_state::check_not_purview): Rename to...
        (module_state::check_circular_import): ...this.  Only handle
        interface dependencies, adjust diagnostic message.
        (module_state::read_imports): Use new function.  Pass location
        of import as from_loc instead of the module location.
        (module_state::do_import): Set module_p when importing the
        primary interface for the current module.
        (import_module): Split out check for imports in own unit.
        Remove incorrect assertion.
        (preprocess_module): Don't overwrite module-decl location with
        later import.

gcc/testsuite/ChangeLog:

        * g++.dg/modules/circ-1_c.C: Adjust diagnostic.
        * g++.dg/modules/mod-decl-1.C: Likewise.
        * g++.dg/modules/mod-decl-2_b.C: Likewise.
        * g++.dg/modules/pr99174.H: Likewise.
        * g++.dg/modules/import-3_a.C: New test.
        * g++.dg/modules/import-3_b.C: New test.
        * g++.dg/modules/import-3_c.C: New test.
        * g++.dg/modules/mod-decl-9.C: New test.
        * g++.dg/modules/part-10_a.C: New test.
        * g++.dg/modules/part-10_b.C: New test.
        * g++.dg/modules/part-10_c.C: New test.
        * g++.dg/modules/part-10_d.C: New test.

Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
  gcc/cp/module.cc                            | 44 +++++++++++++++------
  gcc/testsuite/g++.dg/modules/circ-1_c.C     |  2 +-
  gcc/testsuite/g++.dg/modules/import-3_a.C   |  6 +++
  gcc/testsuite/g++.dg/modules/import-3_b.C   |  8 ++++
  gcc/testsuite/g++.dg/modules/import-3_c.C   | 11 ++++++
  gcc/testsuite/g++.dg/modules/mod-decl-1.C   |  4 +-
  gcc/testsuite/g++.dg/modules/mod-decl-2_b.C |  2 +-
  gcc/testsuite/g++.dg/modules/mod-decl-9.C   | 13 ++++++
  gcc/testsuite/g++.dg/modules/part-10_a.C    |  5 +++
  gcc/testsuite/g++.dg/modules/part-10_b.C    |  9 +++++
  gcc/testsuite/g++.dg/modules/part-10_c.C    | 10 +++++
  gcc/testsuite/g++.dg/modules/part-10_d.C    | 10 +++++
  gcc/testsuite/g++.dg/modules/pr99174.H      |  2 +-
  13 files changed, 108 insertions(+), 18 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/modules/import-3_a.C
  create mode 100644 gcc/testsuite/g++.dg/modules/import-3_b.C
  create mode 100644 gcc/testsuite/g++.dg/modules/import-3_c.C
  create mode 100644 gcc/testsuite/g++.dg/modules/mod-decl-9.C
  create mode 100644 gcc/testsuite/g++.dg/modules/part-10_a.C
  create mode 100644 gcc/testsuite/g++.dg/modules/part-10_b.C
  create mode 100644 gcc/testsuite/g++.dg/modules/part-10_c.C
  create mode 100644 gcc/testsuite/g++.dg/modules/part-10_d.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 50fc1af0354..ec9d84f11cc 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3782,7 +3782,7 @@ class GTY((chain_next ("%h.parent"), for_user)) 
module_state {
    }
public:
-  bool check_not_purview (location_t loc);
+  bool check_circular_import (location_t loc);
public:
    void mangle (bool include_partition);
@@ -15996,20 +15996,19 @@ recursive_lazy (unsigned snum = ~0u)
    return false;
  }
-/* If THIS is the current purview, issue an import error and return false. */
+/* If THIS has an interface dependency on itself, report an error and
+   return false.  */
bool
-module_state::check_not_purview (location_t from)
+module_state::check_circular_import (location_t from)
  {
-  module_state *imp = this_module ();
-  if (imp && !imp->name)
-    imp = imp->parent;
-  if (imp == this)
+  if (this == this_module ())
      {
        /* Cannot import the current module.  */
        auto_diagnostic_group d;
-      error_at (from, "cannot import module in its own purview");
-      inform (loc, "module %qs declared here", get_flatname ());
+      error_at (from, "module %qs depends on itself", get_flatname ());
+      if (!header_module_p ())
+       inform (loc, "module %qs declared here", get_flatname ());
        return false;
      }
    return true;
@@ -16320,7 +16319,7 @@ module_state::read_imports (bytes_in &sec, cpp_reader 
*reader, line_maps *lmaps)
          if (sec.get_overrun ())
            break;
- if (!imp->check_not_purview (loc))
+         if (!imp->check_circular_import (floc))
            continue;
if (imp->loadedness == ML_NONE)
@@ -21535,6 +21534,12 @@ module_state::do_import (cpp_reader *reader, bool 
outermost)
  {
    gcc_assert (global_namespace == current_scope () && loadedness == ML_NONE);
+ /* If this TU is a partition of the module we're importing,
+     that module is the primary module interface.  */
+  if (this_module ()->is_partition ()
+      && this == get_primary (this_module ()))
+    module_p = true;
+
    loc = linemap_module_loc (line_table, loc, get_flatname ());
if (lazy_open >= lazy_limit)
@@ -21807,7 +21812,17 @@ void
  import_module (module_state *import, location_t from_loc, bool exporting_p,
               tree, cpp_reader *reader)
  {
-  if (!import->check_not_purview (from_loc))
+  /* A non-partition implementation unit has no name.  */
+  if (!this_module ()->name && this_module ()->parent == import)

The patch is OK, but could you add a comment to module_state::parent? It's not immediately obvious what it means.

Jason

Reply via email to