Skip to content

Commit 9aa80d7

Browse files
committed
fix(manifest): the emptiness predicate is a free function, as its sibling is
Windows CI reported `test_modgraph.cpp` failing to compile with optional:262: error: no matching constructor for initialization of '_SMF_control<_Optional_construct_base<basic_string<char,...>>, ...>' against `types.cppm`'s `Profile`, a struct this change never touched. The chain is `Manifest` -> `std::map<std::string, Profile>` -> `Profile`'s `std::optional<std::string>`, and that member's own comment already records the same breakage under clang with the MSVC standard library on an earlier occasion. The cause was making the new emptiness predicate an inline MEMBER of two structs this module exports: that changes what importers materialise from its BMI, and what it materialised here was a copy constructor that does not compile. `append`, the operation this one is the sibling of, has been a free function since it was written. `is_empty` now matches it, and BuildInputs and ConditionalConfig have exactly the member sets they had before. Also records where the four new e2e tests run. All declare `# requires: gcc`, which `run_all.sh` grants only on Linux, so the end-to-end leg is Linux-only and the cross-dialect coverage -- including the MSVC `/ifcOutput` spelling that no Linux runner can reach -- is in the unit tests, which run everywhere.
1 parent c595e5c commit 9aa80d7

5 files changed

Lines changed: 67 additions & 26 deletions

File tree

.agents/docs/2026-09-09-two-answers-and-two-silences.md

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ Landed in mcpp 2026.9.9.1, one change per mechanism.
430430
| A.2 BOM | the same rule for every TOML document mcpp reads | `libs/toml.cppm` |
431431
| B | `module_lang` and `module_output` become PER-EDGE, bound from `providesModule` | `ninja_backend.cppm` |
432432
| B | `moduleImplLangFlag` — the spelling for a module-extension file that is not an interface | `toolchain-model/model.cppm` |
433-
| C | `BuildInputs::empty()` and `ConditionalConfig::empty()`, replacing two hand-written gates | `manifest/types.cppm`, `toml.cppm`, `xpkg.cppm` |
433+
| C | `is_empty(BuildInputs)` and `is_empty(ConditionalConfig)`, replacing two hand-written gates | `manifest/types.cppm`, `toml.cppm`, `xpkg.cppm` |
434434
| D | unknown-key sweeps for `[runtime]` and `[target.<pred>.runtime]` | `manifest/toml.cppm` |
435435

436436
Reading B's row against §3: the recommendation there was A, and the emitter now
@@ -451,12 +451,25 @@ READ. This is not a new restriction: GCC 16.1 refuses the same declaration with
451451
`unrecognized 'MODULE-EXPORT ...'`, so the two versions of mcpp differ in which
452452
sentence the author gets, not in whether the file builds.
453453

454-
`ConditionalConfig::empty()` composes rather than enumerating: it calls
455-
`BuildInputs::empty()` and `XlingsConfig::empty()`, the latter of which already
454+
`is_empty(ConditionalConfig)` composes rather than enumerating: it calls
455+
`is_empty(BuildInputs)` and `XlingsConfig::empty()`, the latter of which already
456456
existed. That is #258's medicine applied one level further out, and it closed a
457457
fourth instance found while writing it — `xpkg.cppm`'s gate omitted
458458
`privateIncludeDirs`, which its own loop fills.
459459

460+
**They are free functions, and that was forced by a measurement rather than
461+
chosen.** The first version made them members, which is the obvious shape and
462+
the wrong one: adding an inline member to a struct this module exports changes
463+
what importers materialise from its BMI, and `Profile` carries a
464+
`std::optional<std::string>` that is already known to break under clang with the
465+
MSVC standard library — the note on that member records the earlier occasion.
466+
Windows CI failed on `test_modgraph.cpp` with
467+
`no matching constructor for _SMF_control<_Optional_construct_base<...>>`,
468+
reported against a struct this change never touched, reached through
469+
`Manifest` -> `std::map<std::string, Profile>` -> `Profile`. `append`, the
470+
sibling operation, has been a free function since it was written; these now
471+
match it, and the structs' member sets are exactly what they were.
472+
460473
### Measurements after
461474

