ci: let clang-tidy-diff parse the two AUTOMOC self-including sources, and name option-coverage by its job id (fixes #624, fixes #637)

[Yaraslaut]

Two tickets on the ci.yml / clang-tidy tree, one commit each.

---

#637 — ci.yml named a job that does not exist

linux-all-features' MORPH_BUILD_BANK_GUI rationale pointed the reader at "the check-workflow-option-coverage job below". The job is option-coverage; check_workflow_option_coverage.py is the script it runs. Reproduced on c55ea5b7:

$ grep -n 'check-workflow-option-coverage' .github/workflows/ci.yml
1838:          # fetch requires. The check-workflow-option-coverage job below is
$ grep -rn '^  check-workflow-option-coverage:' .github/workflows/
$ grep -n '^  option-coverage:' .github/workflows/ci.yml
2449:  option-coverage:

The replacement names both, so the next reader does not have to guess which of the two the sentence meant.

Swept for the same defect tree-wide. Over every # comment line in .github/workflows/*.yml, the hyphenated lowercase tokens immediately preceding the word "job" are automoc-include-lint, scenario-coverage, ladder-tests, linux-all-features, linux-coverage, option-coverage and dependency-free. All but the last resolve to a real job id somewhere in the tree; dependency-free job is an adjective. This was the only stale one.

The residual, decided explicitly: accepted, and filed

#638's check_workflow_job_banners.py would not have caught #637, and still would not. Its rule A inspects the line after a banner and rule B inspects whether a job has one; a job id inside a comment body is neither. So nothing gates this class.

I am accepting that residual here rather than closing it, for two reasons:

1. It is a separate change, not a line of this one. This repository requires a gate to carry a self-test (test_check_workflow_option_coverage.sh, test_check_workflow_job_banners.sh), so closing it means a new script, a new self-test, and a new CI step. AGENTS.md is explicit that a finding gets filed and linked rather than folded in.
2. The cheap version of the rule is not free of judgement. The measurement above is also the feasibility study: restricted to hyphenated lowercase tokens, the rule flags two lines tree-wide — ci.yml's all-features Configure step points at a job name that does not exist #637's, and one false positive — and it cannot catch a stale reference to a single-word job id such as valgrind or windows at all.

Filed as #645, carrying that measurement.

---

#624 — clang-tidy-diff could not parse two translation units

Both sources end with #include "<own-basename>.moc". AUTOMOC writes that header at build time; the job configures and never builds. Reproduced locally on c55ea5b7 with the job's own configure flags and clang-tidy 22.1.8, one line added to each file:

/.../examples/common/testkit/test_qml_surface.cpp:757:10: error: 'test_qml_surface.moc' file not found [clang-diagnostic-error]
/.../src/qt/forms/tests/tst_main.cpp:70:10: error: 'tst_main.moc' file not found [clang-diagnostic-error]
exit=1

Which of the two fixes, and what the other would have cost

The ticket offered "restore AUTOMOC generation" against "filter clang-diagnostic-error for these TUs" and said to pick one. The second one does not exist. clang-tidy does not let a compiler error be filtered — measured on tst_main.cpp with the moc absent:

--- baseline (no filter)              : exit=1   'tst_main.moc' file not found [clang-diagnostic-error]
--- -checks=-clang-diagnostic-error   : exit=1   'tst_main.moc' file not found [clang-diagnostic-error]
--- -checks=-clang-diagnostic-*       : exit=1   'tst_main.moc' file not found [clang-diagnostic-error]
--- -warnings-as-errors= empty        : exit=1   'tst_main.moc' file not found [clang-diagnostic-error]

So "filter it" would have had to mean grepping clang-tidy-diff.py's output and overriding its exit code — morph#479's defect, a gate that cannot fail, rebuilt on purpose. That is what the rejected option costs, and it is why this is not a judgement call between two tenable designs.

What generating them costs, measured

The comment at the old :2229 argues against a build on caching grounds, so the cost was measured rather than assumed. On a 12-core Linux box with USE_COMPILER_CACHE=OFF (the local default is fastcache-cc, which reported the same work as 3s and would have been a useless number):

	edges	compilations	links	moc runs	wall
every *_autogen target (91)	499	264	—	91	179s
the two needed	100	67	7	9	31s

39 of the 67 are the vendored Lightweight ORM, pulled in because a <target>_autogen target depends on its target's link dependencies — that, not moc, is what the step costs. So the step names its two targets instead of building them all. Compiler caching stays unwired: 67 objects against the 703 a real build of this configure would compile.

A four-core hosted runner will be slower than 31s. That is inferred, not measured.

Both comments updated, as the ticket required

• The "No Build step" paragraph answered "which generated headers exist without a build?" for configure_file() output while reading as though it had settled the question. It now says which kind it covers and points at the AUTOMOC step for the kind it does not, and reads "no full Build step".
• The -Wno-missing-include-dirs comment made the matching over-broad claim — that no source under examples/, include/ or src/ includes a generated header. What the "Check no generated moc include ascends" guard covers is an ascending moc_<name>.h; a same-name .moc in the target's own autogen dir is a different shape and the flag says nothing about it, because the error is the file being absent, not the directory. Corrected.

The list is written out; the check is derived

Two target names are spelled out in the step, so they are greppable and reviewable. The check underneath rescans every tracked C++ source for the self-include idiom and fails if a .moc a source names is still missing — which is what a third source adopting the idiom looks like — and fails if it finds no self-include at all.

---

Verification

ci.yml is the only file this branch touches. The safety of #624 rests on one claim: the clang-tidy job now analyses both translation units to completion and reaches the changed line. A green clang-tidy-diff would not show that, so it was made to go red on a line in each file. int laneProbeReachMarker() { return 0; } inserted above each .moc include, run with the job's own clang-tidy-diff.py arguments:

--- violation: clang-tidy-diff exit=1
/.../src/qt/forms/tests/tst_main.cpp:69:5: error: function 'laneProbeReachMarker' can be made static
  or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage,...]
