Skip to content

Commit ce8ff94

Browse files
committed
fix(modgraph): partition import resolves to base module, not own partition
When a module-interface unit imports a sibling partition with `import :foo`, the scanner prepended its own `provides->logicalName` as-is. For a primary interface (`export module M;` → logicalName "M") that's correct: "M:foo". But for a *partition* unit (`export module M:bar;` → logicalName "M:bar") the same code produced "M:bar:foo" — which no other TU provides, so GCC's dyndep step prints a benign-but-noisy warning: <file>: module 'M:bar:foo' imported but not provided in this build (the build itself still succeeds because GCC resolves the import via its own scan; mcpp's warning comes from comparing scanner output to the producer map). Fix: strip our own `:partition` suffix from the base before prepending. before: M:bar + ":foo" → "M:bar:foo" (wrong) after: "M" + ":foo" → "M:foo" (correct sibling) Reproduced under mcpplibs/tinyhttps where `http.cppm`'s `export module mcpplibs.tinyhttps:http;` plus its `import :tls; import :socket;` etc. produced 7 stale "imported but not provided" warnings on every build. After this patch, `mcpp build` of tinyhttps is warning-free. Coverage: three new unit tests in `test_modgraph.cpp`: - `Scanner.PartitionImportFromPrimaryInterface` (regression guard for the working case) - `Scanner.PartitionImportFromAnotherPartition` (the actual bug) - `Scanner.PartitionImportWithDottedModuleName` (xpkg-style names)
1 parent 6d40b97 commit ce8ff94

2 files changed

Lines changed: 58 additions & 2 deletions

File tree

src/modgraph/scanner.cppm

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,18 @@ std::expected<SourceUnit, ScanError> scan_file(const std::filesystem::path& file
286286
++i;
287287
}
288288
if (name.empty()) continue;
289-
// Partition import within the same module: prepend its name.
289+
// Partition import within the same module: prepend the *base*
290+
// module name. If the current TU itself owns a partition (e.g.
291+
// its `export module foo:http;`), `u.provides->logicalName`
292+
// already includes that suffix — concatenating naively would
293+
// produce `foo:http:tls` instead of the intended `foo:tls`.
294+
// Strip our own `:partition` first.
290295
if (name.starts_with(":") && u.provides) {
291-
name = u.provides->logicalName + name;
296+
std::string base = u.provides->logicalName;
297+
if (auto p = base.find(':'); p != std::string::npos) {
298+
base.resize(p);
299+
}
300+
name = base + name;
292301
}
293302
u.requires_.push_back(ModuleId{name});
294303
continue;

tests/unit/test_modgraph.cpp

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,53 @@ export int answer();
4444
std::filesystem::remove_all(dir);
4545
}
4646

47+
TEST(Scanner, PartitionImportFromPrimaryInterface) {
48+
// Primary module interface: `export module foo;` → logicalName = "foo".
49+
// `import :tls;` resolves to "foo:tls".
50+
auto dir = make_tempdir("mcpp-scanner");
51+
write(dir / "src" / "foo.cppm", R"(export module foo;
52+
import :tls;
53+
)");
54+
auto u = scan_file(dir / "src" / "foo.cppm", "pkg");
55+
ASSERT_TRUE(u.has_value()) << u.error().format();
56+
ASSERT_EQ(u->requires_.size(), 1u);
57+
EXPECT_EQ(u->requires_[0].logicalName, "foo:tls");
58+
std::filesystem::remove_all(dir);
59+
}
60+
61+
TEST(Scanner, PartitionImportFromAnotherPartition) {
62+
// Partition interface: `export module foo:http;` → logicalName = "foo:http".
63+
// `import :tls;` must resolve to "foo:tls" (the sibling partition),
64+
// NOT "foo:http:tls" (which is what a naive prepend produces).
65+
auto dir = make_tempdir("mcpp-scanner");
66+
write(dir / "src" / "http.cppm", R"(export module foo:http;
67+
import :tls;
68+
import :socket;
69+
)");
70+
auto u = scan_file(dir / "src" / "http.cppm", "pkg");
71+
ASSERT_TRUE(u.has_value()) << u.error().format();
72+
ASSERT_TRUE(u->provides.has_value());
73+
EXPECT_EQ(u->provides->logicalName, "foo:http");
74+
ASSERT_EQ(u->requires_.size(), 2u);
75+
EXPECT_EQ(u->requires_[0].logicalName, "foo:tls");
76+
EXPECT_EQ(u->requires_[1].logicalName, "foo:socket");
77+
std::filesystem::remove_all(dir);
78+
}
79+
80+
TEST(Scanner, PartitionImportWithDottedModuleName) {
81+
// Dotted module names (xpkg-style, e.g. `mcpplibs.tinyhttps:http`)
82+
// — only the colon-prefixed partition suffix is what we strip.
83+
auto dir = make_tempdir("mcpp-scanner");
84+
write(dir / "src" / "http.cppm", R"(export module mcpplibs.tinyhttps:http;
85+
import :tls;
86+
)");
87+
auto u = scan_file(dir / "src" / "http.cppm", "pkg");
88+
ASSERT_TRUE(u.has_value()) << u.error().format();
89+
ASSERT_EQ(u->requires_.size(), 1u);
90+
EXPECT_EQ(u->requires_[0].logicalName, "mcpplibs.tinyhttps:tls");
91+
std::filesystem::remove_all(dir);
92+
}
93+
4794
TEST(Scanner, RejectsConditionalImport) {
4895
auto dir = make_tempdir("mcpp-scanner");
4996
write(dir / "main.cpp", R"(import std;

0 commit comments

Comments
 (0)