462475
Each row was measured before the change and after, on the same machine, with
@@ -482,7 +495,7 @@ fail on the reverted tree and all four new e2e tests fail against the released
482495
`CorrectlySpelledRuntimeKeysAreSilent` — which exist to fail if the new refusals
483496
are too broad, and are supposed to pass in both.
484497

485-
`ConditionalConfig::empty()` replaced a gate in the descriptor reader as well
498+
`is_empty(ConditionalConfig)` replaced a gate in the descriptor reader as well
486499
as in the manifest reader, and a descriptor is read by every consumer of the
487500
index rather than by one project. All 228 descriptors in `mcpplibs/mcpp-index`
488501
were parsed with both binaries and compared: 228 identical, 0 differing, 0
@@ -499,6 +512,17 @@ today. It is a correctness fix for a channel nothing currently exercises. The
499512
but it is not evidence about the gate, and recording it as though it were is
500513
the failure this record is otherwise about.
501514

515+
WHERE THE NEW e2e TESTS RUN, STATED RATHER THAN ASSUMED. All four declare
516+
`# requires: gcc`, and `run_all.sh` grants that capability only in its `Linux`
517+
branch — so they run on the Linux shards and are skipped on macOS and Windows.
518+
That is the right boundary for what they assert (three of the four defects are
519+
invisible on gcc, so their criteria are assertions on the emitted graph rather
520+
than on a build outcome, and the graph is the same everywhere), but it means the
521+
end-to-end leg exists on one platform only. The cross-dialect coverage is in the
522+
unit tests, which construct Clang, GCC and MSVC plans explicitly and run in
523+
every platform's unit job — including the MSVC spelling, whose empty-value
524+
symptom (`/ifcOutput` consuming the next token) no Linux runner could reach.
525+
502526
One denominator is worth recording because it is empty where a reader would
503527
expect it not to be: mcpp's own build emits **zero** `cxx_module` edges, since
504528
every module unit in it takes the two-phase split path (`cxx_precompile` /

CHANGELOG.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,12 @@ GCC 的接口拼法本就是纯语言,因此同一个工程在 GCC 上能构建
6262
结构体时没有同时加入它。于是该块被解析、被填充,然后丢弃;在同一谓词下随便再写一条
6363
无关的 `defines` 就能让它生效。
6464

65-
闸门改为 `ConditionalConfig::empty()`,定义在字段旁边,并由 mcpp.toml 与 xpkg 两个读者
66-
共用;它进一步组合 `BuildInputs::empty()` 与既有的 `XlingsConfig::empty()`。同一形状的
65+
闸门改为 `is_empty(ConditionalConfig)`,与既有的 `append` 并列为自由函数,并由 mcpp.toml
66+
与 xpkg 两个读者共用;它进一步组合 `is_empty(BuildInputs)` 与既有的 `XlingsConfig::empty()`
67+
自由函数而非成员是被实测逼出来的:给这个模块导出的结构体加内联成员会改变导入方从它的 BMI
68+
里实例化的东西,而 `Profile``std::optional<std::string>` 成员在 clang + MSVC 标准库下
69+
本就会崩——初版写成成员,Windows CI 上 `test_modgraph.cpp` 因此编译失败,报错指向一个本次
70+
改动从未碰过的结构体。同一形状的
6771
第四处随之关闭:xpkg 的闸门漏掉了它自己会填的 `privateIncludeDirs`。索引里 228 份
6872
descriptor 用新旧两个二进制解析,结果逐字节相同。
6973

modules/manifest/src/toml.cppm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2918,11 +2918,11 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
29182918
std::vector<std::string>{});
29192919
}
29202920
}
2921-
// `ConditionalConfig::empty()` and not a disjunction written here:
2921+
// `is_empty(ConditionalConfig)` and not a disjunction written here:
29222922
// this list omitted `libraries` and `linkLibraryDirs`, so a
29232923
// predicate carrying only a `[target.<pred>.runtime]` table was
29242924
// parsed and then dropped. See the note on that member.
2925-
if (!cc.empty())
2925+
if (!mcpp::manifest::is_empty(cc))
29262926
m.conditionalConfigs.push_back(std::move(cc));
29272927
}
29282928
}

