From 99843e22e81045ba5e6776095a2cb3ef5704c70e Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 22 Feb 2018 10:43:35 -0800 Subject: [PATCH] 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 --- src/go/types/interfaces.go | 21 ++++++++++++--------- src/go/types/testdata/importdecl1a.src | 11 +++++++++++ src/go/types/testdata/importdecl1b.src | 4 ++++ src/go/types/typexpr.go | 13 +++++++++---- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/go/types/interfaces.go b/src/go/types/interfaces.go index 33f6524b162ac..66669ce36bed6 100644 --- 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 8301820dda981..d377c01638a53 100644 --- 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 f24bb9ade9777..ee70bbd8e73f7 100644 --- 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 e86834efdd24b..1a82b613cb1a0 100644 --- 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