cl: toFuncType (testcase: _testmockc/type) - #708
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
Clean, well-scoped refactor. getPubName now returns (pubName, rewritten) instead of mutating through a pointer, toType/newParam/newVariadicParam take an explicit *types.Package, and new helpers toFuncType/toFuncParams/toFuncResults add function-pointer parameter support. The _testmockc/type fixture covers both the callback-parameter path and the void-return / no-arg path. Build passes.
No correctness or security issues introduced by this diff. Two minor notes are inline; a couple of non-blocking observations below.
Non-blocking observations
- Minor optional perf:
toFuncParams(cl/type_and_var.go) andnewParams(cl/compile.go) growparamsfrom anilslice viaappend, though the count is known up front.make([]*types.Var, 0, n)would avoid reallocations. Trivial; compile-time path only. - Out of scope (pre-existing):
cPubName(cl/blockctx.go:109) andmanglingName[1:](cl/compile.go:192) index position 0/1 without a length check and would panic on an empty name. Not touched by this PR, flagged only for awareness.
| for i := range n { | ||
| item := fn.ArgType(c.Uint(i)) | ||
| tyParam := toType(ctx, pkg, item, flagIsParam) | ||
| nameParam := "_llcppg_param" + strconv.Itoa(int(i)+1) |
There was a problem hiding this comment.
toFuncParams unconditionally names each parameter _llcppg_param<N> and never consults the declared C argument name or applies avoidKeyword. This diverges from newParam (cl/compile.go:217-230), which adopts the real C name when present. For a function-pointer prototype that carries argument names (e.g. int (*cmp)(const void* lhs, const void* rhs)), those names are silently dropped. If dropping them is intentional (proto types often have none), a short comment would help; otherwise consider sharing naming logic with newParam.
| pointee := toType(ctx, typ.PointeeType(), flags) | ||
| elem := typ.PointeeType() | ||
| if elem.Kind == lc.TypeFunctionProto { | ||
| return toFuncType(ctx, pkg, elem) |
There was a problem hiding this comment.
The pointer branch forwards flags recursively (line 78), but the function-proto branch drops flags when calling toFuncType, and toFuncParams then hardcodes flagIsParam. Harmless today since flags is unused in the reachable branches, but the asymmetry could surprise if downstream logic ever keys off flagRetType/flagIsStructField inside a function pointer.
1 participant
No description provided.