Skip to content

Commit c7c17e9

Browse files
committed
fix(build): a platform limit nobody asked about is not a diagnostic
The MSVC runtime has no self-contained mechanism at all (no /MT emission), so the default contract degraded on EVERY Windows build and printed a warning nobody could act on. A diagnostic is for a broken promise — mcpp said the artifact would be self-contained and it is not. Where mcpp never made the promise, the cell now stays quiet unless the contract was written down explicitly. Cells that DO promise something (a missing libc++.a under the default) still report regardless.
1 parent c92b4cb commit c7c17e9

3 files changed

Lines changed: 52 additions & 20 deletions

File tree

src/build/distribution.cppm

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,17 @@ Contract default_contract(Role r) {
134134
struct MechanismInput {
135135
Contract requested = Contract::SelfContained;
136136
Role role = Role::Distributable;
137+
// Did a human write this contract down, or is it just our default?
138+
//
139+
// The distinction decides whether a cell with no mechanism SPEAKS. A
140+
// diagnostic is for a BROKEN PROMISE: mcpp said the artifact would be
141+
// self-contained and it is not. Under the MSVC runtime mcpp never made
142+
// that promise — there is no /MT emission at all — so warning on every
143+
// Windows build would be noise nobody can act on. Write
144+
// `cxx_runtime = "self-contained"` there and you get told, once, that it
145+
// is not implemented. Cells where mcpp DOES promise something (a missing
146+
// libc++.a under the default, say) report regardless.
147+
bool explicitRequest = false;
137148
// Toolchain capability id: "libstdc++", "libc++", or an MSVC STL spelling.
138149
std::string_view stdlibId;
139150
Format format = Format::Elf;
@@ -205,7 +216,7 @@ Mechanism resolve(const MechanismInput& in) {
205216
// Mach-O without libc++ is not a configuration mcpp produces.
206217
m.effective = Contract::HostCoupled;
207218
m.unitFlags = " -lc++";
208-
if (in.requested != Contract::HostCoupled) {
219+
if (in.requested != Contract::HostCoupled && in.explicitRequest) {
209220
m.degraded = true;
210221
m.diagnostic = std::format(
211222
"cxx_runtime = \"{}\" is not available for stdlib '{}' on "
@@ -266,12 +277,14 @@ Mechanism resolve(const MechanismInput& in) {
266277
// producing the same bytes and reporting success.
267278
m.effective = Contract::HostCoupled;
268279
if (in.requested == Contract::SelfContained) {
269-
m.degraded = true;
270-
m.diagnostic =
271-
"cxx_runtime = \"self-contained\" is not implemented for the "
272-
"MSVC runtime yet (it would need the /MT runtime); using "
273-
"host-coupled — the artifact needs the VC++ redistributable";
280+
m.degraded = in.explicitRequest;
281+
m.diagnostic = in.explicitRequest
282+
? "cxx_runtime = \"self-contained\" is not implemented for the "
283+
"MSVC runtime yet (it would need the /MT runtime); using "
284+
"host-coupled — the artifact needs the VC++ redistributable"
285+
: "";
274286
} else if (in.requested == Contract::ToolchainCoupled) {
287+
// Only reachable from an explicit request: it is never a default.
275288
m.degraded = true;
276289
m.diagnostic =
277290
"cxx_runtime = \"toolchain-coupled\" has no meaning for the "
@@ -338,7 +351,7 @@ Mechanism resolve(const MechanismInput& in) {
338351
// Unknown stdlib on ELF: emit nothing rather than guess, but say so
339352
// when something was actually asked for.
340353
m.effective = Contract::HostCoupled;
341-
if (in.requested != Contract::HostCoupled) {
354+
if (in.requested != Contract::HostCoupled && in.explicitRequest) {
342355
m.degraded = true;
343356
m.diagnostic = std::format(
344357
"cxx_runtime = \"{}\" has no mechanism for stdlib '{}'; "

src/build/flags.cppm

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -609,12 +609,18 @@ CompileFlags compute_flags(const BuildPlan& plan) {
609609
mi.libunwindArchive = find_archive("libunwind.a");
610610
}
611611

612-
for (auto [role, requested] : {
613-
std::pair{dist::Role::Distributable, base},
614-
std::pair{dist::Role::Test, testsContract},
615-
std::pair{dist::Role::Intermediate, base}}) {
616-
mi.role = role;
617-
mi.requested = requested;
612+
// "Explicit" = a human wrote it down. `static_stdlib = false` counts:
613+
// nobody sets a flag to its default to get non-default behavior.
614+
const bool explicitBase = !bc.cxxRuntime.empty() || !bc.staticStdlib;
615+
const bool explicitTests = explicitBase || !bc.cxxRuntimeTests.empty();
616+
617+
for (auto [role, requested, wasAsked] : {
618+
std::tuple{dist::Role::Distributable, base, explicitBase},
619+
std::tuple{dist::Role::Test, testsContract, explicitTests},
620+
std::tuple{dist::Role::Intermediate, base, explicitBase}}) {
621+
mi.role = role;
622+
mi.requested = requested;
623+
mi.explicitRequest = wasAsked;
618624
auto r = dist::resolve(mi);
619625
auto i = static_cast<std::size_t>(role);
620626
f.ldStdlibByRole[i] = r.unitFlags;

tests/unit/test_distribution.cpp

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,9 @@ TEST(Distribution, MingwParity) {
217217
// success; now it names the gap.
218218
TEST(Distribution, MsvcSelfContainedIsAnHonestGap) {
219219
dist::MechanismInput in;
220-
in.format = dist::Format::Pe;
221-
in.stdlibId = "msvc";
220+
in.format = dist::Format::Pe;
221+
in.stdlibId = "msvc";
222+
in.explicitRequest = true;
222223
auto m = dist::resolve(in);
223224
EXPECT_EQ(m.effective, dist::Contract::HostCoupled);
224225
EXPECT_TRUE(m.degraded);
@@ -227,6 +228,16 @@ TEST(Distribution, MsvcSelfContainedIsAnHonestGap) {
227228

228229
in.requested = dist::Contract::HostCoupled;
229230
EXPECT_FALSE(dist::resolve(in).degraded);
231+
232+
// ...but the DEFAULT must be quiet. mcpp never promised a self-contained
233+
// MSVC artifact — it emits no /MT at all — so warning on every Windows
234+
// build would be unactionable noise. A diagnostic is for a broken
235+
// promise, not for a platform limit nobody asked about.
236+
in.requested = dist::Contract::SelfContained;
237+
in.explicitRequest = false;
238+
auto quiet = dist::resolve(in);
239+
EXPECT_FALSE(quiet.degraded);
240+
EXPECT_TRUE(quiet.diagnostic.empty());
230241
}
231242

232243
// ---------------------------------------------------------------------------
@@ -249,11 +260,13 @@ TEST(Distribution, TableIsTotalAndEveryDowngradeExplainsItself) {
249260
for (auto role : roles)
250261
for (bool archives : {false, true}) {
251262
dist::MechanismInput in;
252-
in.requested = c;
253-
in.format = fmt;
254-
in.stdlibId = sl;
255-
in.role = role;
256-
in.macosFloor = true;
263+
in.requested = c;
264+
in.format = fmt;
265+
in.stdlibId = sl;
266+
in.role = role;
267+
in.macosFloor = true;
268+
in.explicitRequest = true; // the question is "what if you ASK for it"
269+
257270
if (archives) {
258271
in.libcxxArchive = "/a.a";
259272
in.libcxxAbiArchive = "/b.a";

0 commit comments

Comments
 (0)