Skip to content

Commit 1449ce2

Browse files
mpywclaude
andauthored
Fix recursive pointer unwrapping in carrier type matching (#19)
* fix: recursively unwrap all pointer layers in carrier type matching Fixes #18 ## Problem When SSA captures a pointer variable (*CarrierType) in a closure, it adds an extra level of indirection, resulting in **CarrierType in FreeVars. The previous single-layer unwrapPointer only handled one level: **CarrierType → *CarrierType (still doesn't match CarrierType) This caused false positives when carrier types were used via field access. ## Solution - Made typeutil.UnwrapPointer() recursive to handle arbitrary pointer depth - Removed duplicate unwrapPointer definitions in carrier and funcspec packages - All packages now use the centralized typeutil.UnwrapPointer() ## Changes - internal/typeutil: Made UnwrapPointer recursive and exported it - internal/directive/carrier: Use typeutil.UnwrapPointer, remove local version - internal/funcspec: Use typeutil.UnwrapPointer, remove local version 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * docs: update version reference to v0.7.3 --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 28e4ad8 commit 1449ce2

File tree

4 files changed

+29
-25
lines changed

4 files changed

+29
-25
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ go run github.com/mpyw/goroutinectx/cmd/goroutinectx@latest ./...
4040
```
4141

4242
> [!CAUTION]
43-
> To prevent supply chain attacks, pin to a specific version tag instead of `@latest` in CI/CD pipelines (e.g., `@v0.7.2`).
43+
> To prevent supply chain attacks, pin to a specific version tag instead of `@latest` in CI/CD pipelines (e.g., `@v0.7.3`).
4444
4545
### As a Library
4646

internal/directive/carrier/carrier.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ package carrier
44
import (
55
"go/types"
66
"strings"
7+
8+
"github.com/mpyw/goroutinectx/internal/typeutil"
79
)
810

911
// Carrier represents a type that can carry context.
@@ -15,7 +17,7 @@ type Carrier struct {
1517

1618
// Matches checks if the given type matches this carrier.
1719
func (c Carrier) Matches(t types.Type) bool {
18-
t = unwrapPointer(t)
20+
t = typeutil.UnwrapPointer(t)
1921

2022
named, ok := t.(*types.Named)
2123
if !ok {
@@ -30,14 +32,6 @@ func (c Carrier) Matches(t types.Type) bool {
3032
return matchPkg(obj.Pkg().Path(), c.PkgPath) && obj.Name() == c.TypeName
3133
}
3234

33-
// unwrapPointer returns the element type if t is a pointer.
34-
func unwrapPointer(t types.Type) types.Type {
35-
if ptr, ok := t.(*types.Pointer); ok {
36-
return ptr.Elem()
37-
}
38-
return t
39-
}
40-
4135
// matchPkg checks if pkgPath matches targetPkg, allowing version suffixes.
4236
func matchPkg(pkgPath, targetPkg string) bool {
4337
if pkgPath == targetPkg {

internal/funcspec/funcspec.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"unicode"
99

1010
"golang.org/x/tools/go/analysis"
11+
12+
"github.com/mpyw/goroutinectx/internal/typeutil"
1113
)
1214

1315
// Spec holds parsed components of a function specification.
@@ -80,7 +82,7 @@ func (s Spec) Matches(fn *types.Func) bool {
8082
return false
8183
}
8284

83-
recvType := unwrapPointer(recv.Type())
85+
recvType := typeutil.UnwrapPointer(recv.Type())
8486

8587
named, ok := recvType.(*types.Named)
8688
if !ok {
@@ -116,14 +118,6 @@ func ExtractFunc(pass *analysis.Pass, call *ast.CallExpr) *types.Func {
116118
return nil
117119
}
118120

119-
// unwrapPointer returns the element type if t is a pointer.
120-
func unwrapPointer(t types.Type) types.Type {
121-
if ptr, ok := t.(*types.Pointer); ok {
122-
return ptr.Elem()
123-
}
124-
return t
125-
}
126-
127121
// shortPkgName returns the last component of a package path.
128122
func shortPkgName(pkgPath string) string {
129123
if idx := strings.LastIndex(pkgPath, "/"); idx >= 0 {

internal/typeutil/typeutil.go

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const contextPkgPath = "context"
88

99
// IsContextType checks if the type is context.Context.
1010
func IsContextType(t types.Type) bool {
11-
t = unwrapPointer(t)
11+
t = UnwrapPointer(t)
1212

1313
named, ok := t.(*types.Named)
1414
if !ok {
@@ -23,10 +23,26 @@ func IsContextType(t types.Type) bool {
2323
return obj.Pkg().Path() == contextPkgPath && obj.Name() == "Context"
2424
}
2525

26-
// unwrapPointer returns the element type if t is a pointer, otherwise returns t.
27-
func unwrapPointer(t types.Type) types.Type {
28-
if ptr, ok := t.(*types.Pointer); ok {
29-
return ptr.Elem()
26+
// UnwrapPointer recursively unwraps all pointer layers.
27+
//
28+
// This is critical for SSA-based carrier type matching. When a closure captures
29+
// a pointer variable, SSA represents it with an additional level of indirection:
30+
//
31+
// func handler(ctx *CarrierType) {
32+
// go func() {
33+
// _ = ctx // SSA FreeVars: **CarrierType (not *CarrierType)
34+
// }()
35+
// }
36+
//
37+
// Therefore, we must unwrap ALL pointer layers to match against the registered
38+
// carrier type (CarrierType, no pointer). Single-layer unwrapping would leave
39+
// *CarrierType, which wouldn't match.
40+
func UnwrapPointer(t types.Type) types.Type {
41+
for {
42+
ptr, ok := t.(*types.Pointer)
43+
if !ok {
44+
return t
45+
}
46+
t = ptr.Elem()
3047
}
31-
return t
3248
}

0 commit comments

Comments
 (0)