From e7e47e769cf9f9efaa1a8d1283b73623e932097e Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Sat, 9 May 2026 08:15:48 +0800 Subject: [PATCH] fix(modgraph): partition import resolves to base module, not own partition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: : 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) --- src/modgraph/scanner.cppm | 13 ++++++++-- tests/unit/test_modgraph.cpp | 47 ++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/modgraph/scanner.cppm b/src/modgraph/scanner.cppm index 88b2402..37dcfc9 100644 --- a/src/modgraph/scanner.cppm +++ b/src/modgraph/scanner.cppm @@ -286,9 +286,18 @@ std::expected scan_file(const std::filesystem::path& file ++i; } if (name.empty()) continue; - // Partition import within the same module: prepend its name. + // Partition import within the same module: prepend the *base* + // module name. If the current TU itself owns a partition (e.g. + // its `export module foo:http;`), `u.provides->logicalName` + // already includes that suffix — concatenating naively would + // produce `foo:http:tls` instead of the intended `foo:tls`. + // Strip our own `:partition` first. if (name.starts_with(":") && u.provides) { - name = u.provides->logicalName + name; + std::string base = u.provides->logicalName; + if (auto p = base.find(':'); p != std::string::npos) { + base.resize(p); + } + name = base + name; } u.requires_.push_back(ModuleId{name}); continue; diff --git a/tests/unit/test_modgraph.cpp b/tests/unit/test_modgraph.cpp index 2c6fad2..f872b0f 100644 --- a/tests/unit/test_modgraph.cpp +++ b/tests/unit/test_modgraph.cpp @@ -44,6 +44,53 @@ export int answer(); std::filesystem::remove_all(dir); } +TEST(Scanner, PartitionImportFromPrimaryInterface) { + // Primary module interface: `export module foo;` → logicalName = "foo". + // `import :tls;` resolves to "foo:tls". + auto dir = make_tempdir("mcpp-scanner"); + write(dir / "src" / "foo.cppm", R"(export module foo; +import :tls; +)"); + auto u = scan_file(dir / "src" / "foo.cppm", "pkg"); + ASSERT_TRUE(u.has_value()) << u.error().format(); + ASSERT_EQ(u->requires_.size(), 1u); + EXPECT_EQ(u->requires_[0].logicalName, "foo:tls"); + std::filesystem::remove_all(dir); +} + +TEST(Scanner, PartitionImportFromAnotherPartition) { + // Partition interface: `export module foo:http;` → logicalName = "foo:http". + // `import :tls;` must resolve to "foo:tls" (the sibling partition), + // NOT "foo:http:tls" (which is what a naive prepend produces). + auto dir = make_tempdir("mcpp-scanner"); + write(dir / "src" / "http.cppm", R"(export module foo:http; +import :tls; +import :socket; +)"); + auto u = scan_file(dir / "src" / "http.cppm", "pkg"); + ASSERT_TRUE(u.has_value()) << u.error().format(); + ASSERT_TRUE(u->provides.has_value()); + EXPECT_EQ(u->provides->logicalName, "foo:http"); + ASSERT_EQ(u->requires_.size(), 2u); + EXPECT_EQ(u->requires_[0].logicalName, "foo:tls"); + EXPECT_EQ(u->requires_[1].logicalName, "foo:socket"); + std::filesystem::remove_all(dir); +} + +TEST(Scanner, PartitionImportWithDottedModuleName) { + // Dotted module names (xpkg-style, e.g. `mcpplibs.tinyhttps:http`) + // — only the colon-prefixed partition suffix is what we strip. + auto dir = make_tempdir("mcpp-scanner"); + write(dir / "src" / "http.cppm", R"(export module mcpplibs.tinyhttps:http; +import :tls; +)"); + auto u = scan_file(dir / "src" / "http.cppm", "pkg"); + ASSERT_TRUE(u.has_value()) << u.error().format(); + ASSERT_EQ(u->requires_.size(), 1u); + EXPECT_EQ(u->requires_[0].logicalName, "mcpplibs.tinyhttps:tls"); + std::filesystem::remove_all(dir); +} + TEST(Scanner, RejectsConditionalImport) { auto dir = make_tempdir("mcpp-scanner"); write(dir / "main.cpp", R"(import std;