go/types: use correct receiver types for embedded interface methods

Interface methods don't declare a receiver (it's implicit), but after
type-checking the respective *types.Func objects are marked as methods
by having a receiver. For interface methods, the receiver base type used
to be the interface that declared the method in the first place, even if
the method also appeared in other interfaces via embedding. A change in
the computation of method sets for interfaces for Go1.10 changed that
inadvertently, with the consequence that sometimes a method's receiver
type ended up being an interface into which the method was embedded.
The exact behavior also depended on file type-checking order, and because
files are sometimes sorted by name, the behavior depended on file names.

This didn't matter for type-checking (the typechecker doesn't need the
receiver), but it matters for clients, and for printing of methods.

This change fixes interface method receivers at the end of type-checking
when we have all relevant information.

Fixes #28005.

Change-Id: I96c120fb0e517d7f8a14b8530f0273674569d5ea
Reviewed-on: https://go-review.googlesource.com/c/141358
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/go/types/issues_test.go b/src/go/types/issues_test.go
index 8560bb9b7dc8d2a0ec2df8d3700210c227975d94..f8810b6734bb89e1ab66ab8ab5c3757b9a8b150f 100644 (file)
--- a/src/go/types/issues_test.go
+++ b/src/go/types/issues_test.go
@@ -355,3 +355,70 @@ func TestIssue25627(t *testing.T) {
                })
        }
 }
+
+func TestIssue28005(t *testing.T) {
+       // method names must match defining interface name for this test
+       // (see last comment in this function)
+       sources := [...]string{
+               "package p; type A interface{ A() }",
+               "package p; type B interface{ B() }",
+               "package p; type X interface{ A; B }",
+       }
+
+       // compute original file ASTs
+       var orig [len(sources)]*ast.File
+       for i, src := range sources {
+               f, err := parser.ParseFile(fset, "", src, 0)
+               if err != nil {
+                       t.Fatal(err)
+               }
+               orig[i] = f
+       }
+
+       // run the test for all order permutations of the incoming files
+       for _, perm := range [][len(sources)]int{
+               {0, 1, 2},
+               {0, 2, 1},
+               {1, 0, 2},
+               {1, 2, 0},
+               {2, 0, 1},
+               {2, 1, 0},
+       } {
+               // create file order permutation
+               files := make([]*ast.File, len(sources))
+               for i := range perm {
+                       files[i] = orig[perm[i]]
+               }
+
+               // type-check package with given file order permutation
+               var conf Config
+               info := &Info{Defs: make(map[*ast.Ident]Object)}
+               _, err := conf.Check("", fset, files, info)
+               if err != nil {
+                       t.Fatal(err)
+               }
+
+               // look for interface object X
+               var obj Object
+               for name, def := range info.Defs {
+                       if name.Name == "X" {
+                               obj = def
+                               break
+                       }
+               }
+               if obj == nil {
+                       t.Fatal("interface not found")
+               }
+               iface := obj.Type().Underlying().(*Interface) // I must be an interface
+
+               // Each iface method m is embedded; and m's receiver base type name
+               // must match the method's name per the choice in the source file.
+               for i := 0; i < iface.NumMethods(); i++ {
+                       m := iface.Method(i)
+                       recvName := m.Type().(*Signature).Recv().Type().(*Named).Obj().Name()
+                       if recvName != m.Name() {
+                               t.Errorf("perm %v: got recv %s; want %s", perm, recvName, m.Name())
+                       }
+               }
+       }
+}

diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go
index 0f23345792f29c85d126f36c8b7b861a1a63b014..e6d61b7ec746649413eebe56015cb5a4b6be183e 100644 (file)
--- a/src/go/types/typexpr.go
+++ b/src/go/types/typexpr.go
@@ -549,6 +549,15 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
                recvTyp = def
        }
 
+       // Correct receiver type for all methods explicitly declared
+       // by this interface after we're done with type-checking at
+       // this level. See comment below for details.
+       check.later(func() {
+               for _, m := range ityp.methods {
+                       m.typ.(*Signature).recv.typ = recvTyp
+               }
+       })
+
        // collect methods
        var sigfix []*methodInfo
        for i, minfo := range info.methods {
@@ -562,8 +571,22 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
                        // its position, and because interface method
                        // signatures don't get a receiver via regular
                        // type-checking (there isn't a receiver in the
-                       // method's AST). Setting the correct receiver
-                       // type is also important for ptrRecv() (see methodset.go).
+                       // method's AST). Setting the receiver type is
+                       // also important for ptrRecv() (see methodset.go).
+                       //
+                       // Note: For embedded methods, the receiver type
+                       // should be the type of the interface that declared
+                       // the methods in the first place. Since we get the
+                       // methods here via methodInfo, which may be computed
+                       // before we have all relevant interface types, we use
+                       // the current interface's type (recvType). This may be
+                       // the type of the interface embedding the interface that
+                       // declared the methods. This doesn't matter for type-
+                       // checking (we only care about the receiver type for
+                       // the ptrRecv predicate, and it's never a pointer recv
+                       // for interfaces), but it matters for go/types clients
+                       // and for printing. We correct the receiver after type-
+                       // checking.
                        //
                        // TODO(gri) Consider marking methods signatures
                        // as incomplete, for better error messages. See