/.../examples/common/testkit/test_qml_surface.cpp:756:5: error: function 'laneProbeReachMarker' can be
  made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage,...]
    clang-diagnostic-error count: 0

--- harmless: clang-tidy-diff exit=0
    clang-diagnostic-error count: 0

A finding reported at tst_main.cpp:69 and test_qml_surface.cpp:756 — the changed lines — is the proof; the same diff before this change produced two clang-diagnostic-errors and no findings at all. Both probe edits were reverted; neither source file is modified by this branch (test_qml_surface.cpp is held by a concurrent lane and was only read and probed).

The step's own controls, from a clean configure-only tree (rm -rf build/clang-debug, configure, 0 .moc on disk), running the step body extracted from ci.yml rather than a retyped copy:

=== 2. NEGATIVE CONTROL: the step's own check, run before the build ===
::error::examples/common/testkit/test_qml_surface.cpp: test_qml_surface.moc was not generated -- ...
::error::src/qt/forms/tests/tst_main.cpp: tst_main.moc was not generated -- ...
check-before-build exit=1  (must be 1)

=== 3. VACUITY CONTROL: same check with the scan finding nothing ===
::error::no tracked source self-includes a .moc -- this step's scan has stopped detecting the idiom it exists for
empty-scan exit=1  (must be 1)

=== 4. the step as written ===
ok: examples/common/testkit/test_qml_surface.cpp -> test_qml_surface.moc
ok: src/qt/forms/tests/tst_main.cpp -> tst_main.moc
ok: 2 self-included .moc header(s), 0 missing
step exit=0

Gates run locally after the edits, all green:

python3 scripts/check_workflow_job_banners.py .     -> all 24 section banner(s) introduce the job they describe
bash scripts/test_check_workflow_job_banners.sh     -> all cases passed.
python3 scripts/check_workflow_option_coverage.py . -> 16 MORPH_BUILD_* option(s) declared, all accounted for
bash scripts/check_tidy_suppression_scope.sh clang-tidy . -> tests/.clang-tidy records the reach it actually has
bash scripts/check_spec_citations.sh                -> Prose lint OK
bash scripts/check_ci_clang_pin.sh                  -> all 7 CI-clang-pin assertion(s) agree with ci.yml

.github/workflows/ci.yml also parses as YAML after the edit — worth stating because the first draft of the new step was named Generate the AUTOMOC headers two sources #include directly, and YAML silently truncated the name at the unquoted #. Caught by parsing the file and printing the step names back.