modules/manifest/src/types.cppm

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -285,17 +285,6 @@ struct BuildInputs {
285285
// glibc and wrong for picolibc — and while this lived beside the package's
286286
// identity there was no spelling for that difference.
287287
std::vector<std::string> stdModuleFlags;
288-
289-
// Does this carry anything? The sibling of `append` below: that function
290-
// is the one place a contribution is combined, and this is the one place
291-
// it is weighed. Both enumerate every field, and both are here so a field
292-
// added above is a field the compiler puts in front of a reader twice.
293-
bool empty() const {
294-
return sources.empty() && cflags.empty() && cxxflags.empty()
295-
&& ldflags.empty() && defines.empty() && globFlags.empty()
296-
&& includeDirs.empty() && includeDirsAfter.empty()
297-
&& privateIncludeDirs.empty() && stdModuleFlags.empty();
298-
}
299288
};
300289

301290
// The single additive merge. Every conditional axis folds through this, so
@@ -322,6 +311,27 @@ inline void append(BuildInputs& dst, const BuildInputs& src) {
322311
src.stdModuleFlags.end());
323312
}
324313

314+
// Does this carry anything? The sibling of `append` above: that function is the
315+
// one place a contribution is combined, and this is the one place it is weighed.
316+
// Both enumerate every field, and they sit together so that a field added to
317+
// BuildInputs is a field the reader meets twice.
318+
//
319+
// A FREE FUNCTION, NOT A MEMBER, AND THAT IS NOT STYLE. Adding an inline member
320+
// to a struct exported from this module changes what importers materialise from
321+
// its BMI, and `Profile`'s `std::optional<std::string>` member is already known
322+
// to break under clang with the MSVC standard library — see the note on that
323+
// member for the measurement. The first version of this was a member, and it
324+
// failed exactly there: `test_modgraph.cpp` stopped compiling with
325+
// `no matching constructor for _SMF_control<_Optional_construct_base<...>>`,
326+
// reported against a struct the change never touched. `append` has been a free
327+
// function since it was written; this one matches it.
328+
inline bool is_empty(const BuildInputs& b) {
329+
return b.sources.empty() && b.cflags.empty() && b.cxxflags.empty()
330+
&& b.ldflags.empty() && b.defines.empty() && b.globFlags.empty()
331+
&& b.includeDirs.empty() && b.includeDirsAfter.empty()
332+
&& b.privateIncludeDirs.empty() && b.stdModuleFlags.empty();
333+
}
334+
325335
// A build-graph node declared by a build program (`mcpp:action=`).
326336
//
327337
// The architectural point (see
@@ -1156,14 +1166,17 @@ struct ConditionalConfig {
11561166
// This is #258's medicine: a field added below and forgotten here is a
11571167
// question asked at the point where the field is written, instead of in two
11581168
// files that do not mention each other.
1159-
bool empty() const {
1160-
return inputs.empty() && linkLibraryDirs.empty() && libraries.empty()
1161-
&& dependencies.empty() && devDependencies.empty()
1162-
&& buildDependencies.empty() && featureDeps.empty()
1163-
&& xlings.empty();
1164-
}
11651169
};
11661170

1171+
// The same question for a whole conditional block, and a free function for the
1172+
// same reason `is_empty(BuildInputs)` is one.
1173+
inline bool is_empty(const ConditionalConfig& c) {
1174+
return is_empty(c.inputs) && c.linkLibraryDirs.empty() && c.libraries.empty()
1175+
&& c.dependencies.empty() && c.devDependencies.empty()
1176+
&& c.buildDependencies.empty() && c.featureDeps.empty()
1177+
&& c.xlings.empty();
1178+
}
1179+
11671180
// `[lib]` — library "root" interface convention.
11681181
//
11691182
// Convention-over-configuration: a library package's primary module

modules/manifest/src/xpkg.cppm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1530,7 +1530,7 @@ synthesize_from_xpkg_lua(std::string_view luaContent,
15301530
// The same question the mcpp.toml reader asks, asked the same
15311531
// way. This list omitted `privateIncludeDirs`, which this loop
15321532
// fills.
1533-
if (!cc.empty())
1533+
if (!mcpp::manifest::is_empty(cc))
15341534
m.conditionalConfigs.push_back(std::move(cc));
15351535
cur.skip_ws_and_comments();
15361536
}

0 commit comments

Comments
 (0)