From 6e3b4d89a0c7cec503403f0638232d8402495f37 Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:27:55 -0400 Subject: [PATCH] Report compile-only flags on CMake link lines as a warning CMake passes CMAKE__FLAGS to the link line as well as the compile line, so warning flags, -pedantic and -D/-U defines (including the -DNDEBUG of Release builds) appear in the File API's link flags. The driver ignores them when linking, and Bazel's link lines do not carry them, so each one was a link_flags_diff error that every migration had to suppress by hand in ignore.link_flags. Measured in any2bazel-lab: every converged migration of three small projects suppressed the same kinds of flag, each agent rediscovering and justifying them in its own words (-Wall -Wextra -Wformat-security -pedantic on one, -Wno-format-truncation on another, -DNDEBUG on a third). Agents also suppressed front-end flags by hand: -fexceptions on lensfun, -w and -fno-common on CMake's sources, -fvisibility=hidden on GDAL and QGIS. A hand suppression is also where a reviewer can wave a real difference through. _diff_link_flags now sends these missing CMake link flags to a new WARN kind, link_compile_only_flags, which does not block `converged`: - warnings and -Wp, preprocessor pass-throughs (-W..., but not -Wl, or -Wa,, the latter because an LTO link runs the assembler), -w and -pedantic*; - -D and -U; - front-end -f flags that steer only the compiler, listed one by one: -fexceptions, -fno-exceptions, -fcommon, -fno-common, -fvisibility-inlines-hidden and -fvisibility=. Everything else stays a link_flags_diff error. Most -f flags matter at link time, so the list is explicit rather than a prefix, and -fopenmp, -pthread, -static*, -fsanitize* and -flto* are not in it. When an -flto* flag is on either side of a link, warning controls and the front-end flags stay errors, because LTO runs code generation, and diagnostics such as -Werror=lto-type-mismatch, at link time. ignore.link_flags still silences any of them entirely. SKILL.md lists the new kind, and the front-end flags, with the other no-action WARNs. The upstream test that used -fvisibility=hidden as its example of a link error now uses -Wl,--no-undefined. A built-in list is a design choice the maintainers may prefer to own; this one is reported rather than dropped, so nothing disappears silently. Not done: raising an unreferenced _EXPORTS define only when a source reads it, which would need the differ to read source files. In the lab's runs with this change, the first version of the list (without the front-end flags) still left agents suppressing -fexceptions on lensfun and -w and -fno-common on CMake. With the list above, neither project's next run suppressed a link flag by hand. Lab-Proposal: P11 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 10 +++--- docs/REFERENCE-cmake.md | 4 ++- scripts/diff.py | 56 ++++++++++++++++++++++++++--- tests/test_engine.py | 78 +++++++++++++++++++++++++++++++++++++++-- 4 files changed, 136 insertions(+), 12 deletions(-) diff --git a/SKILL.md b/SKILL.md index 627fbf8..9c1380d 100644 --- a/SKILL.md +++ b/SKILL.md @@ -325,9 +325,11 @@ reason about once the TUs underneath agree. also emits **WARN-only** kinds that don't block `converged` — `extra_tu` / `extra_target` (compiled/built on the Bazel side but not the reference) and `extra_test_tu` — note them but they need no action unless they point to -something you didn't intend to add. The WARN `configured_files_not_compared` -does need action: byte-compare each listed file against the one your Bazel -build generates. +something you didn't intend to add. `link_compile_only_flags` (warnings, +`-w`, `-pedantic`, `-D`/`-U`, and front-end flags such as `-fexceptions`, +`-fno-common` and `-fvisibility=` that CMake repeats on link lines) needs none +either. The WARN `configured_files_not_compared` does need action: byte-compare +each listed file against the one your Bazel build generates. #### Triage findings {#triage} @@ -364,7 +366,7 @@ correctness flags). | `defines_diff` | add each `cmake_only` define to `defines`, or `ignore.defines` it | | `includes_diff` | add the missing reference include root to `includes`; for a dep whose root is spelled differently each side, `ignore.include_map` it (preferred) or `ignore.include_prefixes` it | | `flags_diff` | add each `cmake_only` flag to `copts`, or `ignore.flags` it | -| `link_flags_diff`| add each `cmake_only` flag to the target's `linkopts`, or `ignore.link_flags` it if benign (e.g. a compile flag the reference repeats at link) | +| `link_flags_diff`| add each `cmake_only` flag to the target's `linkopts`, or `ignore.link_flags` it if benign (a compile flag the reference repeats at link that is not already reported as `link_compile_only_flags`) | | `missing_dep` | add the missing external/system dep to the target's `deps`/`linkopts`; if it's just a name spelled differently per build (`Catch2Main` vs `catch2_main`, `OpenSSL::SSL` vs `ssl`), add a `dep_map` entry. External deps are captured from both `-l` flags and archive-file inputs (e.g. `external/catch2+/libcatch2_main.a`) | | `missing_test_tu`| (tests on) add the test source to a `cc_test`, or `exclude_targets` if out of scope | | `test_binary_count` | (tests on, warning) differing number of test executables — investigate which side has the extra/missing binary | diff --git a/docs/REFERENCE-cmake.md b/docs/REFERENCE-cmake.md index 35c6f80..ce6227a 100644 --- a/docs/REFERENCE-cmake.md +++ b/docs/REFERENCE-cmake.md @@ -55,7 +55,9 @@ aquery must mirror. `catch2_main` / `ssl`. - **Link flags**: CMake repeats compile/codegen flags (`-fvisibility=hidden`, `-fno-common`) on the link line, where they're benign, while Bazel doesn't. - This is the common `ignore.link_flags` case. + The differ reports warnings, `-D`/`-U` and the common front-end `-f` flags + as `link_compile_only_flags`; any other benign one is an `ignore.link_flags` + case. ## Tests diff --git a/scripts/diff.py b/scripts/diff.py index 3d434cc..f77bed0 100644 --- a/scripts/diff.py +++ b/scripts/diff.py @@ -75,6 +75,8 @@ class Kind(str, Enum): INCLUDES_DIFF = "includes_diff" FLAGS_DIFF = "flags_diff" LINK_FLAGS_DIFF = "link_flags_diff" # executable link flags + # compile-only flags CMake repeats on a link line, absent on Bazel's (WARN) + LINK_COMPILE_ONLY_FLAGS = "link_compile_only_flags" MISSING_DEP = "missing_dep" # Java (CompileGroup) source-set comparison: MISSING_JAVA_SRC = "missing_java_src" # .java compiled in A but not B @@ -187,6 +189,35 @@ def _diff_tu(target: str, a: TranslationUnit, b: TranslationUnit, return out +# -f flags that only steer the compiler's front end or code generation for a +# translation unit, and that CMake repeats on link lines when they are in +# CMAKE__FLAGS. Listed one by one: most -f flags DO matter at link time +# (-fopenmp, -fsanitize*, -flto*, -fuse-ld=, -fprofile-*), so -f in general is +# not compile-only. +_COMPILE_ONLY_F_FLAGS = ("-fexceptions", "-fno-exceptions", "-fcommon", + "-fno-common", "-fvisibility-inlines-hidden") +_COMPILE_ONLY_F_PREFIXES = ("-fvisibility=",) + + +def _compile_only_on_link(flag: str, lto: bool = False) -> bool: + """A flag that only means something to the COMPILER, which CMake repeats on + link lines because CMAKE__FLAGS reach both: warnings (-W..., -w) and + -Wp, preprocessor pass-throughs (but not -Wl, or -Wa,), -pedantic*, -D/-U, + and the front-end -f flags listed above. The driver ignores them when + linking. -Wl, is the linker's own, and -Wa, is kept because an LTO link + runs the assembler. With `lto` (an -flto* flag on that link), warning + controls and the -f flags are kept too: code generation, and diagnostics + such as -Werror=lto-type-mismatch, happen at link time then. Flags that do + matter at link time (-fopenmp, -pthread, -static*, -fsanitize*, -flto*, + -f... in general) are not in this set and stay errors.""" + if flag.startswith(("-Wl,", "-Wa,")): + return False + is_f = flag in _COMPILE_ONLY_F_FLAGS or flag.startswith(_COMPILE_ONLY_F_PREFIXES) + if lto and ((flag.startswith("-W") and not flag.startswith("-Wp,")) or flag == "-w" or is_f): + return False + return is_f or flag == "-w" or flag.startswith(("-W", "-D", "-U", "-pedantic")) + + def _diff_link_flags(name: str, ta: "TargetView", tb: "TargetView", cfg: "MigrationConfig", notes: Optional[DiffNotes] = None) -> List[Discrepancy]: @@ -199,16 +230,31 @@ def _diff_link_flags(name: str, ta: "TargetView", tb: "TargetView", b_lf = {f for f in tb.link_flags if not cfg.link_flag_ignored(f)} if notes is not None: notes.bazel_only_link_flags.update(b_lf - a_lf) - if a_lf - b_lf: - return [Discrepancy( + out: List[Discrepancy] = [] + missing = a_lf - b_lf + lto = any(f.startswith("-flto") for f in a_lf | b_lf) + compile_only = {f for f in missing if _compile_only_on_link(f, lto)} + if missing - compile_only: + out.append(Discrepancy( kind=Kind.LINK_FLAGS_DIFF.value, severity=Severity.ERROR.value, target=name, detail="cmake link flags missing on bazel side", - cmake_only=sorted(a_lf - b_lf), + cmake_only=sorted(missing - compile_only), bazel_only=sorted(b_lf - a_lf), - )] - return [] + )) + if compile_only: + # Reported, not blocking: every CMake project that sets warning flags + # or -DNDEBUG in CMAKE__FLAGS has them on each link line. + out.append(Discrepancy( + kind=Kind.LINK_COMPILE_ONLY_FLAGS.value, + severity=Severity.WARN.value, + target=name, + detail="compile-only flags (warnings, -pedantic, -D/-U, front-end " + "-f flags) on the cmake link line; no effect when linking", + cmake_only=sorted(compile_only), + )) + return out def _norm_includes(includes, cfg) -> tuple: diff --git a/tests/test_engine.py b/tests/test_engine.py index 4b83922..0fc56d0 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -1090,6 +1090,78 @@ def test_missing_link_flag_is_caught(): assert "-rdynamic" in lf[0].cmake_only +def test_compile_only_flags_on_cmake_link_line_are_a_warning(): + # CMAKE__FLAGS reach CMake's link line too; warnings, -pedantic and + # -D/-U mean nothing to the linker, so they must not block convergence -- + # but link-relevant flags (-fopenmp, -Wl,...) still must. + link = ["-Wall", "-Wno-format-truncation", "-pedantic", "-DNDEBUG", + "-UFOO", "-Wp,-D_FORTIFY_SOURCE=2", "-fopenmp", "-Wl,--as-needed", + "-Wa,--noexecstack"] + a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", link, False)) + b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", ["-fopenmp"], True)) + discs = diff_models(a, b) + by_kind = {d.kind: d for d in discs} + assert set(by_kind) == {"link_flags_diff", "link_compile_only_flags"}, discs + assert by_kind["link_flags_diff"].severity == "error" + assert by_kind["link_flags_diff"].cmake_only == ["-Wa,--noexecstack", + "-Wl,--as-needed"] + w = by_kind["link_compile_only_flags"] + assert w.severity == "warn" + assert w.cmake_only == ["-DNDEBUG", "-UFOO", "-Wall", "-Wno-format-truncation", + "-Wp,-D_FORTIFY_SOURCE=2", "-pedantic"] + + +def test_front_end_f_flags_and_w_on_cmake_link_line_are_a_warning(): + # Seen hand-suppressed in lab runs: -fexceptions (lensfun, 7 runs), -w and + # -fno-common (CMake), -fvisibility=hidden (GDAL, QGIS). They steer the + # compiler only. Other -f flags stay link errors. + link = ["-fexceptions", "-fno-common", "-w", "-fvisibility=hidden", + "-fvisibility-inlines-hidden", "-fopenmp", "-fsanitize=address", + "-fuse-ld=gold"] + a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", link, False)) + b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", [], True)) + by_kind = {d.kind: d for d in diff_models(a, b)} + assert by_kind["link_flags_diff"].cmake_only == ["-fopenmp", "-fsanitize=address", + "-fuse-ld=gold"] + assert by_kind["link_compile_only_flags"].cmake_only == [ + "-fexceptions", "-fno-common", "-fvisibility-inlines-hidden", + "-fvisibility=hidden", "-w"] + + +def test_front_end_f_flags_stay_link_errors_under_lto(): + a = _bs(CanonicalModel(), is_bazel=False) + a.add(_exe("app", ["-flto", "-fvisibility=hidden", "-w", "-fexceptions"], False)) + b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", ["-flto"], True)) + by_kind = {d.kind: d for d in diff_models(a, b)} + assert by_kind["link_flags_diff"].cmake_only == ["-fexceptions", "-fvisibility=hidden", "-w"] + assert "link_compile_only_flags" not in by_kind + + +def test_warning_controls_stay_link_errors_under_lto(): + # With -flto the compiler's middle and back end run at link time, and so + # do their diagnostics: a missing -Werror=lto-type-mismatch matters there. + a = _bs(CanonicalModel(), is_bazel=False) + a.add(_exe("app", ["-flto", "-Werror=lto-type-mismatch", "-DNDEBUG", + "-Wp,-D_FORTIFY_SOURCE=2", "-pedantic"], False)) + b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", ["-flto"], True)) + by_kind = {d.kind: d for d in diff_models(a, b)} + assert by_kind["link_flags_diff"].cmake_only == ["-Werror=lto-type-mismatch"] + # preprocessor and front-end flags still have no effect at an LTO link + assert by_kind["link_compile_only_flags"].cmake_only == \ + ["-DNDEBUG", "-Wp,-D_FORTIFY_SOURCE=2", "-pedantic"] + + +def test_only_compile_only_link_flags_missing_converges(): + a = _bs(CanonicalModel(), is_bazel=False) + a.add(_exe("app", ["-Wall", "-pedantic", "-fopenmp"], False)) + b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", ["-fopenmp"], True)) + res = summarize(diff_models(a, b)) + assert res["converged"] and res["warnings"] == 1, res + # and ignore.link_flags still silences them entirely + cfg = MigrationConfig(ignore_link_flag_prefixes=("-W", "-pedantic")) + assert diff_models(a, b, cfg) == [] + + def test_extra_bazel_link_flag_tolerated(): # Asymmetric: an extra link flag only on the bazel side must NOT fail. a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", ["-pthread"], False)) @@ -1111,10 +1183,12 @@ def test_link_toolchain_noise_stripped(): def test_ignore_link_flags_suppresses(): - a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", ["-fvisibility=hidden", "-pthread"], False)) + # (-fvisibility=hidden was the example here; it is now reported as a + # compile-only warning, so a link-relevant flag stands in for it.) + a = _bs(CanonicalModel(), is_bazel=False); a.add(_exe("app", ["-Wl,--no-undefined", "-pthread"], False)) b = _bs(CanonicalModel(), is_bazel=True); b.add(_exe("app", ["-pthread"], True)) assert not summarize(diff_models(a, b))["converged"] - cfg = MigrationConfig(ignore_link_flags={"-fvisibility=hidden"}) + cfg = MigrationConfig(ignore_link_flags={"-Wl,--no-undefined"}) assert summarize(diff_models(a, b, cfg))["converged"]