]> Cypherpunks repositories - gostls13.git/commitdiff
go/types: support type parameters in NewMethodSet
authorRob Findley <rfindley@google.com>
Mon, 19 Apr 2021 21:18:54 +0000 (17:18 -0400)
committerRobert Findley <rfindley@google.com>
Tue, 20 Apr 2021 15:13:47 +0000 (15:13 +0000)
Add handling for TypeParams in NewMethodSet, to bring it in sync with
lookupFieldOrMethod. Also add a test, since we had none. I wanted this
fix to get gopls completion working with type params, but due to the
subtlety of lookupFieldOrMethod, I left a TODO to confirm that there are
no behavioral differences between the APIs.

Updates #45639

Change-Id: I16723e16d4d944ca4ecb4d87fc196815abb6fcff
Reviewed-on: https://go-review.googlesource.com/c/go/+/311455
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
src/go/types/api_test.go
src/go/types/lookup.go
src/go/types/methodset.go
src/go/types/methodset_test.go [new file with mode: 0644]

index a8e29d3fdab7a434aad2958ec1ec95ad3390e4a3..6998fc0a0d57360d62651d5eab00f08dfa3bf1ac 100644 (file)
@@ -21,6 +21,11 @@ import (
        . "go/types"
 )
 
+// pkgFor parses and type checks the package specified by path and source,
+// populating info if provided.
+//
+// If source begins with "package generic_" and type parameters are enabled,
+// generic code is permitted.
 func pkgFor(path, source string, info *Info) (*Package, error) {
        fset := token.NewFileSet()
        mode := modeForSource(source)
@@ -1213,6 +1218,8 @@ func TestLookupFieldOrMethod(t *testing.T) {
        // Test cases assume a lookup of the form a.f or x.f, where a stands for an
        // addressable value, and x for a non-addressable value (even though a variable
        // for ease of test case writing).
+       //
+       // Should be kept in sync with TestMethodSet.
        var tests = []struct {
                src      string
                found    bool
index 3691b1ecaa53f98215513d2aaf4cb1440a2e2dd3..9c7bfd4bb97da8db276c633874c8dda8186dffda 100644 (file)
@@ -142,6 +142,8 @@ func (check *Checker) rawLookupFieldOrMethod(T Type, addressable bool, pkg *Pack
 
                                // continue with underlying type, but only if it's not a type parameter
                                // TODO(gri) is this what we want to do for type parameters? (spec question)
+                               // TODO(#45639) the error message produced as a result of skipping an
+                               //              underlying type parameter should be improved.
                                typ = named.under()
                                if asTypeParam(typ) != nil {
                                        continue
index c44009f1a5e3a1ae2159cf54e34f9dd82297f3f6..ae8011a2eeef92f8246631d13cf24d1cc5780504 100644 (file)
@@ -63,7 +63,7 @@ func (s *MethodSet) Lookup(pkg *Package, name string) *Selection {
 var emptyMethodSet MethodSet
 
 // Note: NewMethodSet is intended for external use only as it
-//       requires interfaces to be complete. If may be used
+//       requires interfaces to be complete. It may be used
 //       internally if LookupFieldOrMethod completed the same
 //       interfaces beforehand.
 
@@ -73,8 +73,8 @@ func NewMethodSet(T Type) *MethodSet {
        // WARNING: The code in this function is extremely subtle - do not modify casually!
        //          This function and lookupFieldOrMethod should be kept in sync.
 
-       // TODO(gri) This code is out-of-sync with the lookup code at this point.
-       //           Need to update.
+       // TODO(rfindley) confirm that this code is in sync with lookupFieldOrMethod
+       //                with respect to type params.
 
        // method set up to the current depth, allocated lazily
        var base methodSet
@@ -127,8 +127,12 @@ func NewMethodSet(T Type) *MethodSet {
 
                                mset = mset.add(named.methods, e.index, e.indirect, e.multiples)
 
-                               // continue with underlying type
+                               // continue with underlying type, but only if it's not a type parameter
+                               // TODO(rFindley): should this use named.under()? Can there be a difference?
                                typ = named.underlying
+                               if _, ok := typ.(*_TypeParam); ok {
+                                       continue
+                               }
                        }
 
                        switch t := typ.(type) {
@@ -154,6 +158,9 @@ func NewMethodSet(T Type) *MethodSet {
 
                        case *Interface:
                                mset = mset.add(t.allMethods, e.index, true, e.multiples)
+
+                       case *_TypeParam:
+                               mset = mset.add(t.Bound().allMethods, e.index, true, e.multiples)
                        }
                }
 
diff --git a/src/go/types/methodset_test.go b/src/go/types/methodset_test.go
new file mode 100644 (file)
index 0000000..4a373fa
--- /dev/null
@@ -0,0 +1,109 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package types_test
+
+import (
+       "testing"
+
+       "go/internal/typeparams"
+       . "go/types"
+)
+
+func TestNewMethodSet(t *testing.T) {
+       type method struct {
+               name     string
+               index    []int
+               indirect bool
+       }
+
+       // Tests are expressed src -> methods, for simplifying the composite literal.
+       // Should be kept in sync with TestLookupFieldOrMethod.
+       tests := map[string][]method{
+               // Named types
+               "var a T; type T struct{}; func (T) f() {}":   {{"f", []int{0}, false}},
+               "var a *T; type T struct{}; func (T) f() {}":  {{"f", []int{0}, true}},
+               "var a T; type T struct{}; func (*T) f() {}":  {},
+               "var a *T; type T struct{}; func (*T) f() {}": {{"f", []int{0}, true}},
+
+               // Interfaces
+               "var a T; type T interface{ f() }":                           {{"f", []int{0}, true}},
+               "var a T1; type ( T1 T2; T2 interface{ f() } )":              {{"f", []int{0}, true}},
+               "var a T1; type ( T1 interface{ T2 }; T2 interface{ f() } )": {{"f", []int{0}, true}},
+
+               // Embedding
+               "var a struct{ E }; type E interface{ f() }":            {{"f", []int{0, 0}, true}},
+               "var a *struct{ E }; type E interface{ f() }":           {{"f", []int{0, 0}, true}},
+               "var a struct{ E }; type E struct{}; func (E) f() {}":   {{"f", []int{0, 0}, false}},
+               "var a struct{ *E }; type E struct{}; func (E) f() {}":  {{"f", []int{0, 0}, true}},
+               "var a struct{ E }; type E struct{}; func (*E) f() {}":  {},
+               "var a struct{ *E }; type E struct{}; func (*E) f() {}": {{"f", []int{0, 0}, true}},
+
+               // collisions
+               "var a struct{ E1; *E2 }; type ( E1 interface{ f() }; E2 struct{ f int })":            {},
+               "var a struct{ E1; *E2 }; type ( E1 struct{ f int }; E2 struct{} ); func (E2) f() {}": {},
+       }
+
+       genericTests := map[string][]method{
+               // By convention, look up a in the scope of "g"
+               "type C interface{ f() }; func g[T C](a T){}":                       {{"f", []int{0}, true}},
+               "type C interface{ f() }; func g[T C]() { var a T; _ = a }":         {{"f", []int{0}, true}},
+               "type C interface{ f() }; func g[T C]() { var a struct{T}; _ = a }": {{"f", []int{0, 0}, true}},
+
+               // Issue #45639.
+               "type C interface{ f() }; func g[T C]() { type Y T; var a Y; _ = a }": {},
+       }
+
+       check := func(src string, methods []method, generic bool) {
+               pkgName := "p"
+               if generic {
+                       // The generic_ prefix causes pkgFor to allow generic code.
+                       pkgName = "generic_p"
+               }
+               pkg, err := pkgFor("test", "package "+pkgName+";"+src, nil)
+               if err != nil {
+                       t.Errorf("%s: incorrect test case: %s", src, err)
+                       return
+               }
+
+               scope := pkg.Scope()
+               if generic {
+                       fn := pkg.Scope().Lookup("g").(*Func)
+                       scope = fn.Scope()
+               }
+               obj := scope.Lookup("a")
+               if obj == nil {
+                       t.Errorf("%s: incorrect test case - no object a", src)
+                       return
+               }
+
+               ms := NewMethodSet(obj.Type())
+               if got, want := ms.Len(), len(methods); got != want {
+                       t.Errorf("%s: got %d methods, want %d", src, got, want)
+                       return
+               }
+               for i, m := range methods {
+                       sel := ms.At(i)
+                       if got, want := sel.Obj().Name(), m.name; got != want {
+                               t.Errorf("%s [method %d]: got name = %q at, want %q", src, i, got, want)
+                       }
+                       if got, want := sel.Index(), m.index; !sameSlice(got, want) {
+                               t.Errorf("%s [method %d]: got index = %v, want %v", src, i, got, want)
+                       }
+                       if got, want := sel.Indirect(), m.indirect; got != want {
+                               t.Errorf("%s [method %d]: got indirect = %v, want %v", src, i, got, want)
+                       }
+               }
+       }
+
+       for src, methods := range tests {
+               check(src, methods, false)
+       }
+
+       if typeparams.Enabled {
+               for src, methods := range genericTests {
+                       check(src, methods, true)
+               }
+       }
+}