Skip to content

cl newPointer: rm types.Signature ptr - #709

Merged
xushiwei merged 1 commit into
goplus:devfrom
xushiwei:q
Sep 13, 2026
Merged

cl newPointer: rm types.Signature ptr#709
xushiwei merged 1 commit into
goplus:devfrom
xushiwei:q

Conversation

@xushiwei

Copy link
Copy Markdown
Member

No description provided.

@xushiwei
xushiwei merged commit 4e9e49a into goplus:dev Sep 13, 2026
2 checks passed

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: function proto / function pointer type support

The change is correct and well-targeted. It adds a top-level case lc.TypeFunctionProto in toType (handles a decayed bare function-type parameter such as void f(int (callback)(void))) and removes the *types.Signature panic branch in newPointer so a pointer-to-function-pointer lowers to *func().

Traced against the fixtures, all three shapes are handled consistently:

  • int (*cmp)(...) → bare func(...) c.Int (single pointer elided at toType:70-72)
  • int (callback)(void) → bare func() c.Int (new TypeFunctionProto case)
  • void (**callbackPtr)(void)*func() (recurses; inner func() wrapped by newPointer, which now falls through instead of panicking)

The _testmockc/type in.h/out.go fixtures match this behavior and provide adequate coverage.

Findings: No correctness, performance, or security issues. One optional maintainability note inline.

Note: go test ./cl -run TestMockC could not be executed in the review sandbox (missing llgo/libclang linkage — relocation target C.clang_createIndex not defined); fixture correctness was verified by manual trace-through rather than a live test run.

Additional findings

  • cl/type_and_var.go:72: [P3] Consider a comment on the intentional pointer-elision asymmetry: A single-level function pointer (int (*cmp)(...)) is intentionally lowered to a bare func(...) here, while a double pointer (void (**)(void)) becomes *func(). This one-level elision is the central behavior of the PR but is not documented in code. A brief comment on this branch would help future maintainers avoid "fixing" it into *func(...). Optional / non-blocking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant