Skip to content

Commit d14e2ed

Browse files
committed
test(targets): the two pure producers the Android path changed now have criteria
THE PRE-MERGE REVIEW'S OWN FINDING. Mapping every behaviour change in this PR to a criterion left seven without one, all of them "the Android build works" -- testable only with a 704 MB payload CI does not have. Two of the seven are pure functions of a `Toolchain` and needed no payload at all, which makes them the two worth closing. `std_module_build_commands`: the PRECOMPILE carries the machine flags when only `stdModuleTargetFlags` has them, and states the target exactly ONCE when `stdModuleFlags` also does -- the superset case, where concatenating both would put `--target=` on the command line twice. `host_compile_tokens`: an own-sysroot target receives EXACTLY the target flag and nothing else. Asserted as the whole vector rather than as "contains", because the property is that nothing else is emitted: this host's glibc headers reaching a wasm compile is the measured failure the gate exists for. THE CONTROL TOOK TWO ATTEMPTS AND BOTH FAILURES WERE THE CONTROL'S. Asserting that a hosted target receives more than one token failed, because a bare `Toolchain` carrying no payload has nothing to reconstruct either -- both sides produced exactly the triple. Reaching for `--no-default-config` without a payload that HAS a cfg was the same mistake once removed. `FakeClangPayload` writes a `clang++.cfg`, which is what makes the discriminator real, and the bypass is the right one to use because withholding it from an SDK is a property the gate's own comment states. Both verified in both directions: reverting each fix turns its test red.
1 parent a1248b0 commit d14e2ed

2 files changed

Lines changed: 179 additions & 0 deletions

File tree

