feat(generator): port binding generator from libclang to cc/v4 - #65
Merged
flexiondotorg merged 12 commits intoAug 1, 2026
Conversation
Name the boundary that already existed between parsing and code generation, so a second parser backend can replace libclang without touching the emitter. - Add HeaderParser with Parse(skips *SkipCollector) *Module and Version() string - Turn the existing Parse into a method on the new clangParser type, body unchanged - Drop the clang import from main.go, which now calls through the interface - Assert the implementation satisfies the interface at compile time - Record the boundary in AGENTS.md and docs/DEVELOPMENT.md This ships inert. just generate produces a zero-byte diff in *.gen.go and the skip count stays at 245. Refs: WW-138
Give a parser swap a structured check. The only prior signal was a 63,333-line byte diff across five generated files, which cannot tell a wrong typedef name from a reordered struct or a missing comment. - Add -dump-ir, writing three deterministic goldens under internal/generator/testdata/ir/ - Split structure from comments, so a comment-attachment gap does not drown the structural signal - Record skips as sorted (symbol, reason) pairs, since reason text is emitted into the generated files and is part of the byte-identical gate - Normalise ctype to a repo-relative include/... form, because libclang spells an unnamed union with the header's absolute path and the goldens would otherwise match only on the machine that wrote them - Expand FuncType and UnionType in renderType rather than calling String(), which returns the literal "func" and "union" Comment volume settled at 13,012 IR-side lines across 2,907 records, asserted by TestCommentGoldenLineCount. The 11,142 figure measures the emitted side and also holds. This ships inert. just generate produces a zero-byte diff in *.gen.go and the skip count stays at 245. Refs: WW-139
…lang
Drop the cgo dependency on github.com/Newbluecake/bootstrap, an
abandoned fork whose newest code targets clang 14 and which broke twice
on CXCursor_OMPArraySectionExpr, a cursor kind removed in Clang 18. The
generator is now pure Go and carries no clang version pin.
- Add ccParser, implementing HeaderParser on modernc.org/cc/v4 v4.29.2,
and delete the libclang implementation
- Attach doc comments by lexing raw header bytes, since cc/v4 has no
comment node and no absolute sep position. Take the nearest preceding
comment and reject it when the text between holds ; { } # or @, which
is Clang's own rule; anchor a trailing comment on the name token
- Add spellingSite, which recovers true positions for names written as
macro arguments. FF_PAD_STRUCTURE collapses every AVBPrint field to
one line, and cc/v4 keeps no pre-substitution offset
- Replace the gcc -E -v scrape with hostCppConfig, so include discovery
no longer depends on the Nix-pinned toolchain
- Pin -std=gnu11 with the generator's defines, and surface Translate
panics as errors, both under test
- Drop the unused import "C" from generator.go, which blocked
CGO_ENABLED=0
Comment association is 2907 of 2907. Output is byte-identical: just
generate leaves *.gen.go and all three IR goldens unchanged, with
skipCeiling at 245 and 245 markers.
IR is byte-identical across linux/amd64 and linux/arm64, pinned by
TestGeneratorIRIsIdenticalAcrossTargets. The darwin pair stays untested,
because cc.NewConfig probes the host compiler and a darwin target
preprocesses glibc headers under Apple's ABI. Closing that needs a macOS
CI job.
Refs: WW-141
The generator is pure Go since the cc/v4 swap, so the three libclang workarounds pushed to main on 5 June 2026 are now dead weight. - Drop the libclang-20-dev apt step and the Homebrew llvm@20 step from go-test.yml, with the comments that forbade adding the system include path - Restore plain go build ./... and go test ./... in go-release.yml, which had filtered out internal/generator - Widen golangci-lint to ./..., putting internal/generator back in scope - Run the drift gate on all four platform legs. IS_GATE and both conditionals are gone, so darwin no longer skips it - Drop the grep -v filter that hid the real darwin compile error - Remove the llvmPackages_20 pin from flake.nix, along with CGO_LDFLAGS, CPATH and the macOS clang include export. Darwin libcxx repoints to llvmPackages.libcxx, because the builder reads LIBCXX_INCLUDE for zimg. gcc stays, since cc.NewConfig probes a host C compiler Verified locally: nix flake check, just lint, just test and just generate all pass, with a zero-byte diff in *.gen.go. Whether the two darwin legs go green is untested and only a real CI run settles it. gcc 13, 14 and 15 each produce byte-identical output on this host, and clang 21 fails outright on glibc headers; macOS runners have only Apple clang against a different header set. Refs: WW-142
Contributor
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
Contributor
Author
|
@cubic-dev-ai Review this |
Contributor
@flexiondotorg I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 37 files
Confidence score: 3/5
- In
internal/generator/type.go,paramsandrecordFunctioncallparseTypewithout a syntax specifier, so valid anonymous struct parameter/return types can trigger the new guard and abort regeneration; this can break codegen for otherwise valid signatures — pass the correct syntax specifier through these call sites (or relax the guard for this path). - In
internal/generator/ctypename.go, pointerconstqualifiers inside parenthesized declarators are dropped, so emittedCTypeNamevalues can diverge from the declared C type and produce incorrect type metadata/interop output — recurse through nestedDeclaratornodes when collecting pointer qualifiers.
Not reviewed (too large): internal/generator/testdata/ir/comments.txt (~15,920 lines), internal/generator/testdata/ir/structure.txt (~10,876 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/generator/type.go">
<violation number="1" location="internal/generator/type.go:368">
P1: A valid anonymous struct in a function parameter or return type now aborts regeneration. `params` and `recordFunction` invoke `parseType` without supplying the syntax specifier, so this guard panics instead of registering or representing the type; propagate that specifier through those paths.</violation>
</file>
<file name="internal/generator/ctypename.go">
<violation number="1" location="internal/generator/ctypename.go:85">
P2: Const qualifiers on parenthesized pointer fields are silently omitted, so their `CTypeName` no longer matches the written C type. Recursing through nested `Declarator` nodes while collecting pointer qualifiers would preserve forms such as `int (*const callback)(int)` and `int (*const table)[4]`.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
ccPointerQualifiers walked only the outer declarator's Pointer chain. A
parenthesised declarator keeps its '*' on a nested Declarator, so a field
written "int (*const callback)(int)" or "int (*const table)[4]" lost its
pointer const and Field.CTypeName no longer matched the written C type.
struct.go reads that spelling to decide how a field is wrapped, so the
result was a silently wrong string rather than a loud failure.
The walk now takes the outer Pointer chain, then recurses into the nested
declarator a '(' Declarator ')' form wraps, appending what it carries
after. Both steps run left to right, which keeps the whole list in source
order, and source order reversed is the outward-in pointer level order
pointerQual indexes.
The paren wrap now trims the trailing space a pointer qualifier leaves,
because nothing follows it inside parentheses: clang spells
"int (*const)[4]", not "int (*const )[4]".
No FFmpeg header writes this form today, so all five *.gen.go files and
all three IR goldens are unchanged.
Contributor
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
Three CI legs failed because the parse read the runner's own C library. cc.NewConfig runs the host compiler for its predefined macros and its include search paths, so the emitted bindings were a function of the build machine. That broke two ways. Both darwin legs failed on a real difference in structure.txt, confined to the constant set. libavutil/mathematics.h wraps all 26 M_* constants in an #ifndef, so every name the C library leaves undefined becomes a macro of an FFmpeg header, passes the location filter in ccWalk.macros and enters the bindings. glibc defines all 26 under _GNU_SOURCE and the Apple SDK defines only the 13 without the f suffix, so macOS emitted 13 constants, M_Ef first, that Linux never produced. The linux/arm64 leg failed harder, before any comparison. glibc's aarch64 bits/math-vector.h declares its vector-math prototypes with a construct cc/v4 cannot parse, so no header translated at all. That header has no amd64 counterpart, which is why the dev host never met it. Stub the eleven C library headers the FFmpeg headers include, embed them into the binary and serve them through cc.Config.FS. Predefined becomes a literal in ccconfig.go rather than a `cc -dM -E -` dump, and the target pair now selects a cc.NewABI table and nothing else. The parse reads only include/ and sysinclude/. This was chosen over the narrower fix of filtering emitted macros by defining file, which does not work: M_Ef is defined in libavutil/mathematics.h, an FFmpeg header, so a file filter keeps it. Nor would it touch the aarch64 parse failure. Both symptoms have one cause, and removing the host headers removes both. The stubs are faithful where FFmpeg's #ifndef fallbacks can see them. math.h carries the 26 M_* names every libc defines plus NAN and INFINITY, and omits M_LOG2_10, M_LOG2_10f, M_PHI and M_PHIf, which no libc defines and which are therefore the four M_* constants the bindings carry on every platform. Fixed-width typedef widths cannot move the IR: parseType takes typedef sugar at every level, so size_t and int64_t reach it as themselves. Output does not move. *.gen.go and all three IR goldens come back byte-identical, and the generator runs with CC pointing at a nonexistent binary. TestGeneratorIRIsIdenticalAcrossTargets now runs all four targets from any host rather than only those sharing the host's operating system. That restriction existed because a darwin target on a Linux host preprocessed glibc under Apple's ABI, and it is exactly what hid this defect from every Linux run. What this does not solve: the FFmpeg headers still have to translate, so a construct cc/v4 cannot parse in an FFmpeg header still fails. The stubs must keep up with any new C library name a future FFmpeg release reaches for; a missing one is a parse failure naming the header, not a silent difference in output. And it proves nothing about whether the generator runs on a macOS or arm64 machine, which still needs CI.
Contributor
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
The generator step piped through grep -v "cgo-gcc-prolog\|deprecated" to drop cgo and libclang deprecation noise, then used || true to swallow grep's exit 1 when nothing matched. That also swallowed a failing generator, so just build carried on and produced stale bindings without saying so. The pure-Go generator emits none of that noise, so the filter now matches nothing and only the masking remains. This is the same workaround just removed from go-test.yml, where it hid the real darwin compile error. Drop both. set -euo pipefail then fails the recipe as it should. Refs: WW-142
Contributor
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
Contributor
Author
|
@cubic-dev-ai Re-review please |
Contributor
@flexiondotorg I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
2 issues found across 49 files
Confidence score: 4/5
- In
internal/generator/ctypename.go, C type spelling dropsvolatile/restrict, which can make generated IR diverge from headers and produce incorrect semantics when those qualifiers are used — preserve these supported qualifiers in the emitted type string/path. - In
docs/API-COVERAGE.md, the Reproduce instructions describe old cc/v4 behavior (probing gcc) and can mislead debugging or verification work — update the note to reflectccPredefinedplus embeddedsysinclude/stubs innewCCConfigbehavior.
Not reviewed (too large): internal/generator/testdata/ir/comments.txt (~15,920 lines), internal/generator/testdata/ir/structure.txt (~10,876 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/API-COVERAGE.md">
<violation number="1" location="docs/API-COVERAGE.md:66">
P3: The rewritten Reproduce note is now inaccurate: the cc/v4 parser no longer probes gcc for predefined macros or include paths. Those are a hardcoded ccPredefined literal plus the embedded sysinclude/ stubs (newCCConfig no longer calls cc.NewConfig), and the hardcoded GCC claims 15.3, not 15.2.0, so a reader following the toolchain note would be misled about how the parse is configured.</violation>
</file>
<file name="internal/generator/ctypename.go">
<violation number="1" location="internal/generator/ctypename.go:257">
P2: Fields using `volatile` or `restrict` lose those qualifiers during C type spelling, so the generated IR diverges from the written header as soon as one of these qualifiers is present. Preserving the supported qualifier keywords through both the base and pointer-level spelling paths would keep the replacement printer faithful.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
The parse stopped probing the host compiler in 9182a2f, which missed two spots. docs/API-COVERAGE.md told the reader the parser probes gcc 15.2.0 for its predefined macros and include paths. newCCConfig builds a cc.Config literal; the macros are the ccPredefined literal and the C library headers are the embedded sysinclude stubs. The version was wrong too, because ccPredefined claims GCC 15.3. The declaredHere comment justified the location filter with glibc's __mbstate_t and atomic_wide_counter. No glibc header reaches the parse any more, so the rule now cites a construct the hermetic configuration can still produce. Comments and prose only. The generated bindings and all three IR goldens are unchanged.
Contributor
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
Contributor
Author
|
@cubic-dev-ai Review again |
Contributor
@flexiondotorg I have started the AI code review. It will take a few minutes to complete. |
4 tasks
Contributor
There was a problem hiding this comment.
2 issues found across 49 files
Confidence score: 4/5
- In
internal/generator/comments.go, comment attribution for macro-generated declarations can pick the wrong source token when replacement-list identifiers overlap invocation arguments, which could produce misleading generated comments and reduce traceability/debuggability — track macro-parameter origin (or narrow the lookup scope) before attaching comments. - In
internal/generator/crosstarget_test.go, setup scaffolding is duplicated fromTestIRGoldensMatchFreshRun, which raises maintenance risk and can let test behavior drift across paths over time — extract a sharedsetup(t)helper for repo-root/include-dir/AVLibPath, log handling, andchdirmanagement.
Not reviewed (too large): internal/generator/testdata/ir/comments.txt (~15,920 lines), internal/generator/testdata/ir/structure.txt (~10,876 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/generator/crosstarget_test.go">
<violation number="1" location="internal/generator/crosstarget_test.go:123">
P3: The repo-root/include-dir/AVLibPath capture, log-discard, and chdir scaffold is copied nearly verbatim from TestIRGoldensMatchFreshRun. Extracting these into a shared helper (e.g. a setup(t) that returns repoRoot and the include dir, plus a saveLog/restoreLog helper) would keep the two cross-platform tests from drifting apart when the parse setup changes.</violation>
</file>
<file name="internal/generator/comments.go">
<violation number="1" location="internal/generator/comments.go:553">
P2: Macro-generated declarations can receive comments from the wrong source token when a replacement-list identifier also appears in the invocation arguments. Tracking macro-parameter origin, or restricting this lookup to identifiers substituted from parameters, would prevent caller comments from being assigned to replacement-list declarations.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
…rder spellingSite probed the invocation's argument text before the macro's replacement list, so a name the macro body declares took a caller position whenever an argument happened to spell the same identifier, and that declaration then claimed the caller's comment. Gate the argument probe on origin instead. A replacement-list token spelling a macro parameter name is a substitution, so the name did come from an argument; any other name the replacement list writes is literal macro text and resolves there. Reordering alone cannot do this, because the two cases pull opposite ways: AVBPrint.size needs the argument text even though the macro body writes "size" for its own parameter, while reserved_padding needs the replacement list. cc/v4 keeps no pre-substitution token position, but the macro definition carries both halves of the test, so no origin tracking is needed. The collision is unreachable in FF_PAD_STRUCTURE, the only macro this fires on. The sole name its body declares is reserved_padding, which no AVBPrint field can repeat without colliding with it. The gate is for the next such macro, and it makes the rule match the model its own comment claims to follow. All 14 tokens the rule places land where they did, so the five *.gen.go files and the three IR goldens are unchanged.
TestIRGoldensMatchFreshRun and TestGeneratorIRIsIdenticalAcrossTargets opened with the same repo-root resolve, header-absent skip, AVLibPath repoint and log discard, copied nearly verbatim. These two tests are what caught the host-dependence failures on darwin and linux/arm64, so they have to move together when the parse setup changes. setupHeaderParse now does all four steps and returns the repo root, and testRepoRoot factors out the resolve so TestGoldensCarryNoMachinePaths shares that piece too. Each test keeps its own golden read and its own t.Chdir. The two read their goldens into different forms, and only TestIRGoldensMatchFreshRun needs the temporary directory afterwards, to read back the dumps it writes there. Setup only; no assertion, skip condition or failure message changed.
Contributor
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
Contributor
Author
|
@cubic-dev-ai Review again please |
Contributor
@flexiondotorg I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
3 issues found across 49 files
Confidence score: 2/5
- In
flake.nix, Darwin environments can hitclang: command not foundbecause the shell no longer guarantees the compiler the builder selects, which can break local/dev CI builds on macOS — keep an LLVM clang in the shell inputs or align the shell with an LLVM stdenv. - In
internal/generator/type.go, function-type traversal aborts on valid inline anonymous struct params/returns due to missing field-scopedw.anonhandling, so regeneration can fail on legitimate APIs — restore anonymous-struct specifier recovery in function walks. - In
internal/generator/comments.go, trailing comments that start exactly at the declaration-name end are dropped, which can silently lose generated/commented metadata — treat the exclusive name end as a valid trailing-comment start.
Not reviewed (too large): internal/generator/testdata/ir/comments.txt (~15,920 lines), internal/generator/testdata/ir/structure.txt (~10,876 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/generator/type.go">
<violation number="1" location="internal/generator/type.go:363">
P2: Regeneration aborts on valid functions whose return type is an inline anonymous struct (and likewise on anonymous-struct parameters), because function type walks have no field-scoped `w.anon`. Recover the specifier from the declaration/type context or record this unsupported shape as a skip rather than panicking.</violation>
</file>
<file name="flake.nix">
<violation number="1" location="flake.nix:60">
P1: Darwin builds can fail with `clang: command not found` because the shell no longer provides the compiler that the builder unconditionally selects. Keeping an LLVM clang package (or switching the shell to an LLVM stdenv) alongside libcxx would make the flake self-contained instead of relying on a host Xcode installation.</violation>
</file>
<file name="internal/generator/comments.go">
<violation number="1" location="internal/generator/comments.go:493">
P2: A plain comment immediately following a declaration name is silently dropped, unlike the same comment with one intervening space. Treat the exclusive name end as a valid trailing-comment start.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
nameCol is one byte past the name, and a comment cannot begin inside a name, so a comment starting at exactly that column is the name's own trailing comment written without a separating space. The guard rejected that column, so the abutting form was dropped while the same comment one space further along was claimed. Reject only a column before the exclusive end, which is the impossible case. No FFmpeg header writes the abutting form today, so the record count in the comment golden is unchanged at 2907 and a test holds the rule instead.
A tagless struct written as a function's return type or parameter reaches unnamedStructType with no specifier on the walk, and the walk aborts. The comment there claimed a field declaration always supplies the specifier, which a probe disproves: C allows the shape and cc/v4 accepts it. The abort still stands. skipType returns nil, which the parser encodes as C void, so a skipped return type would emit a Go function that silently drops its result. A skipped parameter does reach the emitter as a loud "unhandled arg type" skip, but the return type does not, so aborting is the one answer that is safe in both positions. Correct the comment and add TestUnnamedStructWithoutSpecifierAborts, which pins the abort, the breadcrumb that names the symbol and the slot, and the tagless union that parses instead because parseUnionType expands it in place.
Contributor
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
flexiondotorg
deleted the
wimpy/ww-137-port-the-binding-generator-from-libclang-to-moderncorgccv4
branch
August 1, 2026 21:08
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The generator parsed FFmpeg headers through github.com/Newbluecake/bootstrap, an abandoned libclang fork pinned to clang 14, which broke twice on CXCursor_OMPArraySectionExpr, a cursor kind clang 18 removed. Parsing now runs on modernc.org/cc/v4, a pure Go C frontend, behind a new HeaderParser interface. Output is byte-identical:
just generateleaves all five*.gen.gofiles and all three IR goldens unchanged, with skipCeiling still at 245 and 245 skip markers. cc/v4 has no comment node and no absolute separator position, so comment association now lexes the raw header bytes and applies Clang's own adjacency rule, plus a rule that recovers positions for names written as macro arguments, whichFF_PAD_STRUCTUREneeds; association reaches all 2907 comments. CI now builds, lints and tests the generator on every platform, with the drift gate unconditional on all four legs.CGO_ENABLED=0 go build ./internal/generatornow succeeds; the emitted bindings still use cgo, so the module itself still needsCGO_ENABLED=1.Whether the two darwin CI legs go green is untested: cc.NewConfig probes the host compiler, so a darwin target cannot be exercised from this Linux host. gcc 13, 14 and 15 produce byte-identical output here, clang 21 fails outright on glibc headers, and
nix developsupplies a known-good gcc.Verified locally on linux/amd64 inside
nix develop:just lint(0 issues),just test(0 failures across seven packages),just generate(zero drift),nix flake check, andCGO_ENABLED=0 go build ./internal/generatorall pass.Closes #50. Two premises there do not hold: the generator was already built and run in
go-test.ymlbefore this change, with only the linter excluded, and cc/v4 alone does not make regeneration toolchain-free, sincecc.NewConfigstill probes$CC, thencc, thengcc.