c++: Recursing header imports and other duplicates [PR 99174]

The fix for 98741 introduced two issues.  (a) recursive header units
iced because we tried to read the preprocessor state after having
failed to read the config.  (b) we could have duplicate imports of
named modules in our pending queue, and that would lead to duplicate
requests for pathnames, which coupled with the use of a null-pathname
to indicate we'd asked could lead to desynchronization with the module
mapper.  Fixed by adding a 'visited' flag to module state, so we ask
exactly once.

	PR c++/99174
	gcc/cp
	* module.cc (struct module_state): Add visited_p flag.
	(name_pending_imports): Use it to avoid duplicate requests.
	(preprocess_module): Don't read preprocessor state if we failed to
	load a module's config.
	gcc/testsuite/
	* g++.dg/modules/pr99174-1_a.C: New.
	* g++.dg/modules/pr99174-1_b.C: New.
	* g++.dg/modules/pr99174-1_c.C: New.
	* g++.dg/modules/pr99174.H: New.

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 7a40be3db353ce2bb78d9da5c8d32cf789b316ad..766f2ab853dc68c44aab48291789e0e030d0a926 100644 (file)
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3551,9 +3551,10 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state {
   bool call_init_p : 1; /* This module's global initializer needs
                           calling.  */
   bool inform_read_p : 1; /* Inform of a read.  */
+  bool visited_p : 1;    /* A walk-once flag. */
   /* Record extensions emitted or permitted.  */
   unsigned extensions : SE_BITS;
-  /* 13 bits used, 3 bits remain  */
+  /* 14 bits used, 2 bits remain  */
 
  public:
   module_state (tree name, module_state *, bool);
@@ -3787,6 +3788,7 @@ module_state::module_state (tree name, module_state *parent, bool partition)
   partition_p = partition;
 
   inform_read_p = false;
+  visited_p = false;
 
   extensions = 0;
   if (name && TREE_CODE (name) == STRING_CST)
@@ -19305,16 +19307,16 @@ name_pending_imports (cpp_reader *reader, bool at_end)
     {
       module_state *module = (*pending_imports)[ix];
       gcc_checking_assert (module->is_direct ());
-      if (!module->filename)
+      if (!module->filename
+         && !module->visited_p
+         && (module->is_header () || !only_headers))
        {
-         Cody::Flags flags
-           = (flag_preprocess_only ? Cody::Flags::None
-              : Cody::Flags::NameOnly);
+         module->visited_p = true;
+         Cody::Flags flags = (flag_preprocess_only
+                              ? Cody::Flags::None : Cody::Flags::NameOnly);
 
-         if (only_headers && !module->is_header ())
-           ;
-         else if (module->module_p
-                  && (module->is_partition () || module->exported_p))
+         if (module->module_p
+             && (module->is_partition () || module->exported_p))
            mapper->ModuleExport (module->get_flatname (), flags);
          else
            mapper->ModuleImport (module->get_flatname (), flags);
@@ -19326,15 +19328,13 @@ name_pending_imports (cpp_reader *reader, bool at_end)
   for (unsigned ix = 0; ix != pending_imports->length (); ix++)
     {
       module_state *module = (*pending_imports)[ix];
-      gcc_checking_assert (module->is_direct ());
-      if (only_headers && !module->is_header ())
-       ;
-      else if (!module->filename)
+      if (module->visited_p)
        {
-         Cody::Packet const &p = *r_iter;
-         ++r_iter;
+         module->visited_p = false;
+         gcc_checking_assert (!module->filename);
 
-         module->set_filename (p);
+         module->set_filename (*r_iter);
+         ++r_iter;
        }
     }
 
@@ -19444,7 +19444,8 @@ preprocess_module (module_state *module, location_t from_loc,
          /* Now read the preprocessor state of this particular
             import.  */
          unsigned n = dump.push (module);
-         if (module->read_preprocessor (true))
+         if (module->loadedness == ML_CONFIG
+             && module->read_preprocessor (true))
            module->import_macros ();
          dump.pop (n);
 

diff --git a/gcc/testsuite/g++.dg/modules/pr99174-1_a.C b/gcc/testsuite/g++.dg/modules/pr99174-1_a.C
new file mode 100644 (file)
index 0000000..c22b45b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99174-1_a.C
@@ -0,0 +1,8 @@
+// PR 99174 what if we import the same module twice (with no
+// intervening header import)?
+// { dg-additional-options -fmodules-ts }
+
+export module Foo;
+// { dg-module-cmi Foo }
+
+export void Foo ();

diff --git a/gcc/testsuite/g++.dg/modules/pr99174-1_b.C b/gcc/testsuite/g++.dg/modules/pr99174-1_b.C
new file mode 100644 (file)
index 0000000..aaa0a94
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99174-1_b.C
@@ -0,0 +1,6 @@
+// { dg-additional-options -fmodules-ts }
+
+export module Bar;
+// { dg-module-cmi Bar }
+
+export void Bar ();

diff --git a/gcc/testsuite/g++.dg/modules/pr99174-1_c.C b/gcc/testsuite/g++.dg/modules/pr99174-1_c.C
new file mode 100644 (file)
index 0000000..58936f2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99174-1_c.C
@@ -0,0 +1,11 @@
+// { dg-additional-options -fmodules-ts }
+
+import Foo;
+import Foo;
+import Bar;
+
+int main ()
+{
+  Foo ();
+  Bar ();
+}

diff --git a/gcc/testsuite/g++.dg/modules/pr99174.H b/gcc/testsuite/g++.dg/modules/pr99174.H
new file mode 100644 (file)
index 0000000..62d5513
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99174.H
@@ -0,0 +1,5 @@
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi !{} }
+import "pr99174.H"; // { dg-error "cannot import" }
+
+// { dg-prune-output {not writing module} }