Skip to content

fix(modgraph): partition import resolves to base module, not own partition#14

Merged
Sunrisepeak merged 1 commit intomainfrom
fix/partition-import-scanner
May 9, 2026
Merged

fix(modgraph): partition import resolves to base module, not own partition#14
Sunrisepeak merged 1 commit intomainfrom
fix/partition-import-scanner

Conversation

@Sunrisepeak
Copy link
Copy Markdown
Member

Summary

import :foo from a partition-interface unit (export module M:bar;)
was being resolved to M:bar:foo — the scanner prepended
provides->logicalName verbatim, which already contains the unit's
own :bar suffix. GCC's dyndep step still resolves the import
correctly (its own scan sees the right name), so builds succeed, but
mcpp prints

warning: <file>: module 'M:bar:foo' imported but not provided in this build

once for every such import. Reproduced under
mcpplibs/tinyhttps: seven
warnings per build.

Fix

 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;
 }

So for M:bar importing :foo we now produce M:foo (the sibling),
not M:bar:foo.

Coverage

Three new tests in test_modgraph.cpp:

  • Scanner.PartitionImportFromPrimaryInterface — regression guard
  • Scanner.PartitionImportFromAnotherPartition — the bug
  • Scanner.PartitionImportWithDottedModuleName — xpkg-style names

Verification

  • mcpp test — 9/9 unit binaries pass (incl. 3 new cases)
  • mcpp build of mcpplibs/tinyhttps (which exercises the bug
    with 7 import :partition sites) is now warning-free
  • CI green

…ition

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)
@Sunrisepeak Sunrisepeak merged commit ce8ff94 into main May 9, 2026
1 check passed
Sunrisepeak added a commit that referenced this pull request May 9, 2026
The v0.0.2 GitHub Release is being re-issued with the same version
number to fold in three improvements that landed against main *after*
the original tag was pushed:

* fix(modgraph): partition import scanner no longer concatenates the
  unit's own `:partition` into imported names (PR #14)
* feat: lib-root convention `src/<package-tail>.cppm` + optional
  `[lib].path` (PR #15)

CHANGELOG's [0.0.2] entry is amended in place to capture them, since
no v0.0.3 has been published yet and we're keeping the version number
stable while the project is still pre-1.0. After this PR merges, the
v0.0.2 git tag and GitHub Release will be re-pointed at the new HEAD.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant