diff --git a/go/types/check.go b/go/types/check.go index 3ad2889c88d..15eae5f2090 100644 --- a/go/types/check.go +++ b/go/types/check.go @@ -10,7 +10,6 @@ import ( "fmt" "go/ast" "go/token" - "sort" "code.google.com/p/go.tools/go/exact" ) @@ -200,17 +199,6 @@ func (check *checker) handleBailout(err *error) { } } -func mapObjects(m map[Object]*declInfo) []Object { - list := make([]Object, len(m)) - i := 0 - for obj := range m { - list[i] = obj - i++ - } - sort.Sort(inSourceOrder(list)) - return list -} - // Files checks the provided files as part of the checker's package. func (check *checker) Files(files []*ast.File) (err error) { defer check.handleBailout(&err) @@ -219,7 +207,7 @@ func (check *checker) Files(files []*ast.File) (err error) { check.collectObjects() - objList := mapObjects(check.objMap) + objList := check.resolveOrder() check.packageObjects(objList) diff --git a/go/types/decl.go b/go/types/decl.go index a81feb90a04..8ef003d4514 100644 --- a/go/types/decl.go +++ b/go/types/decl.go @@ -50,7 +50,7 @@ func (check *checker) objDecl(obj Object, def *Named, path []*TypeName) { d := check.objMap[obj] if d == nil { - check.dump("%s: %s should have been declared", obj.Pos(), obj) + check.dump("%s: %s should have been declared", obj.Pos(), obj.Name()) unreachable() } diff --git a/go/types/ordering.go b/go/types/ordering.go new file mode 100644 index 00000000000..cc38cfedcc2 --- /dev/null +++ b/go/types/ordering.go @@ -0,0 +1,108 @@ +// 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) identifer 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)) + if debug { + for _, obj := range ifaces { + assert(check.objMap[obj].mark == 0) + } + } + for _, obj := range ifaces { + check.appendInPostOrder(&order, obj) + } + + // 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) { + d := check.objMap[obj] + if d.mark != 0 { + // 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 + } + d.mark = 1 + + for _, obj := range orderedSetObjects(d.deps) { + check.appendInPostOrder(order, obj) + } + + *order = append(*order, obj) +} diff --git a/go/types/resolver.go b/go/types/resolver.go index 18f9b8b1c7a..90217f1cc19 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -25,8 +25,8 @@ type declInfo struct { init ast.Expr // init expression, or nil fdecl *ast.FuncDecl // func declaration, or nil - deps map[Object]bool // init dependencies; lazily allocated - mark int // see check.dependencies + deps map[Object]bool // type and init dependencies; lazily allocated + mark int // for dependency analysis } // hasInitializer reports whether the declared object has an initialization @@ -437,7 +437,7 @@ func (check *checker) unusedImports() { } } -func setObjects(set map[Object]bool) []Object { +func orderedSetObjects(set map[Object]bool) []Object { list := make([]Object, len(set)) i := 0 for obj := range set { @@ -449,11 +449,6 @@ func setObjects(set map[Object]bool) []Object { return list } -// TODO(gri) Instead of this support function, introduce type -// that combines an map[Object]*declInfo and []Object so that -// we can encapsulate this and don't have to depend on correct -// Pos() information. - // inSourceOrder implements the sort.Sort interface. type inSourceOrder []Object @@ -526,7 +521,7 @@ func (check *checker) dependencies(obj Object, path []Object) { path = append(path, obj) // len(path) > 0 init.mark = len(path) // init.mark > 0 - for _, obj := range setObjects(init.deps) { + for _, obj := range orderedSetObjects(init.deps) { check.dependencies(obj, path) } init.mark = -1 // init.mark < 0 diff --git a/go/types/testdata/cycles.src b/go/types/testdata/cycles.src index 2a6a3ec962f..621d83c9450 100644 --- a/go/types/testdata/cycles.src +++ b/go/types/testdata/cycles.src @@ -52,9 +52,9 @@ type ( // interfaces I0 /* ERROR cycle */ interface{ I0 } - I1 /* ERROR cycle */ interface{ I2 } + I1 interface{ I2 } I2 interface{ I3 } - I3 interface{ I1 } + I3 /* ERROR cycle */ interface{ I1 } I4 interface{ f(I4) } diff --git a/go/types/testdata/cycles4.src b/go/types/testdata/cycles4.src index 528f205777c..445babca68b 100644 --- a/go/types/testdata/cycles4.src +++ b/go/types/testdata/cycles4.src @@ -66,3 +66,45 @@ type T4 interface { func _(x T1, y T3) { x = y } + +// Check that interfaces are type-checked in order of +// (embedded interface) dependencies (was issue 7158). + +var x1 T5 = T7(nil) + +type T5 interface { + T6 +} + +type T6 interface { + m() T7 +} +type T7 interface { + T5 +} + +// Actual test case from issue 7158. + +func wrapNode() Node { + return wrapElement() +} + +func wrapElement() Element { + return nil +} + +type EventTarget interface { + AddEventListener(Event) +} + +type Node interface { + EventTarget +} + +type Element interface { + Node +} + +type Event interface { + Target() Element +} diff --git a/go/types/testdata/decls0.src b/go/types/testdata/decls0.src index 5ab15b4f574..9da75b90ba3 100644 --- a/go/types/testdata/decls0.src +++ b/go/types/testdata/decls0.src @@ -155,18 +155,14 @@ type ( I8 /* ERROR "illegal cycle" */ interface { I8 } - // Use I09 (rather than I9) because it appears lexically before - // I10 so that we get the illegal cycle here rather then in the - // declaration of I10. If the implementation sorts by position - // rather than name, the error message will still be here. - I09 /* ERROR "illegal cycle" */ interface { + I9 interface { I10 } I10 interface { I11 } - I11 interface { - I09 + I11 /* ERROR "illegal cycle" */ interface { + I9 } C1 chan int