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)
+ {
+ auto_diagnostic_group d;
+ error_at (from_loc, "import of %qs within its own implementation unit",
+ import->get_flatname());
+ inform (import->loc, "module declared here");
+ return;
+ }
+
+ if (!import->check_circular_import (from_loc))
return;
if (!import->is_header () && current_lang_depth ())
@@ -21829,7 +21844,7 @@ import_module (module_state *import, location_t
from_loc, bool exporting_p,
from_loc = ordinary_loc_of (line_table, from_loc);
linemap_module_reparent (line_table, import->loc, from_loc);
}
- gcc_checking_assert (!import->module_p);
+
gcc_checking_assert (import->is_direct () && import->has_location ());
direct_import (import, reader);
@@ -22322,7 +22337,10 @@ preprocess_module (module_state *module, location_t
from_loc,
linemap_module_reparent (line_table, module->loc, from_loc);
else
{
- module->loc = from_loc;
+ /* Don't overwrite the location if we're importing ourselves
+ after already having seen a module-declaration. */
+ if (!(is_import && module->is_module ()))
+ module->loc = from_loc;
if (!module->flatname)
module->set_flatname ();
}
diff --git a/gcc/testsuite/g++.dg/modules/circ-1_c.C
b/gcc/testsuite/g++.dg/modules/circ-1_c.C
index cea17d7a2e0..8bb2ce16052 100644
--- a/gcc/testsuite/g++.dg/modules/circ-1_c.C
+++ b/gcc/testsuite/g++.dg/modules/circ-1_c.C
@@ -4,6 +4,6 @@ export module Bob; // { dg-message "declared here" }
import Kevin;
// { dg-error "failed to read" "" { target *-*-* } 0 }
-// { dg-error "cannot import module" "" { target *-*-* } 0 }
+// { dg-error "depends on itself" "" { target *-*-* } 0 }
// { dg-prune-output "fatal error:" }
// { dg-prune-output "compilation terminated" }
diff --git a/gcc/testsuite/g++.dg/modules/import-3_a.C
b/gcc/testsuite/g++.dg/modules/import-3_a.C
new file mode 100644
index 00000000000..8143da0eb86
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/import-3_a.C
@@ -0,0 +1,6 @@
+// PR c++/99682
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi foo }
+
+export module foo;
+export void foo();
diff --git a/gcc/testsuite/g++.dg/modules/import-3_b.C
b/gcc/testsuite/g++.dg/modules/import-3_b.C
new file mode 100644
index 00000000000..3c6a5d703c3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/import-3_b.C
@@ -0,0 +1,8 @@
+// PR c++/99682
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi bar }
+
+export module bar;
+import foo;
+
+export void bar();
diff --git a/gcc/testsuite/g++.dg/modules/import-3_c.C
b/gcc/testsuite/g++.dg/modules/import-3_c.C
new file mode 100644
index 00000000000..2508d6136c7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/import-3_c.C
@@ -0,0 +1,11 @@
+// PR c++/99682
+// { dg-additional-options "-fmodules" }
+
+module foo;
+import bar; // not an interface dependency
+import bar; // double import is not an error either
+
+void test() {
+ foo();
+ bar();
+}
diff --git a/gcc/testsuite/g++.dg/modules/mod-decl-1.C
b/gcc/testsuite/g++.dg/modules/mod-decl-1.C
index ac7bb84699a..45c540914a1 100644
--- a/gcc/testsuite/g++.dg/modules/mod-decl-1.C
+++ b/gcc/testsuite/g++.dg/modules/mod-decl-1.C
@@ -1,10 +1,10 @@
// { dg-additional-options "-fmodules-ts" }
module;
-export module frist;
+export module frist; // { dg-message "declared here" }
// { dg-module-cmi "!frist" }
-import frist; // { dg-error {cannot import module.* in its own purview} }
+import frist; // { dg-error {module 'frist' depends on itself} }
module foo.second; // { dg-error "only permitted as" }
diff --git a/gcc/testsuite/g++.dg/modules/mod-decl-2_b.C
b/gcc/testsuite/g++.dg/modules/mod-decl-2_b.C
index a3ea9b5aa6a..f01f0b06566 100644
--- a/gcc/testsuite/g++.dg/modules/mod-decl-2_b.C
+++ b/gcc/testsuite/g++.dg/modules/mod-decl-2_b.C
@@ -1,7 +1,7 @@
// { dg-additional-options "-fmodules-ts" }
module bob;
-import bob; // { dg-error "cannot import module.* in its own purview" }
+import bob; // { dg-error "import of 'bob' within its own" }
// module linkage
void Baz ()
diff --git a/gcc/testsuite/g++.dg/modules/mod-decl-9.C
b/gcc/testsuite/g++.dg/modules/mod-decl-9.C
new file mode 100644
index 00000000000..16e24a61c94
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/mod-decl-9.C
@@ -0,0 +1,13 @@
+// PR c++/121808
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi !foo }
+
+void test();
+
+export module foo; // { dg-error "module-declaration" }
+
+import foo;
+// { dg-error "failed to read compiled module" "" { target *-*-* } 0 }
+
+// { dg-prune-output "fatal error:" }
+// { dg-prune-output "compilation terminated" }
diff --git a/gcc/testsuite/g++.dg/modules/part-10_a.C
b/gcc/testsuite/g++.dg/modules/part-10_a.C
new file mode 100644
index 00000000000..056da7185e4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-10_a.C
@@ -0,0 +1,5 @@
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi foo }
+
+export module foo;
+void foo();
diff --git a/gcc/testsuite/g++.dg/modules/part-10_b.C
b/gcc/testsuite/g++.dg/modules/part-10_b.C
new file mode 100644
index 00000000000..62865d1732d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-10_b.C
@@ -0,0 +1,9 @@
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi foo:part }
+
+module foo:part;
+import foo;
+
+void part() {
+ foo();
+}
diff --git a/gcc/testsuite/g++.dg/modules/part-10_c.C
b/gcc/testsuite/g++.dg/modules/part-10_c.C
new file mode 100644
index 00000000000..7f0a1ea1c7c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-10_c.C
@@ -0,0 +1,10 @@
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi foo:trans }
+
+module foo:trans;
+import :part;
+
+void trans() {
+ foo();
+ part();
+}
diff --git a/gcc/testsuite/g++.dg/modules/part-10_d.C
b/gcc/testsuite/g++.dg/modules/part-10_d.C
new file mode 100644
index 00000000000..2c0c3ffa4e4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-10_d.C
@@ -0,0 +1,10 @@
+// { dg-additional-options "-fmodules" }
+
+module foo;
+import :trans;
+
+void impl() {
+ foo();
+ part();
+ trans();
+}
diff --git a/gcc/testsuite/g++.dg/modules/pr99174.H
b/gcc/testsuite/g++.dg/modules/pr99174.H
index 60d01c59c77..1ad20b71b7a 100644
--- a/gcc/testsuite/g++.dg/modules/pr99174.H
+++ b/gcc/testsuite/g++.dg/modules/pr99174.H
@@ -1,3 +1,3 @@
// { dg-additional-options -fmodule-header }
// { dg-module-cmi !{} }
-import "pr99174.H"; // { dg-error "cannot import" }
+import "pr99174.H"; // { dg-error "depends on itself" }
--
2.51.0
