Repository navigation
fix(cl): support C-prefixed variables and migrate native bindings - #2882
Merged
Merged
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review: Explicit C variable bindings
Clean, well-scoped change. It adds //go:linkname Name C.symbol support for variables in cl/import.go (symmetric with the existing function-side C. handling at import.go:882), migrates all bare-symbol C linknames across runtime/, _demo/, cl/_testlibc/, and test/cgo/ to the explicit C. form, and adds solid tests plus README docs.
Verified during review (no action needed):
- The new
C.branch is placed first invarName(), matching the function-side ordering;C.-prefixed targets are unambiguous since C identifiers cannot contain.. - Migration is complete — the only remaining bare two-token linkname (
cl/_testdata/llgointrinsics/in.go:83) is an intentional self-trampoline, not a C binding. - README block is embedme-safe (first line has spaces, no
embedmereference comment), and the documented claims — native storage/initializer, no Go-side initializer, function-pointer support, andllgo:linkparity — all match the implementation and tests. Both directives share the sameSetLinkname→varName()path, soC.works forllgo:linktoo. cl/_testrt/cvar/in.goLITTEST target matrix is valid: the fixture is IR-check-only with portableexternal globalassertions and noimport "C", so cross-target coverage needs no C toolchain.- No performance or security concerns. The
_democompiler-rt builtins moving fromgoFunctocFunclowering is an ABI-correctness improvement, not a regression.
Non-blocking note (not introduced by this PR): A malformed directive //go:linkname Name C. (empty symbol after the prefix) is accepted silently — varName() returns v[2:] == "" (cl/import.go:940), and the function path has the same gap (import.go:883). It surfaces only as an opaque link-time failure rather than a positioned compiler diagnostic. If you want to harden it later, reject an empty target at directive-collection time, mirroring the existing //llgo:link requires ... a target error.
One minor inline readability suggestion below. No correctness defects found.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
This was referenced Oct 20, 2026
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
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.
Implement the native-variable migration prerequisite for #2879 (clarified storage rules); also relates to #2878.
Strip
C.and use native storage representation for variable bindings through bothgo:linknameandllgo:link. Migrate 85 native bindings in the runtime, demos, and fixtures. Ordinary Go variable storage/merging follows in #2894; native function entries and placeholder rules follow in #2895. This PR preserves existing bare-target behavior during migration.Add C-backed tests for scalar, array, pointer, and function-pointer globals: native initializers, address identity, Go/C writes, directive aliases, and nil/restored callbacks. Extend aggregate-variable IR checks to Linux and Windows. Fix dependency setup for nested checkouts and allow Windows MSVC cache cleanup to finish within the job timeout.
Validation: native cgo tests, cross-target IR fixtures, WASI smoke, installer tests, and CI workflow checks pass. Latest CI: 115 successful checks, one skipped.
Companion: goplus/lib#53 migrates 14 native variables and must follow this compiler support.
llcppgalready emitsC.targets.