tests/unit/test_hostflags.cpp

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,3 +507,113 @@ TEST(HostFlags, TheCfgBypassSurvivesAGraphSuppliedTargetSide) {
507507
EXPECT_TRUE(has(a, "-nostdinc++"));
508508
EXPECT_FALSE(has(b, "-nostdinc++"));
509509
}
510+
511+
// "A TOOLCHAIN THAT SHIPS ITS OWN SYSROOT IS TOLD NOTHING" WAS ONE TOKEN TOO
512+
// STRONG, AND THIS FUNCTION ALREADY SAID SO FURTHER DOWN.
513+
//
514+
// The early return for `has_own_sysroot()` withholds the target's system
515+
// reconstructed onto the command line -- libc++'s headers, glibc's, the Linux
516+
// UAPI headers, the cfg bypass, the C-runtime prefix -- because an Emscripten
517+
// or Android SDK already has all of it. That is right. It stood in FRONT of
518+
// the paragraph beginning "THE TRIPLE, SAID OUT LOUD", which states the
519+
// opposite rule for the same underlying reason: an ordinary clang emits for the
520+
// machine it is running on unless told otherwise. The stronger claim won by
521+
// position.
522+
//
523+
// Both are right about their own object. The SYSTEM is the payload's; WHICH
524+
// TARGET is still mcpp's to say, because one NDK serves both Android ABIs and
525+
// nothing else on the command line distinguishes them. The defect was reported
526+
// by neither compile but by the module loader:
527+
//
528+
// error: AST file 'std.pcm' was compiled for the target
529+
// 'aarch64-unknown-linux-android21' but the current translation unit is
530+
// being compiled for target 'x86_64-unknown-linux-gnu'
531+
//
532+
// followed by eight cascading "use of undeclared identifier 'std'" lines,
533+
// which is what a reader sees first.
534+
TEST(HostFlags, AnOwnSysrootTargetIsToldWhichTargetAndNothingElse) {
535+
HostFlagOptions opt;
536+
537+
for (auto name : {"aarch64-linux-android", "x86_64-linux-android",
538+
"wasm32-emscripten"}) {
539+
auto tc = tc_for(CompilerId::Clang);
540+
tc.targetTriple = name;
541+
tc.crossTargetFlag = "--target=SENTINEL-TRIPLE";
542+
543+
auto tokens = mcpp::toolchain::host_compile_tokens(
544+
tc, opt, mcpp::toolchain::no_escape);
545+
546+
// EXACTLY the target flag. Asserted as the whole vector rather than as
547+
// "contains", because the property is that nothing ELSE is emitted:
548+
// this host's glibc headers reaching a wasm compile is the measured
549+
// failure this gate exists for.
550+
ASSERT_EQ(tokens.size(), 1u) << name << ": " << [&] {
551+
std::string all;
552+
for (auto const& t : tokens) { all += t; all += ' '; }
553+
return all;
554+
}();
555+
EXPECT_EQ(tokens[0], "--target=SENTINEL-TRIPLE") << name;
556+
}
557+
558+
// AND THE GATE IS STILL A GATE, DISCRIMINATED BY THE cfg BYPASS.
559+
//
560+
// A first version of this control asserted that a HOSTED target receives
561+
// more than one token, and it failed -- with a bare `Toolchain` carrying no
562+
// payload paths, the hosted path has nothing to reconstruct either, so both
563+
// sides produced exactly the triple and the control could not tell them
564+
// apart. The control was wrong, not the code.
565+
//
566+
// `--no-default-config` is the discriminator, and it is a property the
567+
// gate's own comment states: the bypass exists to stop clang reading a
568+
// per-install `clang++.cfg`, while `em++` is a wrapper whose entire job is
569+
// to supply configuration, so suppressing it would be suppressing the
570+
// toolchain. It is therefore emitted past the gate and never before it,
571+
// which is exactly what a control needs.
572+
// A first version of this control asserted only that a HOSTED target
573+
// receives more than one token, and it failed -- with a bare `Toolchain`
574+
// carrying no payload the hosted path has nothing to reconstruct either,
575+
// so both sides produced exactly the triple and the control could not tell
576+
// them apart. A second version reached for `--no-default-config` without a
577+
// payload that HAS a cfg, which is the same mistake once removed. The
578+
// fixture is what makes the discriminator real.
579+
//
580+
// `--no-default-config` is the right discriminator because it is a
581+
// property the gate's own comment states: the bypass exists to stop clang
582+
// reading a per-install `clang++.cfg`, while `em++` is a wrapper whose
583+
// entire job is to supply configuration, so suppressing it would be
584+
// suppressing the toolchain. Emitted past the gate, never before it.
585+
FakeClangPayload payload{"own-sysroot-gate"};
586+
HostFlagOptions bypass;
587+
bypass.cfgBypass = HostFlagOptions::CfgBypass::Always;
588+
589+
auto host = tc_for(CompilerId::Clang);
590+
host.binaryPath = payload.root / "bin" / "clang++";
591+
host.crossTargetFlag = "--target=x86_64-unknown-linux-gnu";
592+
auto hostTokens = mcpp::toolchain::host_compile_tokens(
593+
host, bypass, mcpp::toolchain::no_escape);
594+
EXPECT_NE(std::ranges::find(hostTokens, "--no-default-config"),
595+
hostTokens.end())
596+
<< "a hosted clang with a cfg beside it must reach the bypass";
597+
598+
for (auto name : {"aarch64-linux-android", "wasm32-emscripten"}) {
599+
auto sdk = tc_for(CompilerId::Clang);
600+
sdk.binaryPath = payload.root / "bin" / "clang++"; // same payload
601+
sdk.targetTriple = name;
602+
sdk.crossTargetFlag = "--target=SENTINEL-TRIPLE";
603+
auto sdkTokens = mcpp::toolchain::host_compile_tokens(
604+
sdk, bypass, mcpp::toolchain::no_escape);
605+
EXPECT_EQ(std::ranges::find(sdkTokens, "--no-default-config"),
606+
sdkTokens.end())
607+
<< name << ": the cfg bypass must be withheld from an SDK whose "
608+
"driver's job is to supply configuration";
609+
EXPECT_EQ(sdkTokens.size(), 1u) << name;
610+
}
611+
612+
// A row with no cross flag emits nothing at all rather than an empty
613+
// token: an empty argv element is an argument the driver must interpret.
614+
auto bare = tc_for(CompilerId::Clang);
615+
bare.targetTriple = "wasm32-emscripten";
616+
ASSERT_TRUE(bare.crossTargetFlag.empty());
617+
EXPECT_TRUE(mcpp::toolchain::host_compile_tokens(
618+
bare, opt, mcpp::toolchain::no_escape).empty());
619+
}

