diff --git a/src/go/types/check.go b/src/go/types/check.go index 26db5769b9..6234d5a0b5 100644 --- a/src/go/types/check.go +++ b/src/go/types/check.go @@ -85,11 +85,12 @@ type Checker struct { files []*ast.File // package files unusedDotImports map[*Scope]map[*Package]token.Pos // positions of unused dot-imported packages for each file scope - firstErr error // first error encountered - methods map[string][]*Func // maps type names to associated methods - untyped map[ast.Expr]exprInfo // map of expressions without final type - funcs []funcInfo // list of functions to type-check - delayed []func() // delayed checks requiring fully setup types + firstErr error // first error encountered + methods map[string][]*Func // maps package scope type names to associated non-blank, non-interface methods + interfaces map[*TypeName]*ifaceInfo // maps interface type names to corresponding interface infos + untyped map[ast.Expr]exprInfo // map of expressions without final type + funcs []funcInfo // list of functions to type-check + delayed []func() // delayed checks requiring fully setup types // context within which the current object is type-checked // (valid only for the duration of type-checking a specific object) @@ -186,6 +187,7 @@ func (check *Checker) initFiles(files []*ast.File) { check.firstErr = nil check.methods = nil + check.interfaces = nil check.untyped = nil check.funcs = nil check.delayed = nil @@ -236,19 +238,21 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) { check.collectObjects() - check.packageObjects(check.resolveOrder()) + check.packageObjects() check.functionBodies() check.initOrder() - if !check.conf.DisableUnusedImportCheck { - check.unusedImports() + // perform delayed checks + // (cannot use range - delayed checks may add more delayed checks; + // e.g., when type-checking delayed embedded interfaces) + for i := 0; i < len(check.delayed); i++ { + check.delayed[i]() } - // perform delayed checks - for _, f := range check.delayed { - f() + if !check.conf.DisableUnusedImportCheck { + check.unusedImports() } check.recordUntyped() diff --git a/src/go/types/check_test.go b/src/go/types/check_test.go index e3ca90a6bd..720d8db293 100644 --- a/src/go/types/check_test.go +++ b/src/go/types/check_test.go @@ -61,6 +61,7 @@ var tests = [][]string{ {"testdata/cycles2.src"}, {"testdata/cycles3.src"}, {"testdata/cycles4.src"}, + {"testdata/cycles5.src"}, {"testdata/init0.src"}, {"testdata/init1.src"}, {"testdata/init2.src"}, diff --git a/src/go/types/decl.go b/src/go/types/decl.go index 9b250b30e7..f78aed9368 100644 --- a/src/go/types/decl.go +++ b/src/go/types/decl.go @@ -37,6 +37,18 @@ func (check *Checker) declare(scope *Scope, id *ast.Ident, obj Object, pos token } } +// pathString returns a string of the form a->b-> ... ->g for a path [a, b, ... g]. +func pathString(path []*TypeName) string { + var s string + for i, p := range path { + if i > 0 { + s += "->" + } + s += p.Name() + } + return s +} + // objDecl type-checks the declaration of obj in its respective (file) context. // See check.typ for the details on def and path. func (check *Checker) objDecl(obj Object, def *Named, path []*TypeName) { @@ -45,7 +57,7 @@ func (check *Checker) objDecl(obj Object, def *Named, path []*TypeName) { } if trace { - check.trace(obj.Pos(), "-- declaring %s", obj.Name()) + check.trace(obj.Pos(), "-- declaring %s (path = %s)", obj.Name(), pathString(path)) check.indent++ defer func() { check.indent-- diff --git a/src/go/types/interfaces.go b/src/go/types/interfaces.go new file mode 100644 index 0000000000..f529377b9c --- /dev/null +++ b/src/go/types/interfaces.go @@ -0,0 +1,440 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this src code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package types + +import ( + "bytes" + "fmt" + "go/ast" + "go/token" +) + +// This file implements the collection of an interface's methods +// without relying on partially computed types of methods or interfaces +// for interface types declared at the package level. +// +// Because interfaces must not embed themselves, directly or indirectly, +// the method set of a valid interface can always be computed independent +// of any cycles that might exist via method signatures (see also issue #18395). +// +// Except for blank method name and interface cycle errors, no errors +// are reported. Affected methods or embedded interfaces are silently +// dropped. Subsequent type-checking of the interface will check +// signatures and embedded interfaces and report errors at that time. +// +// Only infoFromTypeLit should be called directly from code outside this file +// to compute an ifaceInfo. + +// ifaceInfo describes the method set for an interface. +// The zero value for an ifaceInfo is a ready-to-use ifaceInfo representing +// the empty interface. +type ifaceInfo struct { + explicits int // number of explicitly declared methods + methods []*methodInfo // all methods, starting with explicitly declared ones in source order +} + +// emptyIfaceInfo represents the ifaceInfo for the empty interface. +var emptyIfaceInfo ifaceInfo + +func (info *ifaceInfo) String() string { + var buf bytes.Buffer + fmt.Fprintf(&buf, "interface{") + for i, m := range info.methods { + if i > 0 { + fmt.Fprint(&buf, " ") + } + fmt.Fprint(&buf, m) + } + fmt.Fprintf(&buf, "}") + return buf.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.) +type methodInfo struct { + 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 { + if info.fun != nil { + return info.fun.name + } + return info.src.Names[0].Name +} + +func (info *methodInfo) Pos() token.Pos { + if info.fun != nil { + return info.fun.Pos() + } + return info.src.Pos() +} + +func (info *methodInfo) id(pkg *Package) string { + if info.fun != nil { + return info.fun.Id() + } + return Id(pkg, info.src.Names[0].Name) +} + +// A methodInfoSet maps method ids to methodInfos. +// It is used to determine duplicate declarations. +// (A methodInfo set is the equivalent of an objset +// but for methodInfos rather than Objects.) +type methodInfoSet map[string]*methodInfo + +// insert attempts to insert an method m into the method set s. +// If s already contains an alternative method alt with +// the same name, insert leaves s unchanged and returns alt. +// Otherwise it inserts m and returns nil. +func (s *methodInfoSet) insert(pkg *Package, m *methodInfo) *methodInfo { + id := m.id(pkg) + if alt := (*s)[id]; alt != nil { + return alt + } + if *s == nil { + *s = make(methodInfoSet) + } + (*s)[id] = m + return nil +} + +// like Checker.declareInSet but for method infos. +func (check *Checker) declareInMethodSet(mset *methodInfoSet, pos token.Pos, m *methodInfo) bool { + if alt := mset.insert(check.pkg, m); alt != nil { + check.errorf(pos, "%s redeclared", m) + check.reportAltMethod(alt) + return false + } + return true +} + +// like Checker.reportAltDecl but for method infos. +func (check *Checker) reportAltMethod(m *methodInfo) { + if pos := m.Pos(); pos.IsValid() { + // We use "other" rather than "previous" here because + // the first declaration seen may not be textually + // earlier in the source. + check.errorf(pos, "\tother declaration of %s", m) // secondary error, \t indented + } +} + +// infoFromTypeLit computes the method set for the given interface iface. +// 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 +// interfaces. A non-nil result doesn't mean that there were no errors, +// 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) { + assert(iface != nil) + + // lazy-allocate interfaces map + if check.interfaces == nil { + check.interfaces = make(map[*TypeName]*ifaceInfo) + } + + if trace { + check.trace(iface.Pos(), "-- collect methods for %s (path = %s)", iface, pathString(path)) + check.indent++ + defer func() { + check.indent-- + check.trace(iface.Pos(), "=> %s", info) + }() + } + + // If the interface is named, check if we computed info already. + // + // This is not simply an optimization; we may run into stack + // overflow with recursive interface declarations. Example: + // + // type T interface { + // m() interface { T } + // } + // + // (Since recursive definitions can only be expressed via names, + // it is sufficient to track named interfaces here.) + // + // While at it, use the same mechanism to detect cycles. (We still + // have the path-based cycle check because we want to report the + // entire cycle if present.) + if tname != nil { + assert(path[len(path)-1] == tname) // tname must be last path element + var found bool + if info, found = check.interfaces[tname]; found { + if info == nil { + // We have a cycle and use check.cycle to report it. + // We are guaranteed that check.cycle also finds the + // cycle because when infoFromTypeLit is called, any + // tname that's already in check.interfaces was also + // added to the path. (But the converse is not true: + // A non-nil tname is always the last element in path.) + ok := check.cycle(tname, path, true) + assert(ok) + } + return + } + check.interfaces[tname] = nil // computation started but not complete + } + + if iface.Methods == nil { + // fast track for empty interface + info = &emptyIfaceInfo + } else { + // (syntactically) non-empty interface + info = new(ifaceInfo) + + // collect explicitly declared methods and embedded interfaces + var mset methodInfoSet + var embeddeds []*ifaceInfo + var positions []token.Pos // entries correspond to positions of embeddeds; used for error reporting + for _, f := range iface.Methods.List { + if len(f.Names) > 0 { + // We have a method with name f.Names[0]. + // (The parser ensures that there's only one method + // and we don't care if a constructed AST has more.) + + // spec: "As with all method sets, in an interface type, + // each method must have a unique non-blank name." + if name := f.Names[0]; name.Name == "_" { + check.errorf(name.Pos(), "invalid method name _") + continue // ignore + } + + m := &methodInfo{src: f} + if check.declareInMethodSet(&mset, f.Pos(), m) { + info.methods = append(info.methods, m) + } + } else { + // We have an embedded interface and f.Type is its + // (possibly qualified) embedded type name. Collect + // it if it's a valid interface. + var e *ifaceInfo + switch ename := f.Type.(type) { + case *ast.Ident: + e = check.infoFromTypeName(ename, path) + case *ast.SelectorExpr: + e = check.infoFromQualifiedTypeName(ename) + default: + // The parser makes sure we only see one of the above. + // Constructed ASTs may contain other (invalid) nodes; + // we simply ignore them. The full type-checking pass + // will report those as errors later. + } + if e != nil { + embeddeds = append(embeddeds, e) + positions = append(positions, f.Type.Pos()) + } + } + } + info.explicits = len(info.methods) + + // collect methods of embedded interfaces + for i, e := range embeddeds { + pos := positions[i] // position of type name of embedded interface + for _, m := range e.methods { + if check.declareInMethodSet(&mset, pos, m) { + info.methods = append(info.methods, m) + } + } + } + } + + // mark check.interfaces as complete + assert(info != nil) + if tname != nil { + check.interfaces[tname] = info + } + + return +} + +// infoFromTypeName computes the method set for the given type name +// which must denote a type whose underlying type is an interface. +// The same result qualifications apply as for infoFromTypeLit. +// infoFromTypeName should only be called from infoFromTypeLit. +func (check *Checker) infoFromTypeName(name *ast.Ident, path []*TypeName) *ifaceInfo { + // A single call of infoFromTypeName handles a sequence of (possibly + // recursive) type declarations connected via unqualified type names. + // Each type declaration leading to another typename causes a "tail call" + // (goto) of this function. The general scenario looks like this: + // + // ... + // type Pn T // previous declarations leading to T, path = [..., Pn] + // type T interface { T0; ... } // T0 leads to call of infoFromTypeName + // + // // infoFromTypeName(name = T0, path = [..., Pn, T]) + // type T0 T1 // path = [..., Pn, T, T0] + // type T1 T2 <-+ // path = [..., Pn, T, T0, T1] + // type T2 ... | // path = [..., Pn, T, T0, T1, T2] + // type Tn T1 --+ // path = [..., Pn, T, T0, T1, T2, Tn] and T1 is in path => cycle + // + // infoFromTypeName returns nil when such a cycle is detected. But in + // contrast to cycles involving interfaces, we must not report the + // error for "type name only" cycles because they will be found again + // during type-checking of embedded interfaces. Reporting those cycles + // here would lead to double reporting. Cycles involving embedding are + // not reported again later because type-checking of interfaces relies + // on the ifaceInfos computed here which are cycle-free by design. + // + // Remember the path length to detect "type name only" cycles. + start := len(path) + +typenameLoop: + // name must be a type name denoting a type whose underlying type is an interface + _, obj := check.scope.LookupParent(name.Name, check.pos /* use Eval position, if any */) + if obj == nil { + return nil + } + tname, _ := obj.(*TypeName) + if tname == nil { + return nil + } + + // We have a type name. It may be predeclared (error type), + // imported (dot import), or declared by a type declaration. + // It may not be an interface (e.g., predeclared type int). + // Resolve it by analyzing each possible case. + + // Abort but don't report an error if we have a "type name only" + // cycle (see big function comment). + if check.cycle(tname, path[start:], false) { + return nil + } + + // Abort and report an error if we have a general cycle. + if check.cycle(tname, path, true) { + return nil + } + + path = append(path, tname) + + // If tname is a package-level type declaration, it must be + // in the objMap. Follow the RHS of that declaration if so. + // The RHS may be a literal type (likely case), or another + // (possibly parenthesized and/or qualified) type name. + // (The declaration may be an alias declaration, but it + // doesn't matter for the purpose of determining the under- + // lying interface.) + if decl := check.objMap[tname]; decl != nil { + switch typ := unparen(decl.typ).(type) { + case *ast.Ident: + // type tname T + name = typ + goto typenameLoop + case *ast.SelectorExpr: + // type tname p.T + return check.infoFromQualifiedTypeName(typ) + case *ast.InterfaceType: + // type tname interface{...} + return check.infoFromTypeLit(typ, tname, path) + } + // type tname X // and X is not an interface type + return nil + } + + // If tname is not a package-level declaration, in a well-typed + // program it should be a predeclared (error type), imported (dot + // import), or function local declaration. Either way, it should + // have been fully declared before use, except if there is a direct + // cycle, and direct cycles will be caught above. Also, the denoted + // type should be an interface (e.g., int is not an interface). + if typ := tname.typ; typ != nil { + // typ should be an interface + if ityp, _ := typ.Underlying().(*Interface); ityp != nil { + return infoFromType(ityp) + } + } + + // In all other cases we have some error. + return nil +} + +// infoFromQualifiedTypeName computes the method set for the given qualified type name, or nil. +func (check *Checker) infoFromQualifiedTypeName(qname *ast.SelectorExpr) *ifaceInfo { + // see also Checker.selector + name, _ := qname.X.(*ast.Ident) + if name == nil { + return nil + } + _, obj1 := check.scope.LookupParent(name.Name, check.pos /* use Eval position, if any */) + if obj1 == nil { + return nil + } + pname, _ := obj1.(*PkgName) + if pname == nil { + return nil + } + assert(pname.pkg == check.pkg) + obj2 := pname.imported.scope.Lookup(qname.Sel.Name) + if obj2 == nil || !obj2.Exported() { + return nil + } + tname, _ := obj2.(*TypeName) + if tname == nil { + return nil + } + ityp, _ := tname.typ.Underlying().(*Interface) + if ityp == nil { + return nil + } + return infoFromType(ityp) +} + +// infoFromType computes the method set for the given interface type. +// The result is never nil. +func infoFromType(typ *Interface) *ifaceInfo { + assert(typ.allMethods != nil) // typ must be completely set up + + // fast track for empty interface + n := len(typ.allMethods) + if n == 0 { + return &emptyIfaceInfo + } + + info := new(ifaceInfo) + info.explicits = len(typ.methods) + info.methods = make([]*methodInfo, n) + + // If there are no embedded interfaces, simply collect the + // explicitly declared methods (optimization of common case). + if len(typ.methods) == n { + for i, m := range typ.methods { + info.methods[i] = &methodInfo{fun: m} + } + return info + } + + // Interface types have a separate list for explicitly declared methods + // which shares its methods with the list of all (explicitly declared or + // embedded) methods. Collect all methods in a set so we can separate + // the embedded methods from the explicitly declared ones. + all := make(map[*Func]bool, n) + for _, m := range typ.allMethods { + all[m] = true + } + assert(len(all) == n) // methods must be unique + + // collect explicitly declared methods + info.methods = make([]*methodInfo, n) + for i, m := range typ.methods { + info.methods[i] = &methodInfo{fun: m} + delete(all, m) + } + + // collect remaining (embedded) methods + i := len(typ.methods) + for m := range all { + info.methods[i] = &methodInfo{fun: m} + i++ + } + assert(i == n) + + return info +} diff --git a/src/go/types/ordering.go b/src/go/types/ordering.go deleted file mode 100644 index 3579abf7d7..0000000000 --- a/src/go/types/ordering.go +++ /dev/null @@ -1,123 +0,0 @@ -// Copyright 2014 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. - -// This file implements resolveOrder. - -package types - -import ( - "go/ast" - "sort" -) - -// resolveOrder computes the order in which package-level objects -// must be type-checked. -// -// Interface types appear first in the list, sorted topologically -// by dependencies on embedded interfaces that are also declared -// in this package, followed by all other objects sorted in source -// order. -// -// TODO(gri) Consider sorting all types by dependencies here, and -// in the process check _and_ report type cycles. This may simplify -// the full type-checking phase. -// -func (check *Checker) resolveOrder() []Object { - var ifaces, others []Object - - // collect interface types with their dependencies, and all other objects - for obj := range check.objMap { - if ityp := check.interfaceFor(obj); ityp != nil { - ifaces = append(ifaces, obj) - // determine dependencies on embedded interfaces - for _, f := range ityp.Methods.List { - if len(f.Names) == 0 { - // Embedded interface: The type must be a (possibly - // qualified) identifier denoting another interface. - // Imported interfaces are already fully resolved, - // so we can ignore qualified identifiers. - if ident, _ := f.Type.(*ast.Ident); ident != nil { - embedded := check.pkg.scope.Lookup(ident.Name) - if check.interfaceFor(embedded) != nil { - check.objMap[obj].addDep(embedded) - } - } - } - } - } else { - others = append(others, obj) - } - } - - // final object order - var order []Object - - // sort interface types topologically by dependencies, - // and in source order if there are no dependencies - sort.Sort(inSourceOrder(ifaces)) - visited := make(objSet) - for _, obj := range ifaces { - check.appendInPostOrder(&order, obj, visited) - } - - // sort everything else in source order - sort.Sort(inSourceOrder(others)) - - return append(order, others...) -} - -// interfaceFor returns the AST interface denoted by obj, or nil. -func (check *Checker) interfaceFor(obj Object) *ast.InterfaceType { - tname, _ := obj.(*TypeName) - if tname == nil { - return nil // not a type - } - d := check.objMap[obj] - if d == nil { - check.dump("%s: %s should have been declared", obj.Pos(), obj.Name()) - unreachable() - } - if d.typ == nil { - return nil // invalid AST - ignore (will be handled later) - } - ityp, _ := d.typ.(*ast.InterfaceType) - return ityp -} - -func (check *Checker) appendInPostOrder(order *[]Object, obj Object, visited objSet) { - if visited[obj] { - // We've already seen this object; either because it's - // already added to order, or because we have a cycle. - // In both cases we stop. Cycle errors are reported - // when type-checking types. - return - } - visited[obj] = true - - d := check.objMap[obj] - for _, obj := range orderedSetObjects(d.deps) { - check.appendInPostOrder(order, obj, visited) - } - - *order = append(*order, obj) -} - -func orderedSetObjects(set objSet) []Object { - list := make([]Object, len(set)) - i := 0 - for obj := range set { - // we don't care about the map element value - list[i] = obj - i++ - } - sort.Sort(inSourceOrder(list)) - return list -} - -// inSourceOrder implements the sort.Sort interface. -type inSourceOrder []Object - -func (a inSourceOrder) Len() int { return len(a) } -func (a inSourceOrder) Less(i, j int) bool { return a[i].order() < a[j].order() } -func (a inSourceOrder) Swap(i, j int) { a[i], a[j] = a[j], a[i] } diff --git a/src/go/types/resolver.go b/src/go/types/resolver.go index d03c1799af..b5bec13d9d 100644 --- a/src/go/types/resolver.go +++ b/src/go/types/resolver.go @@ -9,6 +9,7 @@ import ( "go/ast" "go/constant" "go/token" + "sort" "strconv" "strings" "unicode" @@ -453,8 +454,17 @@ func (check *Checker) collectObjects() { } } -// packageObjects typechecks all package objects in objList, but not function bodies. -func (check *Checker) packageObjects(objList []Object) { +// packageObjects typechecks all package objects, but not function bodies. +func (check *Checker) packageObjects() { + // process package objects in source order for reproducible results + objList := make([]Object, len(check.objMap)) + i := 0 + for obj := range check.objMap { + objList[i] = obj + i++ + } + sort.Sort(inSourceOrder(objList)) + // add new methods to already type-checked types (from a prior Checker.Files call) for _, obj := range objList { if obj, _ := obj.(*TypeName); obj != nil && obj.typ != nil { @@ -476,6 +486,13 @@ func (check *Checker) packageObjects(objList []Object) { check.methods = nil } +// inSourceOrder implements the sort.Sort interface. +type inSourceOrder []Object + +func (a inSourceOrder) Len() int { return len(a) } +func (a inSourceOrder) Less(i, j int) bool { return a[i].order() < a[j].order() } +func (a inSourceOrder) Swap(i, j int) { a[i], a[j] = a[j], a[i] } + // functionBodies typechecks all function bodies. func (check *Checker) functionBodies() { for _, f := range check.funcs { diff --git a/src/go/types/testdata/cycles.src b/src/go/types/testdata/cycles.src index b4bd5d8b15..0165baa21a 100644 --- a/src/go/types/testdata/cycles.src +++ b/src/go/types/testdata/cycles.src @@ -52,9 +52,9 @@ type ( // interfaces I0 /* ERROR cycle */ interface{ I0 } - I1 interface{ I2 } + I1 /* ERROR cycle */ interface{ I2 } I2 interface{ I3 } - I3 /* ERROR cycle */ interface{ I1 } + I3 interface{ I1 } I4 interface{ f(I4) } diff --git a/src/go/types/testdata/cycles4.src b/src/go/types/testdata/cycles4.src index 3f6304be6b..445babca68 100644 --- a/src/go/types/testdata/cycles4.src +++ b/src/go/types/testdata/cycles4.src @@ -108,15 +108,3 @@ type Element interface { type Event interface { Target() Element } - -// Recognize issue #13895. - -type ( - _ interface{ m(B1) } - A1 interface{ a(D1) } - B1 interface{ A1 } - C1 interface{ B1 /* ERROR issue #18395 */ } - D1 interface{ C1 } -) - -var _ A1 = C1 /* ERROR cannot use C1 */ (nil) \ No newline at end of file diff --git a/src/go/types/testdata/cycles5.src b/src/go/types/testdata/cycles5.src new file mode 100644 index 0000000000..984da681bd --- /dev/null +++ b/src/go/types/testdata/cycles5.src @@ -0,0 +1,117 @@ +// Copyright 2017 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 p + +import "unsafe" + +// test case from issue #18395 + +type ( + A interface { B } + B interface { C } + C interface { D; F() A } + D interface { G() B } +) + +var _ = A(nil).G // G must be found + + +// test case from issue #21804 + +type sourceBridge interface { + listVersions() ([]Version, error) +} + +type Constraint interface { + copyTo(*ConstraintMsg) +} + +type ConstraintMsg struct{} + +func (m *ConstraintMsg) asUnpairedVersion() UnpairedVersion { + return nil +} + +type Version interface { + Constraint +} + +type UnpairedVersion interface { + Version +} + +var _ Constraint = UnpairedVersion(nil) + + +// derived test case from issue #21804 + +type ( + _ interface{ m(B1) } + A1 interface{ a(D1) } + B1 interface{ A1 } + C1 interface{ B1 } + D1 interface{ C1 } +) + +var _ A1 = C1(nil) + + +// derived test case from issue #22701 + +func F(x I4) interface{} { + return x.Method() +} + +type Unused interface { + RefersToI1(a I1) +} + +type I1 interface { + I2 + I3 +} + +type I2 interface { + RefersToI4() I4 +} + +type I3 interface { + Method() interface{} +} + +type I4 interface { + I1 +} + + +// check embedding of error interface + +type Error interface{ error } + +var err Error +var _ = err.Error() + + +// more esoteric cases + +type ( + T1 interface { T2 /* ERROR not an interface */ } + T2 /* ERROR cycle */ T2 +) + +type ( + T3 interface { T4 /* ERROR not an interface */ } + T4 /* ERROR cycle */ T5 + T5 = T6 + T6 = T7 + T7 = T4 +) + + +// arbitrary code may appear inside an interface + +type I interface { + m([unsafe.Sizeof(func() { I.m(nil) })]byte) // should report an error (see #22992) +} diff --git a/src/go/types/testdata/decls0.src b/src/go/types/testdata/decls0.src index d4df386b13..0e637f4f01 100644 --- a/src/go/types/testdata/decls0.src +++ b/src/go/types/testdata/decls0.src @@ -159,13 +159,13 @@ type ( I8 /* ERROR "illegal cycle" */ interface { I8 } - I9 interface { + I9 /* ERROR "illegal cycle" */ interface { I10 } I10 interface { I11 } - I11 /* ERROR "illegal cycle" */ interface { + I11 interface { I9 } diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go index 92ab06b0f2..0d95e8018d 100644 --- a/src/go/types/typexpr.go +++ b/src/go/types/typexpr.go @@ -69,20 +69,10 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *Named, path []*TypeNa case *TypeName: x.mode = typexpr - // check for cycle - // (it's ok to iterate forward because each named type appears at most once in path) - for i, prev := range path { - if prev == obj { - check.errorf(obj.pos, "illegal cycle in declaration of %s", obj.name) - // print cycle - for _, obj := range path[i:] { - check.errorf(obj.Pos(), "\t%s refers to", obj.Name()) // secondary error, \t indented - } - check.errorf(obj.Pos(), "\t%s", obj.Name()) - // maintain x.mode == typexpr despite error - typ = Typ[Invalid] - break - } + if check.cycle(obj, path, true) { + // maintain x.mode == typexpr despite error + typ = Typ[Invalid] + break } case *Var: @@ -116,6 +106,26 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *Named, path []*TypeNa x.typ = typ } +// cycle reports whether obj appears in path or not. +// If it does, and report is set, it also reports a cycle error. +func (check *Checker) cycle(obj *TypeName, path []*TypeName, report bool) bool { + // (it's ok to iterate forward because each named type appears at most once in path) + for i, prev := range path { + if prev == obj { + if report { + check.errorf(obj.pos, "illegal cycle in declaration of %s", obj.name) + // print cycle + for _, obj := range path[i:] { + check.errorf(obj.Pos(), "\t%s refers to", obj.Name()) // secondary error, \t indented + } + check.errorf(obj.Pos(), "\t%s", obj.Name()) + } + return true + } + } + return false +} + // typExpr type-checks the type expression e and returns its type, or Typ[Invalid]. // If def != nil, e is the type specification for the named type def, declared // in a type declaration, and def.underlying will be set to the type of e before @@ -456,44 +466,86 @@ func (check *Checker) declareInSet(oset *objset, pos token.Pos, obj Object) bool return true } -func (check *Checker) interfaceType(iface *Interface, ityp *ast.InterfaceType, def *Named, path []*TypeName) { - // empty interface: common case - if ityp.Methods == nil { +func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, def *Named, path []*TypeName) { + // fast-track empty interface + if iface.Methods == nil { + ityp.allMethods = markComplete return } - // The parser ensures that field tags are nil and we don't - // care if a constructed AST contains non-nil tags. + // 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 delayed function + check.delay(func() { + check.scope = interfaceScope + if trace { + check.trace(iface.Pos(), "-- delayed checking embedded interfaces of %s", iface) + check.indent++ + defer func() { + check.indent-- + }() + } + for _, f := range iface.Methods.List { + 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 + // constructed ASTs may be wrong). + if typ == Typ[Invalid] { + continue // error reported before + } + if !isNamed(typ) { + check.invalidAST(f.Type.Pos(), "%s is not a named type", f.Type) + continue + } + embed, _ := typ.Underlying().(*Interface) + if embed == nil { + check.errorf(f.Type.Pos(), "%s is not an interface", typ) + continue + } + // Correct embedded interfaces must be complete - + // don't just assert, but report error since this + // used to be the underlying cause for issue #18395. + if embed.allMethods == nil { + check.dump("%s: incomplete embedded interface %s", f.Type.Pos(), typ) + unreachable() + } + // collect interface + // (at this point we know that typ must be a named, non-basic type) + ityp.embeddeds = append(ityp.embeddeds, typ.(*Named)) + } + } + // sort to match NewInterface + // TODO(gri) we may be able to switch to source order + sort.Sort(byUniqueTypeName(ityp.embeddeds)) + }) + + // compute method set + var tname *TypeName + if def != nil { + tname = def.obj + } + info := check.infoFromTypeLit(iface, tname, path) + if info == nil || info == &emptyIfaceInfo { + // error or empty interface - exit early + ityp.allMethods = markComplete + return + } // use named receiver type if available (for better error messages) - var recvTyp Type = iface + var recvTyp Type = ityp if def != nil { recvTyp = def } - // Phase 1: Collect explicitly declared methods, the corresponding - // signature (AST) expressions, and the list of embedded - // type (AST) expressions. Do not resolve signatures or - // embedded types yet to avoid cycles referring to this - // interface. - - var ( - mset objset - signatures []ast.Expr // list of corresponding method signatures - embedded []ast.Expr // list of embedded types - ) - for _, f := range ityp.Methods.List { - if len(f.Names) > 0 { - // The parser ensures that there's only one method - // and we don't care if a constructed AST has more. - name := f.Names[0] + // collect methods + var sigfix []*methodInfo + for i, minfo := range info.methods { + fun := minfo.fun + if fun == nil { + name := minfo.src.Names[0] pos := name.Pos() - // spec: "As with all method sets, in an interface type, - // each method must have a unique non-blank name." - if name.Name == "_" { - check.errorf(pos, "invalid method name _") - continue - } // Don't type-check signature yet - use an // empty signature now and update it later. // Since we know the receiver, set it up now @@ -504,91 +556,42 @@ func (check *Checker) interfaceType(iface *Interface, ityp *ast.InterfaceType, d // also the T4 and T5 tests in testdata/cycles2.src. sig := new(Signature) sig.recv = NewVar(pos, check.pkg, "", recvTyp) - m := NewFunc(pos, check.pkg, name.Name, sig) - if check.declareInSet(&mset, pos, m) { - iface.methods = append(iface.methods, m) - iface.allMethods = append(iface.allMethods, m) - signatures = append(signatures, f.Type) - check.recordDef(name, m) - } - } else { - // embedded type - embedded = append(embedded, f.Type) + fun = NewFunc(pos, check.pkg, name.Name, sig) + minfo.fun = fun + check.recordDef(name, fun) + sigfix = append(sigfix, minfo) } + // fun != nil + if i < info.explicits { + ityp.methods = append(ityp.methods, fun) + } + ityp.allMethods = append(ityp.allMethods, fun) } - // Phase 2: Resolve embedded interfaces. Because an interface must not - // embed itself (directly or indirectly), each embedded interface - // can be fully resolved without depending on any method of this - // interface (if there is a cycle or another error, the embedded - // type resolves to an invalid type and is ignored). - // In particular, the list of methods for each embedded interface - // must be complete (it cannot depend on this interface), and so - // those methods can be added to the list of all methods of this - // interface. - - for _, e := range embedded { - pos := e.Pos() - typ := check.typExpr(e, nil, path) - // Determine underlying embedded (possibly incomplete) type - // by following its forward chain. - named, _ := typ.(*Named) - under := underlying(named) - embed, _ := under.(*Interface) - if embed == nil { - if typ != Typ[Invalid] { - check.errorf(pos, "%s is not an interface", typ) - } - continue - } - iface.embeddeds = append(iface.embeddeds, named) - // collect embedded methods - if embed.allMethods == nil { - check.errorf(pos, "internal error: incomplete embedded interface %s (issue #18395)", named) - } - for _, m := range embed.allMethods { - if check.declareInSet(&mset, pos, m) { - iface.allMethods = append(iface.allMethods, m) - } - } - } - - // Phase 3: At this point all methods have been collected for this interface. - // It is now safe to type-check the signatures of all explicitly - // declared methods, even if they refer to this interface via a cycle - // and embed the methods of this interface in a parameter of interface - // type. - - for i, m := range iface.methods { - expr := signatures[i] - typ := check.typ(expr) + // fix signatures now that we have collected all methods + for _, minfo := range sigfix { + typ := check.typ(minfo.src.Type) sig, _ := typ.(*Signature) if sig == nil { if typ != Typ[Invalid] { - check.invalidAST(expr.Pos(), "%s is not a method signature", typ) + check.invalidAST(minfo.src.Type.Pos(), "%s is not a method signature", typ) } continue // keep method with empty method signature } // update signature, but keep recv that was set up before - old := m.typ.(*Signature) + old := minfo.fun.typ.(*Signature) sig.recv = old.recv - *old = *sig // update signature (don't replace it!) + *old = *sig // update signature (don't replace pointer!) } - // TODO(gri) The list of explicit methods is only sorted for now to - // produce the same Interface as NewInterface. We may be able to - // claim source order in the future. Revisit. - sort.Sort(byUniqueMethodName(iface.methods)) + // sort to match NewInterface + // TODO(gri) we may be able to switch to source order + sort.Sort(byUniqueMethodName(ityp.methods)) - // TODO(gri) The list of embedded types is only sorted for now to - // produce the same Interface as NewInterface. We may be able to - // claim source order in the future. Revisit. - sort.Sort(byUniqueTypeName(iface.embeddeds)) - - if iface.allMethods == nil { - iface.allMethods = make([]*Func, 0) // mark interface as complete + if ityp.allMethods == nil { + ityp.allMethods = markComplete } else { - sort.Sort(byUniqueMethodName(iface.allMethods)) + sort.Sort(byUniqueMethodName(ityp.allMethods)) } }