From 4ff2d322f6bf8d784ce35462625a036a01745995 Mon Sep 17 00:00:00 2001 From: Mura Li <2606021+typeless@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:05:29 +0800 Subject: [PATCH] Suppress the unscanned-compile report only for a compile that scans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suppression asked whether a depfile flag appeared anywhere in the rule's command text. It was consulted per rule, before the scans were built, so any word spelling a depfile flag silenced the report for every object the rule declared — including a word in an invocation that no scan would ever have been built from. That made the diagnostic defeatable by placement rather than by meaning, and the failure was double-silent: no scan, no depfile, and no report that either was missing. The tool whose one job is to say when headers go unrecorded had a blind spot a stray word could open. Both scanners now ask the question over the invocations build_dep_scans draws from, gated the way build_dep_scans gates them: the invocation must be the compile itself. Reading only the scannable prefix is not sufficient on its own — the prefix deliberately continues past scan-transparent invocations so the walk can reach a later compile, and build_dep_scans then skips exactly those for want of a compiler index. Without the compiler gate, echo -MD && gcc -c a.c -o a.o && cp a.o b.o still answered true: a program that cannot write a depfile suppressing a compile that carries no flag, leaving b.o uncovered and unreported. The two predicates now read the identical set of invocations, so they cannot disagree. The word test moves with it. Matching -M inside the command text needed a manual check that the preceding character was a space; over tokenized words that check is the tokenizer's job and is gone, which is what retires the string_utils include here. The requirement said "carries no depfile flag anywhere in its text" and now says "no invocation a scan would have been built from carries a depfile flag", and the two classes that escaped the report are down to one. Verified: RED quoted for every leg, including the three second-round tests that failed against the first fix. The E2E leg authored after the fix was proved discriminating by restoring the pre-tightening scanner, observing the failure, and putting it back, verified by sha256. build, format, tidy, iwyu, spec-check all exit 0; make test is green when carried onto the #397 base and fails on a main base only at the pre-existing read-only-/tmp failure in test_builder.cpp, which this change does not touch. Fixes #357 --- spec/requirements/dep-scan.ears.md | 18 +++++--- src/graph/scanners/clang_cl.cpp | 6 ++- src/graph/scanners/gcc.cpp | 37 ++++++++-------- test/unit/test_dep_scanner.cpp | 70 ++++++++++++++++++++++++++++++ test/unit/test_e2e.cpp | 47 ++++++++++++++++++++ 5 files changed, 149 insertions(+), 29 deletions(-) diff --git a/spec/requirements/dep-scan.ears.md b/spec/requirements/dep-scan.ears.md index 8cc2b6ec..1a095316 100644 --- a/spec/requirements/dep-scan.ears.md +++ b/spec/requirements/dep-scan.ears.md @@ -28,11 +28,12 @@ into invocations, each gets its own scan, and a scan may draw a word only from t reproduces. A redirection is not such a divider — it hands its target to the same program — though the scan still carries no flag from beyond it. -Two classes escape the report. A compile-and-link command with no `-c` produces an executable +One class escapes the report. A compile-and-link command with no `-c` produces an executable rather than an object file, so the output-shaped trigger cannot see it (the `HOSTCC` generator -rules in `examples/bsp/gcc/gmp/Tupfile` are the in-tree instance). And the suppression for a -command that writes its own depfile tests the whole command text for a depfile flag, so an -unrelated word spelling one silences the report for that rule. +rules in `examples/bsp/gcc/gmp/Tupfile` are the in-tree instance). The suppression for a command +that writes its own depfile reads only the invocations a scan would have been built from, so a +depfile flag elsewhere — in a later invocation, or behind a prefix that makes the compile +unreproducible — silences nothing. --- @@ -107,8 +108,11 @@ speaks about the object it names, not about the command that declares it. - conformance: putup-only - discharge: test "Scenario: A compile-shaped rule with no dependency scan is reported" - discharge: test "Scenario: An object no scanned invocation writes is reported beside its scanned sibling" +- discharge: test "Scenario: A depfile flag the compile never carried hides no unscanned object" +- discharge: test "A depfile flag outside the scannable prefix suppresses nothing" +- discharge: test "A depfile flag inside the scannable prefix still suppresses" When a rule declares an object file that no generated scan covers — every object it declares, -where no scan at all is generated — and the rule's command carries no depfile flag anywhere in its -text, putup shall name that object and the rule's Tupfile under `parse`, and report how many such -objects exist under a build. +where no scan at all is generated — and no invocation a scan would have been built from carries a +depfile flag, putup shall name that object and the rule's Tupfile under `parse`, and report how +many such objects exist under a build. diff --git a/src/graph/scanners/clang_cl.cpp b/src/graph/scanners/clang_cl.cpp index b1b3f188..de962fef 100644 --- a/src/graph/scanners/clang_cl.cpp +++ b/src/graph/scanners/clang_cl.cpp @@ -182,8 +182,10 @@ auto ClangClScanner::has_dep_flags(CommandTokens const& tokens) const -> bool { // Only a pinned -MF counts: /MD and /MT select the CRT here, and a bare // -MD writes the depfile to the cwd instead of beside the object. - return std::ranges::any_of(tokens.words(), [](auto word) { - return word.starts_with("/clang:-MF") || word.starts_with("-clang:-MF"); + return std::ranges::any_of(scannable_prefix(tokens.invocations()), [](auto invocation) { + return driver_index(invocation).has_value() && std::ranges::any_of(invocation, [](auto word) { + return word.starts_with("/clang:-MF") || word.starts_with("-clang:-MF"); + }); }); } diff --git a/src/graph/scanners/gcc.cpp b/src/graph/scanners/gcc.cpp index d2f356fd..69d37b53 100644 --- a/src/graph/scanners/gcc.cpp +++ b/src/graph/scanners/gcc.cpp @@ -7,7 +7,6 @@ #include "pup/core/global_pool.hpp" #include "pup/core/path.hpp" #include "pup/core/string_pool.hpp" -#include "pup/core/string_utils.hpp" #include "pup/graph/scanners/dep_words.hpp" #include @@ -154,6 +153,18 @@ auto scannable_prefix(std::span const> invocat return invocations.first(length); } +auto is_dep_flag_word(std::string_view word) -> bool +{ + if (!word.starts_with("-M")) { + return false; + } + if (word.size() == 2) { + return true; + } + auto const c = word[2]; + return c == 'D' || c == 'M' || c == 'F' || c == 'G' || c == 'P' || c == 'T' || c == 'Q' || c == 'V'; +} + } // namespace auto matches_gcc_compile(std::string_view command) -> bool @@ -169,25 +180,11 @@ auto GccScanner::matches(CommandInfo const& /*cmd*/, CommandTokens const& tokens auto GccScanner::has_dep_flags(CommandTokens const& tokens) const -> bool { - auto cmd = global_pool().get(tokens.text()); - auto pos = std::string_view::size_type { 0 }; - while ((pos = cmd.find("-M", pos)) != std::string_view::npos) { - if (pos > 0 && !pup::core::is_space(cmd[pos - 1])) { - ++pos; - continue; - } - auto next_pos = pos + 2; - if (next_pos >= cmd.size()) { - return true; - } - auto c = cmd[next_pos]; - if (c == 'D' || c == 'M' || c == 'F' || c == 'G' || c == 'P' || c == 'T' || c == 'Q' || c == 'V' - || pup::core::is_space(c)) { - return true; - } - ++pos; - } - return false; + // The invocations build_dep_scans draws from, gated the same way: the suppression stands for a + // compile that writes its own depfile, and nothing else in the command is that compile (#357). + return std::ranges::any_of(scannable_prefix(tokens.invocations()), [](auto invocation) { + return compiler_index(invocation).has_value() && std::ranges::any_of(invocation, is_dep_flag_word); + }); } auto GccScanner::build_dep_scans(CommandInfo const& /*cmd*/, CommandTokens const& tokens) const diff --git a/test/unit/test_dep_scanner.cpp b/test/unit/test_dep_scanner.cpp index d3ccc7a9..0a7da532 100644 --- a/test/unit/test_dep_scanner.cpp +++ b/test/unit/test_dep_scanner.cpp @@ -1352,6 +1352,76 @@ TEST_CASE("GccScanner scans the prefix before an invocation that is not a compil } } +TEST_CASE("A depfile flag outside the scannable prefix suppresses nothing", "[dep_scanner]") +{ + // The suppression stands for a compile that writes its own depfile, so it may only read the + // invocations a scan would have been built from (#357). + SECTION("gcc reads no flag past the last compile") + { + auto scanner = scanners::GccScanner {}; + REQUIRE(!scanner.has_dep_flags(tokens_of("gcc -c foo.c -o foo.o && echo -MD"))); + } + + SECTION("gcc reads no flag when the prefix is empty") + { + auto scanner = scanners::GccScanner {}; + REQUIRE(!scanner.has_dep_flags(tokens_of("cd sub && gcc -MD -c foo.c -o foo.o"))); + } + + SECTION("clang-cl reads no flag past the last compile") + { + auto scanner = scanners::ClangClScanner {}; + REQUIRE(!scanner.has_dep_flags(tokens_of("clang-cl -c foo.cpp -o foo.obj && echo /clang:-MFfoo.obj.d"))); + } + + SECTION("clang-cl reads no flag when the prefix is empty") + { + auto scanner = scanners::ClangClScanner {}; + REQUIRE(!scanner.has_dep_flags(tokens_of("cd sub && clang-cl /clang:-MFfoo.obj.d -c foo.cpp -o foo.obj"))); + } + + SECTION("gcc reads no flag from an invocation that runs no compiler") + { + auto scanner = scanners::GccScanner {}; + REQUIRE(!scanner.has_dep_flags(tokens_of("echo -MD && gcc -c foo.c -o foo.o"))); + } + + SECTION("clang-cl reads no flag from an invocation that runs no compiler") + { + auto scanner = scanners::ClangClScanner {}; + REQUIRE(!scanner.has_dep_flags(tokens_of("echo /clang:-MFfoo.obj.d && clang-cl -c foo.cpp -o foo.obj"))); + } + + SECTION("an announcing invocation hides no uncovered sibling object") + { + auto scanner = scanners::GccScanner {}; + REQUIRE(!scanner.has_dep_flags( + tokens_of("echo -MD && gcc -c a.c -o a.o && cd . && gcc -c b.c -o b.o") + )); + } +} + +TEST_CASE("A depfile flag inside the scannable prefix still suppresses", "[dep_scanner]") +{ + SECTION("gcc reads a flag behind an invocation that changes nothing") + { + auto scanner = scanners::GccScanner {}; + REQUIRE(scanner.has_dep_flags(tokens_of("echo building && gcc -MD -c foo.c -o foo.o"))); + } + + SECTION("gcc reads a flag in an earlier compile of an all-compile command") + { + auto scanner = scanners::GccScanner {}; + REQUIRE(scanner.has_dep_flags(tokens_of("gcc -MD -c a.c -o a.o && gcc -c b.c -o b.o"))); + } + + SECTION("clang-cl reads a flag behind an invocation that changes nothing") + { + auto scanner = scanners::ClangClScanner {}; + REQUIRE(scanner.has_dep_flags(tokens_of("echo building && clang-cl /clang:-MFfoo.obj.d -c foo.cpp -o foo.obj"))); + } +} + TEST_CASE("A scan reads the object from either spelling of the output flag", "[dep_scanner]") { SECTION("gcc's joined form") diff --git a/test/unit/test_e2e.cpp b/test/unit/test_e2e.cpp index 2593dd66..d7aab8ab 100644 --- a/test/unit/test_e2e.cpp +++ b/test/unit/test_e2e.cpp @@ -11302,6 +11302,53 @@ SCENARIO("A compile-shaped rule with no dependency scan is reported", "[e2e][str } } +SCENARIO("A depfile flag the compile never carried hides no unscanned object", "[e2e][strict][depscan]") +{ + // The suppression exists for a compile that writes its own depfile; read across the whole + // command text it was defeated by any word spelling one, including in a later invocation (#357). + GIVEN("a rule that spells a depfile flag after the compile it could not scan") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", "#include \"a.h\"\nint a(void){return 0;}\n"); + f.write_file("a.h", "#define A 1\n"); + f.write_file("Tupfile", ": a.c |> cd . && gcc -c a.c -o a.o && echo -MD |> a.o\n"); + + WHEN("parse reports on the rule") + { + auto result = f.pup({ "parse" }); + + THEN("the object is still named") + { + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE(result.stderr_output.find("no dependency scan") != std::string::npos); + REQUIRE(result.stderr_output.find("a.o") != std::string::npos); + } + } + } + + GIVEN("a rule whose announcement carries the flag its compile does not") + { + auto f = E2EFixture { "glob_mixed_space" }; + f.write_file("a.c", "#include \"a.h\"\nint a(void){return 0;}\n"); + f.write_file("a.h", "#define A 1\n"); + f.write_file("Tupfile", ": a.c |> echo -MD && gcc -c a.c -o a.o && cp a.o b.o |> a.o b.o\n"); + + WHEN("parse reports on the rule") + { + auto result = f.pup({ "parse" }); + + THEN("the object no scan covers is still named") + { + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE(result.stderr_output.find("no dependency scan") != std::string::npos); + REQUIRE(result.stderr_output.find("b.o") != std::string::npos); + } + } + } +} + SCENARIO("Strict checker exempts the config-tree root in 3-tree builds", "[e2e][strict][out-of-tree-config]") { GIVEN("a 3-tree project whose config-tree root anchors with '='")