Not verified

• Nothing was run on a hosted runner. Local Qt is the distro's 6.11.2, not aqtinstall's QT_VERSION; local clang-tidy is 22.1.8, not apt.llvm.org's clang-tidy-22; local clang++ stands in for clang++-22.
• The CI wall-clock cost of the new step. 31s on 12 cores is measured; four cores will be worse.
• Whether the step behaves when a future source self-includes a .moc from a target this configure does not enable — that path prints a ::warning:: and continues, by design, since clang-tidy cannot analyse such a file either.

---

Findings filed, not folded

• Nothing gates a prose reference to a job id inside a workflow comment #645 — nothing gates a prose reference to a job id in a workflow comment. The ci.yml's all-features Configure step points at a job name that does not exist #637 residual, with the tree-wide sweep as evidence.
• 84 clang-tidy findings become reachable in the two AUTOMOC self-including TUs once #624's fix lands #646 — 84 clang-tidy findings (81 in test_qml_surface.cpp, 3 in tst_main.cpp) become reachable once this lands, because no check has ever run over either file. Nothing breaks today — clang-tidy-diff only reports changed lines — but the first PR to touch one of those lines will fail on debt that is not its own.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[Yaraslaut]

Runner verification

The ticket's framing was wrong and the lane was right to say so. #624 presented a trade-off between generating the AUTOMOC headers and filtering clang-diagnostic-error. I reproduced the lane's finding independently, with clang-tidy 22.1.8 on a two-line probe that self-includes a nonexistent .moc:

control: clean file, real check enabled                       exit=0
missing .moc, -checks=…,-clang-diagnostic-error               exit=1  file not found
missing .moc, -checks=…,-clang-diagnostic-*                   exit=1  file not found

The control matters: my first attempt at this probe passed -checks=-clang-diagnostic-* alone, which disables every check and exits 1 with "no checks enabled" — both the broken file and a clean one exited 1, so the measurement said nothing. Re-run with a real check enabled, the clean file exits 0 and both filtered forms still exit 1. So the diagnostic genuinely cannot be suppressed, and presenting two designs would have been presenting one real option and one that does not exist.

The step's two vacuity guards are in the script, not just in the report — read from the diff:

if found == 0:
    print("::error::no tracked source self-includes a .moc -- this "
          "step's scan has stopped detecting the idiom it exists for")
    sys.exit(1)
print(f"ok: {found} self-included .moc header(s), {missing} missing")
sys.exit(1 if missing else 0)

A scan that stops matching the idiom fails instead of reporting green, and the list is derived from git ls-files rather than hardcoding the two known files, so a third self-including source is covered the day it is added. That is the right shape for this class of gate.

What I did not verify, and it is the branch's load-bearing claim. That the clang-tidy job now analyses both TUs to completion and reaches the changed line rests on the lane's local probe (laneProbeReachMarker findings at tst_main.cpp:69 and test_qml_surface.cpp:756, clang-diagnostic-error count: 0). Reproducing it needs a full Qt configure, which I did not run. I am accepting it on the strength of the probe's design — a marker on the changed line, findings at that line, and the same diff before the change producing two clang-diagnostic-errors and no findings — not on a green CI leg.

And the lane is right that CI cannot confirm it here. This PR's diff is ci.yml only, so clang-tidy-diff will analyse no C++ at all and go green without exercising the effect. The new step still runs and fails loudly if the mocs are absent. When this lands, the first PR that touches either file is the real test.

#646 is the consequence worth flagging before merge: 84 findings become reachable (81 in test_qml_surface.cpp, 73 of them misc-const-correctness). Nothing breaks on landing, but the first PR touching one of those lines inherits debt that is not its own. That is filed, not folded, and it is the kind of thing I would rather have on a ticket than discovered by whoever next edits that file.

#645 (nothing gates a prose job-id reference in a comment) records the #637 residual with the measurement showing why the cheap rule is not clean — two hits tree-wide, one of them a false positive, and no coverage of single-word job ids. Accepting with that recorded beats closing it by assertion.

Not merged: CI incomplete. Next sweep counts the checks.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!