Skip to content

Commit 3e7c812

Browse files
adonovanfindleyr
authored andcommitted
internal/typeparams: work around LookupFieldOrMethod inconsistency
This change adds to x/tools a workaround for a bug in go/types that causes LookupFieldOrMethod and NewTypeSet to be inconsistent wrt an ill-typed method (*T).f where T itself is a pointer. The workaround is that, if Lookup fails, we walk the MethodSet. Updates golang/go#60634 Fixes golang/go#60628 Change-Id: I87caa2ae077e5cdfa40b65a2f52e261384c91167 Reviewed-on: https://go-review.googlesource.com/c/tools/+/501197 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Robert Findley <[email protected]> Run-TryBot: Alan Donovan <[email protected]> gopls-CI: kokoro <[email protected]>
1 parent f948552 commit 3e7c812

File tree

2 files changed

+70
-0
lines changed

2 files changed

+70
-0
lines changed

common.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,26 @@ func OriginMethod(fn *types.Func) *types.Func {
105105
}
106106
orig := NamedTypeOrigin(named)
107107
gfn, _, _ := types.LookupFieldOrMethod(orig, true, fn.Pkg(), fn.Name())
108+
109+
// This is a fix for a gopls crash (#60628) due to a go/types bug (#60634). In:
110+
// package p
111+
// type T *int
112+
// func (*T) f() {}
113+
// LookupFieldOrMethod(T, true, p, f)=nil, but NewMethodSet(*T)={(*T).f}.
114+
// Here we make them consistent by force.
115+
// (The go/types bug is general, but this workaround is reached only
116+
// for generic T thanks to the early return above.)
117+
if gfn == nil {
118+
mset := types.NewMethodSet(types.NewPointer(orig))
119+
for i := 0; i < mset.Len(); i++ {
120+
m := mset.At(i)
121+
if m.Obj().Id() == fn.Id() {
122+
gfn = m.Obj()
123+
break
124+
}
125+
}
126+
}
127+
108128
return gfn.(*types.Func)
109129
}
110130

common_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,12 @@ func TestOriginMethodUses(t *testing.T) {
140140
t.Fatal(err)
141141
}
142142

143+
// Look up func T.m.
143144
T := pkg.Scope().Lookup("T").Type()
144145
obj, _, _ := types.LookupFieldOrMethod(T, true, pkg, "m")
145146
m := obj.(*types.Func)
146147

148+
// Assert that the origin of each t.m() call is p.T.m.
147149
ast.Inspect(f, func(n ast.Node) bool {
148150
if call, ok := n.(*ast.CallExpr); ok {
149151
sel := call.Fun.(*ast.SelectorExpr)
@@ -158,6 +160,54 @@ func TestOriginMethodUses(t *testing.T) {
158160
}
159161
}
160162

163+
// Issue #60628 was a crash in gopls caused by inconsistency (#60634) between
164+
// LookupFieldOrMethod and NewFileSet for methods with an illegal
165+
// *T receiver type, where T itself is a pointer.
166+
// This is a regression test for the workaround in OriginMethod.
167+
func TestOriginMethod60628(t *testing.T) {
168+
const src = `package p; type T[P any] *int; func (r *T[A]) f() {}`
169+
fset := token.NewFileSet()
170+
f, err := parser.ParseFile(fset, "p.go", src, 0)
171+
if err != nil {
172+
t.Fatal(err)
173+
}
174+
175+
// Expect type error: "invalid receiver type T[A] (pointer or interface type)".
176+
info := types.Info{
177+
Uses: make(map[*ast.Ident]types.Object),
178+
}
179+
var conf types.Config
180+
pkg, _ := conf.Check("p", fset, []*ast.File{f}, &info) // error expected
181+
if pkg == nil {
182+
t.Fatal("no package")
183+
}
184+
185+
// Look up methodset of *T.
186+
T := pkg.Scope().Lookup("T").Type()
187+
mset := types.NewMethodSet(types.NewPointer(T))
188+
if mset.Len() == 0 {
189+
t.Errorf("NewMethodSet(*T) is empty")
190+
}
191+
for i := 0; i < mset.Len(); i++ {
192+
sel := mset.At(i)
193+
m := sel.Obj().(*types.Func)
194+
195+
// TODO(adonovan): check the consistency property required to fix #60634.
196+
if false {
197+
m2, _, _ := types.LookupFieldOrMethod(T, true, m.Pkg(), m.Name())
198+
if m2 != m {
199+
t.Errorf("LookupFieldOrMethod(%v, indirect=true, %v) = %v, want %v",
200+
T, m, m2, m)
201+
}
202+
}
203+
204+
// Check the workaround.
205+
if OriginMethod(m) == nil {
206+
t.Errorf("OriginMethod(%v) = nil", m)
207+
}
208+
}
209+
}
210+
161211
func TestGenericAssignableTo(t *testing.T) {
162212
testenv.NeedsGo1Point(t, 18)
163213

0 commit comments

Comments
 (0)