tests/unit/test_toolchain_stdmod.cpp

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,72 @@ TEST(ToolchainStdmod, ClangStdCompatCommandsUseRequestedStandard) {
7070
EXPECT_EQ(cmd.find("-std=c++23"), std::string::npos) << cmd;
7171
}
7272
}
73+
74+
// THE PRECOMPILE HAS TO KNOW WHICH MACHINE, AND ONLY ONE OF TWO SOURCES EVER
75+
// CARRIES IT.
76+
//
77+
// `stdModuleTargetFlags` reached only the CODEGEN command, on the reading that
78+
// the first step needs headers and the second needs the machine. The first step
79+
// needs both: a `--precompile` that does not say which target resolves the
80+
// standard library's own `#include <__config>` against the BUILDING machine,
81+
// and the error names a header rather than the missing flag.
82+
//
83+
// It was invisible while exactly two kinds of toolchain existed. A payload
84+
// whose compiler IS its target needs no flag, and a PACKAGE-provided module
85+
// carries the target inside `stdModuleFlags`. A payload whose compiler serves
86+
// SEVERAL targets is a third kind and has neither -- one NDK clang++ compiles
87+
// for both Android ABIs and is told which by `--target` alone.
88+
TEST(ToolchainStdmod, ThePrecompileCarriesTheMachineWhenOnlyTargetFlagsHaveIt) {
89+
auto tc = clang_toolchain();
90+
tc.targetTriple = "aarch64-linux-android";
91+
// What prepare_build sets for such a row: the machine, and nothing about
92+
// include paths, because the SDK's own driver finds those.
93+
tc.stdModuleTargetFlags =
94+
" --target=aarch64-unknown-linux-android21 -D__BIONIC_CTYPE_INLINE=";
95+
ASSERT_TRUE(tc.stdModuleFlags.empty());
96+
97+
auto cmds = clang::std_module_build_commands(
98+
tc, "cache", "cache/pcm.cache/std.pcm", "", "-std=c++23");
99+
ASSERT_EQ(cmds.size(), 2u);
100+
101+
// BOTH commands, not just the second. The precompile is the one that was
102+
// missing it and the one whose failure names a header.
103+
for (auto const& cmd : cmds) {
104+
EXPECT_NE(cmd.find("--target=aarch64-unknown-linux-android21"),
105+
std::string::npos) << cmd;
106+
}
107+
// And the bionic workaround reaches the step that parses the headers.
108+
EXPECT_NE(cmds[0].find("-D__BIONIC_CTYPE_INLINE="), std::string::npos)
109+
<< cmds[0];
110+
}
111+
112+
// AND IT IS NOT ADDED TWICE. `stdModuleFlags` is a SUPERSET of
113+
// `stdModuleTargetFlags` when it is set at all -- its producer builds the
114+
// machine part first and appends the include part -- so a precompile that
115+
// concatenated both would put `--target=` on the command line twice. Taking
116+
// the superset in preference is what keeps that from happening.
117+
TEST(ToolchainStdmod, APackageProvidedModuleStillStatesTheMachineExactlyOnce) {
118+
auto tc = clang_toolchain();
119+
tc.targetTriple = "aarch64-macos";
120+
tc.stdModuleTargetFlags = " --target=arm64-apple-macos14.0";
121+
tc.stdModuleFlags =
122+
" --target=arm64-apple-macos14.0 -nostdinc++ -isystem /pkg/include";
123+
124+
auto cmds = clang::std_module_build_commands(
125+
tc, "cache", "cache/pcm.cache/std.pcm", "", "-std=c++23");
126+
ASSERT_EQ(cmds.size(), 2u);
127+
128+
const auto count = [](std::string_view hay, std::string_view needle) {
129+
std::size_t n = 0, at = 0;
130+
while ((at = hay.find(needle, at)) != std::string_view::npos) {
131+
++n; at += needle.size();
132+
}
133+
return n;
134+
};
135+
EXPECT_EQ(count(cmds[0], "--target="), 1u) << cmds[0];
136+
// The package's include path reaches the step that needs it, and only it.
137+
EXPECT_NE(cmds[0].find("-isystem /pkg/include"), std::string::npos)
138+
<< cmds[0];
139+
EXPECT_EQ(cmds[1].find("-isystem /pkg/include"), std::string::npos)
140+
<< cmds[1];
141+
}

0 commit comments

Comments
 (0)