cl: newPointer - #703
Conversation
There was a problem hiding this comment.
Review: cl: newPointer
This PR implements toType/newPointer type conversion, handles void return types in compileFunc, threads a c gogen.PkgRef through blockCtx, and adds a cl/_mod test module. The structure is clean and the void-return handling reads correctly.
Two blocking correctness issues share a root cause: the c package reference is wired to the wrong import path and the wrong symbol name, which together break Int/UInt type resolution. Details inline.
Non-blocking notes:
toType(cl/type_and_var.go:66,68) callsctx.c.Ref(...)per type node; sinceInt/UIntresolve to the sametypes.Typefor the whole compilation, these could be resolved once atblockCtxconstruction rather than re-looked-up per occurrence. Optional, low impact.newPointer'styVoidbranch (cl/type_and_var.go:43-48) is currently unreachable:toTypehas nolc.TypeVoidcase, so avoid*pointee hitspanic("todo: toType ...")beforenewPointerever seestyVoid. Expected given the WIP status, but worth flagging as intended functionality that is silently inert.
No security concerns found. This is a compile-time codegen tool consuming trusted headers.
| // ----------------------------------------------------------------------------- | ||
|
|
||
| func loadFile(p *gogen.Package, conf *Config, file Source) (pi *PkgInfo, err error) { | ||
| c := p.Import("github.com/lib/c") |
There was a problem hiding this comment.
[P0] Wrong import path breaks c package resolution
This imports github.com/lib/c, but the package is github.com/goplus/lib/c everywhere else — the file's own import at compile.go:27, the test stub cl/_mod/stub.go:3, and cl/_mod/go.mod (require github.com/goplus/lib v0.5.2).
This PkgRef is stored in blockCtx.c and used by toType to resolve Int/UInt, so the mismatched path prevents type resolution from working.
c := p.Import("github.com/goplus/lib/c")| case lc.TypeInt: | ||
| return ctx.c.Ref("Int").Type() | ||
| case lc.TypeUInt: | ||
| return ctx.c.Ref("UInt").Type() |
There was a problem hiding this comment.
[P0] Ref("UInt") does not exist; symbol is Uint
github.com/goplus/lib/c v0.5.2 defines the unsigned-int alias as Uint (Uint = uint32 in c/c.go:38), not UInt. ctx.c.Ref("UInt") will fail to resolve the object.
return ctx.c.Ref("Uint").Type()The adjacent Ref("Int") on line 66 is correct since Int exists.
| ) | ||
|
|
||
| var ( | ||
| tyVoid = types.Typ[types.UntypedNil] |
There was a problem hiding this comment.
[P3] tyVoid bound to UntypedNil is surprising
tyVoid is bound to types.Typ[types.UntypedNil] and compared in newPointer to emit unsafe.Pointer. This works, but the name suggests a distinct void type rather than the untyped-nil sentinel. A short comment explaining that untyped-nil is used as the void sentinel would prevent future confusion. Minor / WIP, non-blocking.
No description provided.