c: ClosureData, GoClosure - #28
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR adds a symmetric closure round-trip pair — ClosureData[T] (heap-allocates a closure and returns an opaque Pointer for passing across the C boundary) and GoClosure[T] (reconstructs the closure from that Pointer). The core logic is correct, minimal, and follows the existing GoString/GoStringData naming convention in this file.
The only actionable items are documentation of the non-obvious unsafe contract. These are appropriate for a low-level llgo runtime binding but should be spelled out for callers, since the invariants (matching type parameter, GC lifetime, non-nil data) are invisible in the signatures and unchecked at runtime.
No correctness-blocking issues found.
|
|
||
| // ----------------------------------------------------------------------------- | ||
|
|
||
| func ClosureData[ClosureT any](closure ClosureT) Pointer { |
There was a problem hiding this comment.
[P2] Document ClosureData's unsafe contract (GC lifetime + exported doc comment)
ClosureData is an exported function with no doc comment, unlike neighbors such as GoString (line 180). Beyond golint/godoc convention, the lifetime contract is non-obvious: new(ClosureT) allocates a Go-managed object returned as a bare unsafe.Pointer. Once handed to C, the GC no longer sees a typed reference, so if C retains the pointer past the last Go reference the backing object can be collected or moved, leaving a dangling pointer. Recommend a doc comment stating (1) the purpose and (2) that the caller must keep the returned Pointer's backing allocation reachable (e.g. via runtime.KeepAlive/runtime.Pinner) for as long as C uses it.
| return Pointer(ret) | ||
| } | ||
|
|
||
| func GoClosure[ClosureT any](data Pointer) (closure ClosureT) { |
There was a problem hiding this comment.
[P2] Document GoClosure's unchecked type reinterpretation + nil precondition
GoClosure is exported with no doc comment and performs *(*ClosureT)(data), blindly reinterpreting data as *ClosureT. Because ClosureT is inferred independently at each call site and is invisible in the raw Pointer, a mismatch between the type used at ClosureData and here is silent memory corruption with no compiler/runtime guard. Additionally, a nil data (common for a C callback's opaque user-data slot) dereferences nil and crashes. Recommend a doc comment stating that data must originate from ClosureData with the same ClosureT and must be non-nil (and/or add an early nil guard).
1 participant
No description provided.