Skip to content

Commit a16a001

Browse files
committed
fix(build.mcpp): reference the std BMI through the token helper too
cl answered "C2230: could not find module 'std'" because the std reference was still built by string concatenation, producing one argv element with a space inside it. bmi_reference_tokens existed for exactly this and was only applied to the bundled mcpp module. Same shape for the third time — a table entry written for the ninja STRING channel concatenated into an argv element — so it now has a test that covers every family and both spellings, plus one for the language-force tokens. The compiler's own diagnostics name neither the flag nor the reason, which is why this kept costing a CI round each time.
1 parent bcde510 commit a16a001

2 files changed

Lines changed: 46 additions & 8 deletions

File tree

src/build/build_program.cppm

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -628,15 +628,19 @@ std::expected<void, std::string> run_build_program(
628628
if (!usesModule) stdFlags.push_back("-fmodules");
629629
stdStagedInBdir = true;
630630
} else {
631-
stdFlags.push_back(std::string(traits.stdBmiUsePrefix)
632-
+ sm->bmiPath.string());
631+
// Through bmi_reference_tokens, not string concatenation: the
632+
// traits spell these for the ninja STRING channel, where
633+
// `-fmodule-file=std=<p>` (one word) and `/reference std=<p>`
634+
// (two) are indistinguishable. Concatenating produced a single
635+
// argv element with a space inside it, and cl answered
636+
// "C2230: could not find module 'std'".
637+
for (auto& t : mcpp::toolchain::bmi_reference_tokens(
638+
traits.stdBmiUsePrefix, sm->bmiPath))
639+
stdFlags.push_back(t);
633640
if (usesStdCompat && !sm->compatBmiPath.empty())
634-
stdFlags.push_back(std::string(traits.stdCompatBmiUsePrefix)
635-
+ sm->compatBmiPath.string());
636-
// The prefixes carry a leading space for the ninja string channel;
637-
// an argv element must not.
638-
for (auto& f : stdFlags)
639-
if (!f.empty() && f.front() == ' ') f.erase(0, 1);
641+
for (auto& t : mcpp::toolchain::bmi_reference_tokens(
642+
traits.stdCompatBmiUsePrefix, sm->compatBmiPath))
643+
stdFlags.push_back(t);
640644
}
641645
if (!sm->objectPath.empty() && fs::exists(sm->objectPath))
642646
stdObjects.push_back(sm->objectPath.string());

tests/unit/test_hostflags.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,40 @@ TEST(HostFlags, BmiReferenceSplitsOnlyWhenTheSpellingHasASpace) {
146146
EXPECT_EQ(msvc[1], "std=/tmp/std.ifc");
147147
}
148148

149+
// The general invariant, checked for every family: an argv element must never
150+
// contain a space. This bug has now appeared three times in the same shape —
151+
// `-x c++`, the mcpp module reference, the std reference — each time because
152+
// a table entry written for the ninja STRING channel was concatenated into an
153+
// argv element. cl.exe answers with "could not find module 'std'", which
154+
// names neither the flag nor the reason.
155+
TEST(HostFlags, BmiReferencesNeverProduceATokenWithASpace) {
156+
for (auto id : kFamilies) {
157+
auto tc = tc_for(id);
158+
auto t = mcpp::toolchain::bmi_traits(tc);
159+
for (auto prefix : { t.stdBmiUsePrefix, t.stdCompatBmiUsePrefix }) {
160+
for (auto const& tok : mcpp::toolchain::bmi_reference_tokens(
161+
prefix, std::filesystem::path("/tmp/x.bmi"))) {
162+
EXPECT_EQ(tok.find(' '), std::string::npos)
163+
<< "family " << mcpp::toolchain::dialect_for(tc).id
164+
<< " token: " << tok;
165+
}
166+
}
167+
}
168+
}
169+
170+
// Same invariant for the language-force spelling, which has both a positional
171+
// and a per-file form.
172+
TEST(HostFlags, LanguageForceTokensNeverContainASpace) {
173+
for (auto const* d : { &mcpp::toolchain::gnu_dialect(),
174+
&mcpp::toolchain::msvc_dialect() }) {
175+
for (auto f : d->forceCxxLangArgv)
176+
EXPECT_EQ(f.find(' '), std::string_view::npos) << d->id << ": " << f;
177+
for (auto f : d->alwaysFlagsArgv)
178+
EXPECT_EQ(f.find(' '), std::string_view::npos) << d->id << ": " << f;
179+
EXPECT_EQ(d->perFileCxxPrefix.find(' '), std::string_view::npos) << d->id;
180+
}
181+
}
182+
149183
TEST(HostFlags, BmiReferenceIsEmptyForAToolchainThatNamesNothing) {
150184
// GCC finds BMIs implicitly under <cwd>/gcm.cache — its prefix is empty
151185
// and must not produce a stray token.

0 commit comments

Comments
 (0)