Skip to content

Commit 14b28ed

Browse files
speak-agentclaude
andcommitted
two defects found reviewing this branch: the wrong provider, and a name that is a prefix of another
The interface list was read from the first package in the graph that stated one. A graph may carry more than one candidate for a layer --- a workspace member beside a dependency, a second implementation behind a feature that did not activate --- and only one of them is the provider this build resolved. Comparing a consumer's requirements against an implementation the build is not using is a wrong answer rather than a missing one. It now reads the resolved layer. `c_abi_absent_facility_advice` matched the symbol as a substring, and `undefined symbol: open` is a prefix of `undefined symbol: opendir`. A link failure would have been explained by a row with nothing to do with it, and an explanation that is confidently wrong is worse than the linker's own message. The name must now end where the diagnostic's name ends. Co-authored-by: Claude Code <noreply@anthropic.com>
1 parent a4993b0 commit 14b28ed

3 files changed

Lines changed: 47 additions & 3 deletions

File tree

src/build/ninja_backend.cppm

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -697,10 +697,27 @@ std::string c_abi_absent_facility_advice(
697697
// program at run time by construction, so matching them against a
698698
// link diagnostic would report a coincidence of spelling.
699699
if (e.form != mcpp::targetside::CAbiAbsentForm::Link) continue;
700-
const std::string lld = std::format("undefined symbol: {}", e.name);
700+
// THE NAME MUST END WHERE THE DIAGNOSTIC'S NAME ENDS. `undefined
701+
// symbol: open` is a prefix of `undefined symbol: opendir`, so a
702+
// plain substring search explains a link failure with a row that has
703+
// nothing to do with it — and an explanation that is confidently
704+
// wrong is worse than the linker's own message. lld ends the name at
705+
// the line; GNU ld closes it with a quote.
701706
const std::string gnu = std::format("undefined reference to `{}'", e.name);
702-
if (output.find(lld) == std::string_view::npos
703-
&& output.find(gnu) == std::string_view::npos) continue;
707+
bool matched = output.find(gnu) != std::string_view::npos;
708+
if (!matched) {
709+
const std::string lld = std::format("undefined symbol: {}", e.name);
710+
for (std::size_t at = output.find(lld); at != std::string_view::npos;
711+
at = output.find(lld, at + 1)) {
712+
const auto after = at + lld.size();
713+
if (after >= output.size() || output[after] == '\n'
714+
|| output[after] == '\r' || output[after] == ' ') {
715+
matched = true;
716+
break;
717+
}
718+
}
719+
}
720+
if (!matched) continue;
704721
named += std::format("\n {:<20} {}", e.name,
705722
e.note.empty() ? "declared absent" : e.note);
706723
}

src/build/prepare.cppm

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11181,10 +11181,23 @@ prepare_build(bool print_fingerprint,
1118111181
// changes no command line and no diagnostic for every project built
1118211182
// before it.
1118311183
{
11184+
// THE LIST COMES FROM THE PACKAGE THAT RESOLVED AS THE LAYER, NOT
11185+
// FROM THE FIRST ONE IN THE GRAPH THAT STATED ONE. A graph may
11186+
// carry more than one candidate for a layer — a workspace member
11187+
// beside a dependency, a second implementation reached through a
11188+
// feature that did not activate — and only one of them is the
11189+
// provider this build resolved. Reading whichever came first in
11190+
// `packages` would compare a consumer's requirements against an
11191+
// implementation the build is not using, which is a wrong answer
11192+
// rather than a missing one.
1118411193
std::vector<std::string> providedInterfaces;
1118511194
std::string providerId;
1118611195
for (auto& pkg : packages) {
1118711196
if (pkg.manifest.kernelAbiProvidesInterfaces.empty()) continue;
11197+
if (!resolvedTargetSide.kernelAbi.impl.empty()
11198+
&& resolvedTargetSide.kernelAbi.impl.find(
11199+
pkg.manifest.package.name) == std::string::npos)
11200+
continue;
1118811201
providedInterfaces = pkg.manifest.kernelAbiProvidesInterfaces;
1118911202
providerId = pkg.manifest.package.name;
1119011203
break;

tests/unit/test_build_flags.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,3 +267,17 @@ TEST(CAbiAbsentAdvice, ALibraryThatEnumeratedNothingProducesNoNote) {
267267
EXPECT_TRUE(mcpp::build::c_abi_absent_facility_advice(
268268
"ld.lld: error: undefined symbol: fork\n", "musl", {}).empty());
269269
}
270+
271+
TEST(CAbiAbsentAdvice, ALongerSymbolWithTheSamePrefixIsNotExplained) {
272+
// `undefined symbol: open` is a prefix of `undefined symbol: opendir`.
273+
// A substring search would answer a link failure with a row that has
274+
// nothing to do with it, and an explanation that is confidently wrong is
275+
// worse than the linker's own message.
276+
std::vector<mcpp::targetside::CAbiAbsentEntry> absent{
277+
{"open", mcpp::targetside::CAbiAbsentForm::Link, "not supplied"},
278+
};
279+
EXPECT_TRUE(mcpp::build::c_abi_absent_facility_advice(
280+
"ld.lld: error: undefined symbol: opendir\n", "musl", absent).empty());
281+
EXPECT_FALSE(mcpp::build::c_abi_absent_facility_advice(
282+
"ld.lld: error: undefined symbol: open\n", "musl", absent).empty());
283+
}

0 commit comments

Comments
 (0)