Skip to content

The clang runbook's fourth verification row runs in no CI job, and the walk it names exits 0 on the failure it exists to detect #245

Description

@monkopedia-coder

Code-health suggestion (proactive — staged for your review)

Category: test-gap / green signal that measures nothing

Where:

  • clangwalk/src/nativeMain/kotlin/AstWalk.kt:62-67 (the three early returns)
  • .github/workflows/root-build.yml:42 (clangwalk/** in the trigger)
  • docs/clang-runbook.md:103 (row 4 of the verification table)

Two halves of the same gap

docs/clang-runbook.md:98-103 is a four-row "what proves the clang path works" table. #222 pulled
rows 1–3 into root-build.yml. Row 4 — :clangwalk:runReleaseExecutableKlinker — is still the
residue: no workflow compiles :clangwalk, let alone runs it.
And the walk itself, when run, has
three paths that exit 0 having asserted nothing.

1. No CI job compiles :clangwalk, but its path IS in the trigger

root-build.yml fires on clangwalk/** (:42, with the honest comment "in settings.gradle.kts, so
ktlintCheck covers it"
). Both of its jobs were dry-run here, each with a positive control on the
same invocation:

$ ./gradlew --no-daemon :krapper:nativeTest :krapper_model:nativeTest \
      :feature-tests:nativeTest ktlintCheck --dry-run     # root-build job 1
tasks in graph                                   113
:clangwalk: tasks that are not ktlint tasks        0
:krapper: compile tasks           (control)        2

$ ./gradlew --no-daemon :featuregen:nativeTest :cppfixture:nativeTest \
      :krapper:noncopyableDeterminismCheck --dry-run       # root-build job 2
tasks in graph                                    40
:clangwalk: tasks of any kind                      0
:featuregen: compile tasks        (control)        2

$ ./gradlew --no-daemon ktlintCheck --dry-run
tasks in graph                                    80
matching compile|link|kplusplusSync|cinterop       0
matching ktlint                   (control)       80

So a PR whose whole content is clangwalk/ triggers root-build, goes green, and nothing compiled
the change.
:clangwalk is the only module in settings.gradle.kts in that state.

It is not expensive and it is not broken — both measured on this box just now:

$ ./gradlew --no-daemon :clangwalk:linkDebugExecutableKlinker
> Task :clangwalk:kplusplusSync           # executed, not UP-TO-DATE
> Task :clangwalk:cinteropKplusplusNative
> Task :clangwalk:compileKotlinNative
> Task :clangwalk:linkDebugExecutableKlinker
BUILD SUCCESSFUL in 1m 10s

$ LD_LIBRARY_PATH=$(llvm-config --libdir) ./clangwalk/build/bin/klinker/clangwalkDebug/clangwalk
  [PASS] walked exactly 10 top-level decls — got 10
  ... 15 checks, all PASS ...
clangwalk: all self-checks passed — walked 10 top-level decls via real libclang-cpp
EXIT=0

15 working assertions over the committed stage-0 Clang-AST seed (kpp.frontend.clangwalk=seed), for
~70 s, running nowhere. #73"clangwalk release generation broken on LLVM-22.1.6" — is what this
catches, and it was found by hand.

2. The walk reports success on the failure it exists to detect

AstWalk.kt:32-34 states the intent: "let main() exit non-zero if ANY failed — so
:clangwalk:runReleaseExecutableKlinker is self-verifying instead of a println demo."
Three lines
below, it is not:

val unit = buildASTFromCode(source, "input.cc")
    ?: return@memScoped println("clangwalk: buildASTFromCode returned null")   // :62-63
val context = unit.getASTContext()
    ?: return@memScoped println("clangwalk: no ASTContext")                    // :64-65
val tu = context.getTranslationUnitDecl()
    ?: return@memScoped println("clangwalk: no TranslationUnitDecl")           // :66-67

All three println and return normally. main returns Unit, the process exits 0, the
exitProcess(1) at :190-193 is never reached, and runReleaseExecutableKlinker reports
BUILD SUCCESSFUL. Those three are exactly the shape of a broken binding set: the wrapper still
links, the API still exists, and it returns null at runtime — the runtime half of #73.

Compounding it, failures == 0 is also satisfied by zero checks having run. There is no
"at least N checks executed" guard, so the pass condition cannot tell a full run from an aborted one.
Compare noncopyableDeterminismCheck (krapper/.../CppParser.kt:203-209), which does carry that
guard — "Sanity: the fixture really did parse (an empty TU would pass both checks vacuously)" — so
the correct shape already exists in this repo, one module over.

Proposed:

  1. AstWalk.kt: make the three early returns check(...)-and-fail (or exitProcess(1)), and add a
    check("N self-checks ran", checksRun >= 15) so a truncated run cannot report success.
    Watch it fail before accepting it: point buildASTFromCode at unparseable source and confirm
    the exit status is non-zero — today it is 0.
  2. root-build.yml: add :clangwalk:runDebugExecutableKlinker (debug, not release —
    linkerOpts("--gc-sections") already makes debug link, per clangwalk/build.gradle.kts:43) to a
    job. It needs no in-tree :krapper release link, so it fits the cheaper unit-tests job. A
    non-zero exit from the walk fails the task directly, so it needs no JUnit-XML gate.
  3. docs/clang-runbook.md:102 says of row 3 "Wired into :krapper:check (No CI workflow runs the root Gradle build: 566 tests, all ktlint, and the #101 determinism lock execute on developer machines only #222), so it no longer
    needs to be typed by hand."
    Row 4 should be able to say the same.

Why: it converts a documented, working, cheap, end-to-end verification into a standing one, and
it removes a "PASSED" that a real failure produces. Item 1 is worth doing on its own even if item 2
is declined — a self-check that reports success when the thing it checks is absent is worse than no
self-check.

Effort: mechanical (1), mechanical (2), trivial (3).

Refs #222, Refs #73, Refs #8.

Filed by the nightly code-health pass. To act on it: relabel agent-workable (it'll flow into triage→work). To decline: close it — it won't be suggested again.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    health-suggestionProactive code-health suggestion from the nightly analysis pass (staged for review)needs-decisionRequires user input on direction, scope, or design

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions