go/types: type-check embedded methods in correct scope (regression)

Change https://go-review.googlesource.com/79575 fixed the computation
of recursive method sets by separating the method set computation from
type computation. However, it didn't track an embedded method's scope
and as a result, some methods' signatures were typed in the wrong
context.

This change tracks embedded methods together with their scope and
uses that scope for the correct context setup when typing those
method signatures.

Fixes #23914.

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

diff --git a/src/go/types/interfaces.go b/src/go/types/interfaces.go
index 33f6524b162ac65c254c5610bc999a1d1c8f5ad4..66669ce36bed6c5c19acbd59179e6cde34822101 100644 (file)
--- a/src/go/types/interfaces.go
+++ b/src/go/types/interfaces.go
@@ -53,12 +53,14 @@ func (info *ifaceInfo) String() string {
 
 // methodInfo represents an interface method.
 // At least one of src or fun must be non-nil.
-// (Methods declared in the current package have a non-nil src,
-// and eventually a non-nil fun field; imported and predeclared
-// methods have a nil src, and only a non-nil fun field.)
+// (Methods declared in the current package have a non-nil scope
+// and src, and eventually a non-nil fun field; imported and pre-
+// declared methods have a nil scope and src, and only a non-nil
+// fun field.)
 type methodInfo struct {
-       src *ast.Field // syntax tree representation of interface method; or nil
-       fun *Func      // corresponding fully type-checked method type; or nil
+       scope *Scope     // scope of interface method; or nil
+       src   *ast.Field // syntax tree representation of interface method; or nil
+       fun   *Func      // corresponding fully type-checked method type; or nil
 }
 
 func (info *methodInfo) String() string {
@@ -124,7 +126,8 @@ func (check *Checker) reportAltMethod(m *methodInfo) {
        }
 }
 
-// infoFromTypeLit computes the method set for the given interface iface.
+// infoFromTypeLit computes the method set for the given interface iface
+// declared in scope.
 // If a corresponding type name exists (tname != nil), it is used for
 // cycle detection and to cache the method set.
 // The result is the method set, or nil if there is a cycle via embedded
@@ -132,7 +135,7 @@ func (check *Checker) reportAltMethod(m *methodInfo) {
 // but they were either reported (e.g., blank methods), or will be found
 // (again) when computing the interface's type.
 // If tname is not nil it must be the last element in path.
-func (check *Checker) infoFromTypeLit(iface *ast.InterfaceType, tname *TypeName, path []*TypeName) (info *ifaceInfo) {
+func (check *Checker) infoFromTypeLit(scope *Scope, iface *ast.InterfaceType, tname *TypeName, path []*TypeName) (info *ifaceInfo) {
        assert(iface != nil)
 
        // lazy-allocate interfaces map
@@ -207,7 +210,7 @@ func (check *Checker) infoFromTypeLit(iface *ast.InterfaceType, tname *TypeName,
                                        continue // ignore
                                }
 
-                               m := &methodInfo{src: f}
+                               m := &methodInfo{scope: scope, src: f}
                                if check.declareInMethodSet(&mset, f.Pos(), m) {
                                        info.methods = append(info.methods, m)
                                }
@@ -333,7 +336,7 @@ typenameLoop:
                        return check.infoFromQualifiedTypeName(typ)
                case *ast.InterfaceType:
                        // type tname interface{...}
-                       return check.infoFromTypeLit(typ, tname, path)
+                       return check.infoFromTypeLit(decl.file, typ, tname, path)
                }
                // type tname X // and X is not an interface type
                return nil

diff --git a/src/go/types/testdata/importdecl1a.src b/src/go/types/testdata/importdecl1a.src
index 8301820dda981b6ee4faf0afa53ed0e5e6085069..d377c01638a534a111d1c93833c2b05cc3627330 100644 (file)
--- a/src/go/types/testdata/importdecl1a.src
+++ b/src/go/types/testdata/importdecl1a.src
@@ -6,6 +6,17 @@
 
 package importdecl1
 
+import "go/ast"
 import . "unsafe"
 
 var _ Pointer // use dot-imported package unsafe
+
+// Test cases for issue 23914.
+
+type A interface {
+       // Methods m1, m2 must be type-checked in this file scope
+       // even when embedded in an interface in a different
+       // file of the same package.
+       m1() ast.Node
+       m2() Pointer
+}

diff --git a/src/go/types/testdata/importdecl1b.src b/src/go/types/testdata/importdecl1b.src
index f24bb9ade977749ce379a08594cf44194dff5b78..ee70bbd8e73f787c8aa3b3d792f5673d0f0aacbb 100644 (file)
--- a/src/go/types/testdata/importdecl1b.src
+++ b/src/go/types/testdata/importdecl1b.src
@@ -5,3 +5,7 @@
 package importdecl1
 
 import . /* ERROR "imported but not used" */ "unsafe"
+
+type B interface {
+       A
+}

diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go
index e86834efdd24b585858ef185a90151f74349bc24..1a82b613cb1a0e5cc42f2e869246e795c6559652 100644 (file)
--- a/src/go/types/typexpr.go
+++ b/src/go/types/typexpr.go
@@ -481,9 +481,9 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
        // collect embedded interfaces
        // Only needed for printing and API. Delay collection
        // to end of type-checking when all types are complete.
-       interfaceScope := check.scope // capture for use in closure below
+       interfaceContext := check.context // capture for use in closure below
        check.later(func() {
-               check.scope = interfaceScope
+               check.context = interfaceContext
                if trace {
                        check.trace(iface.Pos(), "-- delayed checking embedded interfaces of %s", iface)
                        check.indent++
@@ -495,7 +495,7 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
                        if len(f.Names) == 0 {
                                typ := check.typ(f.Type)
                                // typ should be a named type denoting an interface
-                               // (the parser will make sure it's a name type but
+                               // (the parser will make sure it's a named type but
                                // constructed ASTs may be wrong).
                                if typ == Typ[Invalid] {
                                        continue // error reported before
@@ -531,7 +531,7 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
        if def != nil {
                tname = def.obj
        }
-       info := check.infoFromTypeLit(iface, tname, path)
+       info := check.infoFromTypeLit(check.scope, iface, tname, path)
        if info == nil || info == &emptyIfaceInfo {
                // error or empty interface - exit early
                ityp.allMethods = markComplete
@@ -574,7 +574,11 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
        }
 
        // fix signatures now that we have collected all methods
+       savedContext := check.context
        for _, minfo := range sigfix {
+               // (possibly embedded) methods must be type-checked within their scope and
+               // type-checking them must not affect the current context (was issue #23914)
+               check.context = context{scope: minfo.scope}
                typ := check.typ(minfo.src.Type)
                sig, _ := typ.(*Signature)
                if sig == nil {
@@ -588,6 +592,7 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
                sig.recv = old.recv
                *old = *sig // update signature (don't replace pointer!)
        }
+       check.context = savedContext
 
        // sort to match NewInterface
        // TODO(gri) we may be able to switch to source order