From 68ad1d60a27f7a1fff2ed34e2c3476c77bcd3a6d Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 10 Oct 2018 17:19:29 -0700 Subject: [PATCH] [release-branch.go1.11] 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 #28249 Updates #28005 Change-Id: I96c120fb0e517d7f8a14b8530f0273674569d5ea Reviewed-on: https://go-review.googlesource.com/c/141358 Reviewed-by: Alan Donovan Reviewed-on: https://go-review.googlesource.com/c/146660 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Robert Griesemer --- src/go/types/issues_test.go | 67 +++++++++++++++++++++++++++++++++++++ src/go/types/typexpr.go | 33 ++++++++++++++++-- 2 files changed, 97 insertions(+), 3 deletions(-) diff --git a/src/go/types/issues_test.go b/src/go/types/issues_test.go index 8560bb9b7d..f8810b6734 100644 --- 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 45ada5874b..9a03184758 100644 --- a/src/go/types/typexpr.go +++ b/src/go/types/typexpr.go @@ -571,6 +571,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 { @@ -580,9 +589,27 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d pos := name.Pos() // Don't type-check signature yet - use an // empty signature now and update it later. - // Since we know the receiver, set it up now - // (required to avoid crash in ptrRecv; see - // e.g. test case for issue 6638). + // But set up receiver since we know it and + // 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 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 // also the T4 and T5 tests in testdata/cycles2.src. -- 2.48.1