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

Message ID ca4d1a2f-9fe3-ce67-d83d-f5011f14e888@acm.org
State New
Headers show
Series
  • c++: Recursing header imports and other duplicates [PR 99174]
Related show

Commit Message

Nathan Sidwell Feb. 22, 2021, 3:51 p.m. 
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.

-- 
Nathan Sidwell

Patch
diff mbox series 

diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc
index 3d17b8ddcdb..09a9ca8778b 100644
--- c/gcc/cp/module.cc
+++ w/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)
@@ -19304,16 +19306,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);
@@ -19325,15 +19327,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;
 	}
     }
 
@@ -19443,7 +19443,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 c/gcc/testsuite/g++.dg/modules/pr99174-1_a.C w/gcc/testsuite/g++.dg/modules/pr99174-1_a.C
new file mode 100644
index 00000000000..c22b45bff45
--- /dev/null
+++ w/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 c/gcc/testsuite/g++.dg/modules/pr99174-1_b.C w/gcc/testsuite/g++.dg/modules/pr99174-1_b.C
new file mode 100644
index 00000000000..aaa0a9492ad
--- /dev/null
+++ w/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 c/gcc/testsuite/g++.dg/modules/pr99174-1_c.C w/gcc/testsuite/g++.dg/modules/pr99174-1_c.C
new file mode 100644
index 00000000000..58936f292f8
--- /dev/null
+++ w/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 c/gcc/testsuite/g++.dg/modules/pr99174.H w/gcc/testsuite/g++.dg/modules/pr99174.H
new file mode 100644
index 00000000000..62d5513b19a
--- /dev/null
+++ w/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} }