Repository navigation
cl: emit // llgo:type C for records with callback fields - #953
Conversation
A C/C++ struct/class whose members include function-pointer (callback) fields must carry the // llgo:type C directive so those callbacks use the C calling convention, consistent with how function-pointer typedefs are already handled in defineTypedef. featHasCallback is propagated up from toTypeEx while visiting the record members, so after the member-visit loop in initClassTypeEx we upgrade the doc-only comment to the directive form via directiveTypeC. Updates the affected golden test expectations (TestPython MemAllocatorEx, plus CursorAndRangeVisitor and IndexerCallbacks in the clang-c cases). Fixes #952
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: emit // llgo:type C for records with callback fields
The change is clean and well-targeted. A record (struct) that holds a function-pointer (callback) field now carries the // llgo:type C directive so those callbacks use the C calling convention, mirroring the existing defineTypedef handling. The explanatory comment is accurate and the generated test outputs under tool/_testc/ are formatted consistently with directiveComments (doc block + // separator + directive, or a bare directive when there is no doc).
Verified:
featHasCallbackis set intoTypeEx(cl/type.go:162,174) during member visiting, so the flag is genuinely only known at this point — the overwrite of the doc set at line 225 is correct.- The
featAllIgnoreearly-return at line 251 runs before the callback check, so a record that will be dropped never gets the directive. featHasCallbackpropagation is sound: anonymous nested records are emitted viaemitClass/emitUnionwith their ownfeats, so a nested callback does not spuriously flag the parent.
No blocking issues. One minor, optional observation inline.
Note: the macOS-gated codegen tests that regenerate the tool/_testc/ golden outputs could not be run in this Linux sandbox — worth confirming on CI.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #953 +/- ##
=======================================
Coverage 87.99% 88.00%
=======================================
Files 23 23
Lines 2665 2667 +2
=======================================
+ Hits 2345 2347 +2
Misses 320 320
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Requested by @xushiwei
Fixes #952.
A C/C++ struct/class whose members include callback (function-pointer) fields now carries the
// llgo:type Cdirective on its generated Go type, so those callbacks use the C calling convention — consistent with how function-pointertypedefs are already handled.What changed
cl/class.go: ininitClassTypeEx, after visiting the record members, iffeatHasCallbackwas set (propagated up fromtoTypeExwhen a field is a function pointer), upgrade the doc-only comment to the directive form via the existingdirectiveTypeChelper — reusing the same logic path asdefineTypedef.TestPython:MemAllocatorExintool/_testc/python-3.14.8/pythread.go.TestClang/TestSingleC:CursorAndRangeVisitorandIndexerCallbacksintool/_testc/clang-c-22.1.8/Index.goandIndex/out.go.Verification
go build ./cl/...andgo vet ./cl/pass.TestPython/TestClang/TestSingleCcompare against the clang C library + llgo runtime and are macOS-gated, so they cannot execute in this Linux sandbox (the link step fails onC.clang_*/llgo.*symbols, unrelated to this change). The golden files were updated to match the new generator output and should be confirmed on macOS/CI.