diff --git a/go/types/check_test.go b/go/types/check_test.go index f65ba49259..befeee21ce 100644 --- a/go/types/check_test.go +++ b/go/types/check_test.go @@ -48,6 +48,10 @@ var tests = [][]string{ {"testdata/errors.src"}, {"testdata/importdecl0a.src", "testdata/importdecl0b.src"}, {"testdata/cycles.src"}, + {"testdata/cycles1.src"}, + {"testdata/cycles2.src"}, + {"testdata/cycles3.src"}, + {"testdata/cycles4.src"}, {"testdata/decls0.src"}, {"testdata/decls1.src"}, {"testdata/decls2a.src", "testdata/decls2b.src"}, diff --git a/go/types/errors.go b/go/types/errors.go index fe51597b9b..55a6c96cbe 100644 --- a/go/types/errors.go +++ b/go/types/errors.go @@ -14,7 +14,6 @@ import ( "strings" ) -// TODO(gri) eventually assert should disappear. func assert(p bool) { if !p { panic("assertion failed") @@ -33,7 +32,7 @@ func (check *checker) formatMsg(format string, args []interface{}) string { case operand: panic("internal error: should always pass *operand") case token.Pos: - args[i] = check.fset.Position(a) + args[i] = check.fset.Position(a).String() case ast.Expr: args[i] = exprString(a) } @@ -270,6 +269,17 @@ func writeType(buf *bytes.Buffer, typ Type) { fmt.Fprintf(buf, "", t.name) case *Interface: + // We write the source-level methods and embedded types rather + // than the actual method set since resolved method signatures + // may have non-printable cycles if parameters have anonymous + // interface types that (directly or indirectly) embed the + // current interface. For instance, consider the result type + // of m: + // + // type T interface{ + // m() interface{ T } + // } + // buf.WriteString("interface{") for i, m := range t.methods { if i > 0 { @@ -278,6 +288,12 @@ func writeType(buf *bytes.Buffer, typ Type) { buf.WriteString(m.name) writeSignature(buf, m.typ.(*Signature)) } + for i, typ := range t.types { + if i > 0 || len(t.methods) > 0 { + buf.WriteString("; ") + } + writeType(buf, typ) + } buf.WriteByte('}') case *Map: diff --git a/go/types/eval_test.go b/go/types/eval_test.go index 90a6deeeb3..41663185a7 100644 --- a/go/types/eval_test.go +++ b/go/types/eval_test.go @@ -58,7 +58,7 @@ func TestEvalBasic(t *testing.T) { } func TestEvalComposite(t *testing.T) { - for _, test := range testTypes { + for _, test := range independentTestTypes { testEval(t, nil, nil, test.src, nil, test.str, "") } } diff --git a/go/types/gcimporter.go b/go/types/gcimporter.go index 4648f43041..15c07b2119 100644 --- a/go/types/gcimporter.go +++ b/go/types/gcimporter.go @@ -16,6 +16,7 @@ import ( "io" "os" "path/filepath" + "sort" "strconv" "strings" "text/scanner" @@ -633,7 +634,9 @@ func (p *gcParser) parseInterfaceType() Type { } p.expect('}') + sort.Sort(byUniqueMethodName(methods)) typ.methods = methods + typ.allMethods = methods // ok to share underlying array since we are not changing methods return typ } diff --git a/go/types/lookup.go b/go/types/lookup.go index 398bb626ab..842a416180 100644 --- a/go/types/lookup.go +++ b/go/types/lookup.go @@ -177,7 +177,8 @@ func lookupFieldOrMethod(T Type, pkg *Package, name string) (obj Object, index [ case *Interface: // look for a matching method - if i, m := lookupMethod(t.methods, pkg, name); m != nil { + // TODO(gri) t.allMethods is sorted - use binary search + if i, m := lookupMethod(t.allMethods, pkg, name); m != nil { assert(m.typ != nil) index = concat(e.index, i) if obj != nil || e.multiples { @@ -252,8 +253,9 @@ func MissingMethod(V Type, T *Interface, static bool) (method *Func, wrongType b // TODO(gri) Consider using method sets here. Might be more efficient. if ityp, _ := V.Underlying().(*Interface); ityp != nil { - for _, m := range T.methods { - _, obj := lookupMethod(ityp.methods, m.pkg, m.name) + // TODO(gri) allMethods is sorted - can do this more efficiently + for _, m := range T.allMethods { + _, obj := lookupMethod(ityp.allMethods, m.pkg, m.name) switch { case obj == nil: if static { @@ -267,7 +269,7 @@ func MissingMethod(V Type, T *Interface, static bool) (method *Func, wrongType b } // A concrete type implements T if it implements all methods of T. - for _, m := range T.methods { + for _, m := range T.allMethods { obj, _, indirect := lookupFieldOrMethod(V, m.pkg, m.name) if obj == nil { return m, false diff --git a/go/types/methodset.go b/go/types/methodset.go index 806cd15a4b..4ecd6f5daf 100644 --- a/go/types/methodset.go +++ b/go/types/methodset.go @@ -168,7 +168,7 @@ func NewMethodSet(T Type) *MethodSet { } case *Interface: - mset = mset.add(t.methods, e.index, true, e.multiples) + mset = mset.add(t.allMethods, e.index, true, e.multiples) } } diff --git a/go/types/predicates.go b/go/types/predicates.go index 82af6f800d..cde1ec4f8d 100644 --- a/go/types/predicates.go +++ b/go/types/predicates.go @@ -6,6 +6,8 @@ package types +import "sort" + func isNamed(typ Type) bool { if _, ok := typ.(*Basic); ok { return ok @@ -105,8 +107,22 @@ func hasNil(typ Type) bool { return false } -// IsIdentical returns true if x and y are identical. +// IsIdentical reports whether x and y are identical. func IsIdentical(x, y Type) bool { + return isIdenticalInternal(x, y, nil) +} + +// An ifacePair is a node in a stack of interface type pairs compared for identity. +type ifacePair struct { + x, y *Interface + prev *ifacePair +} + +func (p *ifacePair) identical(q *ifacePair) bool { + return p.x == q.x && p.y == q.y || p.x == q.y && p.y == q.x +} + +func isIdenticalInternal(x, y Type, p *ifacePair) bool { if x == y { return true } @@ -124,13 +140,13 @@ func IsIdentical(x, y Type) bool { // Two array types are identical if they have identical element types // and the same array length. if y, ok := y.(*Array); ok { - return x.len == y.len && IsIdentical(x.elt, y.elt) + return x.len == y.len && isIdenticalInternal(x.elt, y.elt, p) } case *Slice: // Two slice types are identical if they have identical element types. if y, ok := y.(*Slice); ok { - return IsIdentical(x.elt, y.elt) + return isIdenticalInternal(x.elt, y.elt, p) } case *Struct: @@ -145,7 +161,7 @@ func IsIdentical(x, y Type) bool { if f.anonymous != g.anonymous || x.Tag(i) != y.Tag(i) || !f.sameId(g.pkg, g.name) || - !IsIdentical(f.typ, g.typ) { + !isIdenticalInternal(f.typ, g.typ, p) { return false } } @@ -156,14 +172,24 @@ func IsIdentical(x, y Type) bool { case *Pointer: // Two pointer types are identical if they have identical base types. if y, ok := y.(*Pointer); ok { - return IsIdentical(x.base, y.base) + return isIdenticalInternal(x.base, y.base, p) } case *Tuple: // Two tuples types are identical if they have the same number of elements // and corresponding elements have identical types. if y, ok := y.(*Tuple); ok { - return identicalTuples(x, y) + if x.Len() == y.Len() { + if x != nil { + for i, v := range x.vars { + w := y.vars[i] + if !isIdenticalInternal(v.typ, w.typ, p) { + return false + } + } + } + return true + } } case *Signature: @@ -173,8 +199,8 @@ func IsIdentical(x, y Type) bool { // names are not required to match. if y, ok := y.(*Signature); ok { return x.isVariadic == y.isVariadic && - identicalTuples(x.params, y.params) && - identicalTuples(x.results, y.results) + isIdenticalInternal(x.params, y.params, p) && + isIdenticalInternal(x.results, y.results, p) } case *Interface: @@ -182,20 +208,63 @@ func IsIdentical(x, y Type) bool { // the same names and identical function types. Lower-case method names from // different packages are always different. The order of the methods is irrelevant. if y, ok := y.(*Interface); ok { - return identicalMethods(x.methods, y.methods) + a := x.allMethods + b := y.allMethods + if len(a) == len(b) { + // Interface types are the only types where cycles can occur + // that are not "terminated" via named types; and such cycles + // can only be created via method parameter types that are + // anonymous interfaces (directly or indirectly) embedding + // the current interface. Example: + // + // type T interface { + // m() interface{T} + // } + // + // If two such (differently named) interfaces are compared, + // endless recursion occurs if the cycle is not detected. + // + // If x and y were compared before, they must be equal + // (if they were not, the recursion would have stopped); + // search the ifacePair stack for the same pair. + // + // This is a quadratic algorithm, but in practice these stacks + // are extremely short (bounded by the nesting depth of interface + // type declarations that recur via parameter types, an extremely + // rare occurrence). An alternative implementation might use a + // "visited" map, but that is probably less efficient overall. + q := &ifacePair{x, y, p} + for p != nil { + if p.identical(q) { + return true // same pair was compared before + } + p = p.prev + } + if debug { + assert(sort.IsSorted(byUniqueMethodName(a))) + assert(sort.IsSorted(byUniqueMethodName(b))) + } + for i, f := range a { + g := b[i] + if f.Id() != g.Id() || !isIdenticalInternal(f.typ, g.typ, q) { + return false + } + } + return true + } } case *Map: // Two map types are identical if they have identical key and value types. if y, ok := y.(*Map); ok { - return IsIdentical(x.key, y.key) && IsIdentical(x.elt, y.elt) + return isIdenticalInternal(x.key, y.key, p) && isIdenticalInternal(x.elt, y.elt, p) } case *Chan: // Two channel types are identical if they have identical value types // and the same direction. if y, ok := y.(*Chan); ok { - return x.dir == y.dir && IsIdentical(x.elt, y.elt) + return x.dir == y.dir && isIdenticalInternal(x.elt, y.elt, p) } case *Named: @@ -204,53 +273,14 @@ func IsIdentical(x, y Type) bool { if y, ok := y.(*Named); ok { return x.obj == y.obj } + + default: + unreachable() } return false } -// identicalTuples returns true if both tuples a and b have the -// same length and corresponding elements have identical types. -func identicalTuples(a, b *Tuple) bool { - if a.Len() != b.Len() { - return false - } - if a != nil { - for i, x := range a.vars { - y := b.vars[i] - if !IsIdentical(x.typ, y.typ) { - return false - } - } - } - return true -} - -// identicalMethods returns true if both slices a and b have the -// same length and corresponding entries have identical types. -// TODO(gri) make this more efficient (e.g., sort them on completion) -func identicalMethods(a, b []*Func) bool { - if len(a) != len(b) { - return false - } - - m := make(map[string]*Func) - for _, x := range a { - key := x.Id() - assert(m[key] == nil) // method list must not have duplicate entries - m[key] = x - } - - for _, y := range b { - key := y.Id() - if x := m[key]; x == nil || !IsIdentical(x.typ, y.typ) { - return false - } - } - - return true -} - // defaultType returns the default "typed" type for an "untyped" type; // it returns the incoming type for all other types. The default type // for untyped nil is untyped nil. diff --git a/go/types/resolver.go b/go/types/resolver.go index 023bbdc57c..7476e4d14e 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -23,7 +23,7 @@ func (check *checker) reportAltDecl(obj Object) { } } -func (check *checker) declareObj(scope *Scope, id *ast.Ident, obj Object) { +func (check *checker) declare(scope *Scope, id *ast.Ident, obj Object) { if alt := scope.Insert(obj); alt != nil { check.errorf(obj.Pos(), "%s redeclared in this block", obj.Name()) check.reportAltDecl(alt) @@ -34,17 +34,6 @@ func (check *checker) declareObj(scope *Scope, id *ast.Ident, obj Object) { } } -func (check *checker) declareFld(oset *objset, id *ast.Ident, obj Object) { - if alt := oset.insert(obj); alt != nil { - check.errorf(obj.Pos(), "%s redeclared", obj.Name()) - check.reportAltDecl(alt) - return - } - if id != nil { - check.recordObject(id, obj) - } -} - // A declInfo describes a package-level const, type, var, or func declaration. type declInfo struct { file *Scope // scope of file containing this declaration @@ -135,7 +124,7 @@ func (check *checker) resolveFiles(files []*ast.File) { return } - check.declareObj(pkg.scope, ident, obj) + check.declare(pkg.scope, ident, obj) objList = append(objList, obj) objMap[obj] = declInfo{fileScope, typ, init, nil} } @@ -227,7 +216,7 @@ func (check *checker) resolveFiles(files []*ast.File) { if obj.IsExported() { // Note: This will change each imported object's scope! // May be an issue for type aliases. - check.declareObj(fileScope, nil, obj) + check.declare(fileScope, nil, obj) check.recordImplicit(s, obj) } } @@ -241,7 +230,7 @@ func (check *checker) resolveFiles(files []*ast.File) { posSet[imp] = s.Pos() } else { // declare imported package object in file scope - check.declareObj(fileScope, nil, obj) + check.declare(fileScope, nil, obj) } case *ast.ValueSpec: @@ -326,7 +315,7 @@ func (check *checker) resolveFiles(files []*ast.File) { // ok to continue } } else { - check.declareObj(pkg.scope, d.Name, obj) + check.declare(pkg.scope, d.Name, obj) } } else { // Associate method with receiver base type name, if possible. @@ -603,7 +592,7 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, cycleOk // C A // ) // - // When we declare obj = C, typ is the identifier A which is incomplete. + // When we declare object C, typ is the identifier A which is incomplete. u := check.typ(typ, named, cycleOk) // Determine the unnamed underlying type. @@ -723,7 +712,7 @@ func (check *checker) declStmt(decl ast.Decl) { check.arityMatch(s, last) for i, name := range s.Names { - check.declareObj(check.topScope, name, lhs[i]) + check.declare(check.topScope, name, lhs[i]) } case token.VAR: @@ -756,7 +745,7 @@ func (check *checker) declStmt(decl ast.Decl) { check.arityMatch(s, nil) for i, name := range s.Names { - check.declareObj(check.topScope, name, lhs[i]) + check.declare(check.topScope, name, lhs[i]) } default: @@ -765,7 +754,7 @@ func (check *checker) declStmt(decl ast.Decl) { case *ast.TypeSpec: obj := NewTypeName(s.Name.Pos(), pkg, s.Name.Name, nil) - check.declareObj(check.topScope, s.Name, obj) + check.declare(check.topScope, s.Name, obj) check.typeDecl(obj, s.Type, nil, false) default: diff --git a/go/types/stdlib_test.go b/go/types/stdlib_test.go index 413b4204e6..7763dca659 100644 --- a/go/types/stdlib_test.go +++ b/go/types/stdlib_test.go @@ -131,7 +131,6 @@ func TestStdfixed(t *testing.T) { "bug200.go", // TODO(gri) complete duplicate checking in expr switches "bug223.go", "bug413.go", "bug459.go", // TODO(gri) complete initialization checks "bug248.go", "bug302.go", "bug369.go", // complex test instructions - ignore - "bug250.go", // TODO(gri) fix recursive interfaces "issue3924.go", // TODO(gri) && and || produce bool result (not untyped bool) "issue4847.go", // TODO(gri) initialization cycle error not found ) diff --git a/go/types/stmt.go b/go/types/stmt.go index bca5ce804a..7d9a440eb1 100644 --- a/go/types/stmt.go +++ b/go/types/stmt.go @@ -441,7 +441,7 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) { // one; i.e., if any one of them is 'used', all of them are 'used'. // Collect them for later analysis. lhsVars = append(lhsVars, obj) - check.declareObj(check.topScope, nil, obj) + check.declare(check.topScope, nil, obj) check.recordImplicit(clause, obj) } check.stmtList(inner, clause.Body) @@ -592,7 +592,7 @@ func (check *checker) stmt(ctxt stmtContext, s ast.Stmt) { // declare variables for i, ident := range idents { - check.declareObj(check.topScope, ident, vars[i]) + check.declare(check.topScope, ident, vars[i]) } } else { // ordinary assignment diff --git a/go/types/testdata/cycles1.src b/go/types/testdata/cycles1.src new file mode 100644 index 0000000000..ae2b38ebec --- /dev/null +++ b/go/types/testdata/cycles1.src @@ -0,0 +1,77 @@ +// Copyright 2013 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 + +type ( + A interface { + a() interface { + ABC1 + } + } + B interface { + b() interface { + ABC2 + } + } + C interface { + c() interface { + ABC3 + } + } + + AB interface { + A + B + } + BC interface { + B + C + } + + ABC1 interface { + A + B + C + } + ABC2 interface { + AB + C + } + ABC3 interface { + A + BC + } +) + +var ( + x1 ABC1 + x2 ABC2 + x3 ABC3 +) + +func _() { + // all types have the same method set + x1 = x2 + x2 = x1 + + x1 = x3 + x3 = x1 + + x2 = x3 + x3 = x2 + + // all methods return the same type again + x1 = x1.a() + x1 = x1.b() + x1 = x1.c() + + x2 = x2.a() + x2 = x2.b() + x2 = x2.c() + + x3 = x3.a() + x3 = x3.b() + x3 = x3.c() +} diff --git a/go/types/testdata/cycles2.src b/go/types/testdata/cycles2.src new file mode 100644 index 0000000000..3b9f473d07 --- /dev/null +++ b/go/types/testdata/cycles2.src @@ -0,0 +1,66 @@ +// Copyright 2013 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 + +// Test case for issue 5090 + +type t interface { + f(u) +} + +type u interface { + t +} + +func _() { + var t t + var u u + + t.f(t) + t.f(u) + + u.f(t) + u.f(u) +} + + +// Test case for issue 6589. + +type A interface { + a() interface { + AB + } +} + +type B interface { + a() interface { + AB + } +} + +type AB interface { + a() interface { + A + B /* ERROR a redeclared */ + } + b() interface { + A + B /* ERROR a redeclared */ + } +} + +var x AB +var y interface { + A + B /* ERROR a redeclared */ +} +var _ = x /* ERROR cannot compare */ == y + + +// Test case for issue 6638. + +type T /* ERROR cycle */ interface { + m() [T /* ERROR no field or method */ (nil).m()[0]]int +} diff --git a/go/types/testdata/cycles3.src b/go/types/testdata/cycles3.src new file mode 100644 index 0000000000..cf3e911149 --- /dev/null +++ b/go/types/testdata/cycles3.src @@ -0,0 +1,60 @@ +// Copyright 2013 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" + +var ( + _ A = A(nil).a().b().c().d().e().f() + _ A = A(nil).b().c().d().e().f() + _ A = A(nil).c().d().e().f() + _ A = A(nil).d().e().f() + _ A = A(nil).e().f() + _ A = A(nil).f() + _ A = A(nil) +) + +type ( + A interface { + a() B + B + } + + B interface { + b() C + C + } + + C interface { + c() D + D + } + + D interface { + d() E + E + } + + E interface { + e() F + F + } + + F interface { + f() A + } +) + +type ( + U /* ERROR illegal cycle */ interface { + V + } + + V interface { + v() [unsafe.Sizeof(u)]int + } +) + +var u U diff --git a/go/types/testdata/cycles4.src b/go/types/testdata/cycles4.src new file mode 100644 index 0000000000..528f205777 --- /dev/null +++ b/go/types/testdata/cycles4.src @@ -0,0 +1,68 @@ +// Copyright 2013 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 + +// Check that all methods of T are collected before +// determining the result type of m (which embeds +// all methods of T). + +type T interface { + m() interface {T} + E +} + +var _ = T.m(nil).m().e() + +type E interface { + e() int +} + +// Check that unresolved forward chains are followed +// (see also comment in resolver.go, checker.typeDecl). + +var _ = C.m(nil).m().e() + +type A B + +type B interface { + m() interface{C} + E +} + +type C A + +// Check that interface type comparison for identity +// does not recur endlessly. + +type T1 interface { + m() interface{T1} +} + +type T2 interface { + m() interface{T2} +} + +func _(x T1, y T2) { + // Checking for assignability of interfaces must check + // if all methods of x are present in y, and that they + // have identical signatures. The signatures recur via + // the result type, which is an interface that embeds + // a single method m that refers to the very interface + // that contains it. This requires cycle detection in + // identity checks for interface types. + x = y +} + +type T3 interface { + m() interface{T4} +} + +type T4 interface { + m() interface{T3} +} + +func _(x T1, y T3) { + x = y +} diff --git a/go/types/types.go b/go/types/types.go index 262272cd1a..8b54a7fc03 100644 --- a/go/types/types.go +++ b/go/types/types.go @@ -4,7 +4,10 @@ package types -import "go/ast" +import ( + "go/ast" + "sort" +) // TODO(gri) Revisit factory functions - make sure they have all relevant parameters. @@ -238,20 +241,25 @@ func (s *Signature) IsVariadic() bool { return s.isVariadic } // An Interface represents an interface type. type Interface struct { - methods []*Func // methods declared with or embedded in this interface - mset cachedMethodSet // method set for interface, lazily initialized + methods []*Func // explicitly declared methods + types []*Named // explicitly embedded types + + allMethods []*Func // ordered list of methods declared with or embedded in this interface (TODO(gri): replace with mset) + mset cachedMethodSet // method set for interface, lazily initialized } // NewInterface returns a new interface for the given methods. func NewInterface(methods []*Func) *Interface { - return &Interface{methods: methods} + // TODO(gri) should provide receiver to all methods + sort.Sort(byUniqueMethodName(methods)) + return &Interface{methods: methods, allMethods: methods} } // NumMethods returns the number of methods of interface t. -func (t *Interface) NumMethods() int { return len(t.methods) } +func (t *Interface) NumMethods() int { return len(t.allMethods) } // Method returns the i'th method of interface t for 0 <= i < t.NumMethods(). -func (t *Interface) Method(i int) *Func { return t.methods[i] } +func (t *Interface) Method(i int) *Func { return t.allMethods[i] } // A Map represents a map type. type Map struct { diff --git a/go/types/types_test.go b/go/types/types_test.go index 548b4006bd..ca3054883e 100644 --- a/go/types/types_test.go +++ b/go/types/types_test.go @@ -33,7 +33,8 @@ func dup(s string) testEntry { return testEntry{s, s} } -var testTypes = []testEntry{ +// types that don't depend on any other type declarations +var independentTestTypes = []testEntry{ // basic types dup("int"), dup("float32"), @@ -89,8 +90,7 @@ var testTypes = []testEntry{ // interfaces dup("interface{}"), dup("interface{m()}"), - dup(`interface{m(int) float32; String() string}`), - // TODO(gri) add test for interface w/ anonymous field + dup(`interface{String() string; m(int) float32}`), // maps dup("map[string]int"), @@ -102,9 +102,21 @@ var testTypes = []testEntry{ dup("<-chan []func() int"), } +// types that depend on other type declarations (src in TestTypes) +var dependentTestTypes = []testEntry{ + // interfaces + dup(`interface{io.Reader; io.Writer}`), + dup(`interface{m() int; io.Writer}`), + {`interface{m() interface{T}}`, `interface{m() interface{p.T}}`}, +} + func TestTypes(t *testing.T) { - for _, test := range testTypes { - src := "package p; type T " + test.src + var tests []testEntry + tests = append(tests, independentTestTypes...) + tests = append(tests, dependentTestTypes...) + + for _, test := range tests { + src := `package p; import "io"; type _ io.Writer; type T ` + test.src pkg, err := makePkg(t, src) if err != nil { t.Errorf("%s: %s", src, err) diff --git a/go/types/typexpr.go b/go/types/typexpr.go index 5265490ea3..865f7af85b 100644 --- a/go/types/typexpr.go +++ b/go/types/typexpr.go @@ -9,6 +9,7 @@ package types import ( "go/ast" "go/token" + "sort" "strconv" "code.google.com/p/go.tools/go/exact" @@ -67,8 +68,9 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool) } x.val = check.iota } else { - x.val = obj.val // may be nil if we don't know the constant value + x.val = obj.val } + assert(x.val != nil) x.mode = constant case *TypeName: @@ -166,6 +168,7 @@ func (check *checker) funcType(recv *ast.FieldList, ftyp *ast.FuncType, def *Nam if T.obj.pkg != check.pkg { err = "type not defined in this package" } else { + // TODO(gri) This is not correct if the underlying type is unknown yet. switch u := T.underlying.(type) { case *Basic: // unsafe.Pointer is treated like a regular pointer @@ -297,15 +300,7 @@ func (check *checker) typInternal(e ast.Expr, def *Named, cycleOk bool) Type { return check.funcType(nil, e, def) case *ast.InterfaceType: - typ := new(Interface) - var recv Type = typ - if def != nil { - def.underlying = typ - recv = def // use named receiver type if available - } - - typ.methods = check.collectMethods(recv, e.Methods, cycleOk) - return typ + return check.interfaceType(e, def, cycleOk) case *ast.MapType: typ := new(Map) @@ -319,6 +314,7 @@ func (check *checker) typInternal(e ast.Expr, def *Named, cycleOk bool) Type { // spec: "The comparison operators == and != must be fully defined // for operands of the key type; thus the key type must not be a // function, map, or slice." + // TODO(gri) if the key type is not fully defined yet, this test will be incorrect if !isComparable(typ.key) { check.errorf(e.Key.Pos(), "invalid map key type %s", typ.key) // ok to continue @@ -385,13 +381,13 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO } } typ := check.typ(ftype, nil, true) - // the parser ensures that f.Tag is nil and we don't - // care if a constructed AST contains a non-nil tag + // The parser ensures that f.Tag is nil and we don't + // care if a constructed AST contains a non-nil tag. if len(field.Names) > 0 { // named parameter for _, name := range field.Names { par := NewParam(name.Pos(), check.pkg, name.Name, typ) - check.declareObj(scope, name, par) + check.declare(scope, name, par) params = append(params, par) } } else { @@ -411,57 +407,152 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO return } -func (check *checker) collectMethods(recv Type, list *ast.FieldList, cycleOk bool) (methods []*Func) { - if list == nil { - return nil +func (check *checker) declareInSet(oset *objset, pos token.Pos, id *ast.Ident, obj Object) bool { + if alt := oset.insert(obj); alt != nil { + check.errorf(pos, "%s redeclared", obj.Name()) + check.reportAltDecl(alt) + return false + } + if id != nil { + check.recordObject(id, obj) + } + return true +} + +func (check *checker) interfaceType(ityp *ast.InterfaceType, def *Named, cycleOk bool) *Interface { + iface := new(Interface) + if def != nil { + def.underlying = iface } - var mset objset + // empty interface: common case + if ityp.Methods == nil { + return iface + } - for _, f := range list.List { - typ := check.typ(f.Type, nil, cycleOk) - // the parser ensures that f.Tag is nil and we don't - // care if a constructed AST contains a non-nil tag + // The parser ensures that field tags are nil and we don't + // care if a constructed AST contains non-nil tags. + + // 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 { - // methods (the parser ensures that there's only one - // and we don't care if a constructed AST has more) - sig, _ := typ.(*Signature) - if sig == nil { - check.invalidAST(f.Type.Pos(), "%s is not a method signature", typ) - continue - } - sig.recv = NewVar(token.NoPos, check.pkg, "", recv) - for _, name := range f.Names { - m := NewFunc(name.Pos(), check.pkg, name.Name, sig) - check.declareFld(&mset, name, m) - methods = append(methods, m) + // The parser ensures that there's only one method + // and we don't care if a constructed AST has more. + name := f.Names[0] + pos := name.Pos() + // Don't type-check signature yet - use an + // empty signature now and update it later. + m := NewFunc(pos, check.pkg, name.Name, new(Signature)) + if check.declareInSet(&mset, pos, name, m) { + iface.methods = append(iface.methods, m) + iface.allMethods = append(iface.allMethods, m) + signatures = append(signatures, f.Type) } } else { - // embedded interface - switch t := typ.Underlying().(type) { - case nil: - // The underlying type is in the process of being defined - // but we need it in order to complete this type. For now - // complain with an "unimplemented" error. This requires - // a bit more work. - // TODO(gri) finish this. - check.errorf(f.Type.Pos(), "reference to incomplete type %s - unimplemented", f.Type) - case *Interface: - for _, m := range t.methods { - check.declareFld(&mset, nil, m) - methods = append(methods, m) - } - default: - if t != Typ[Invalid] { - check.errorf(f.Type.Pos(), "%s is not an interface type", typ) - } + // embedded type + embedded = append(embedded, f.Type) + } + } + + // 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.typ(e, nil, cycleOk) + if typ == Typ[Invalid] { + continue + } + named, _ := typ.(*Named) + if named == nil { + check.invalidAST(pos, "%s is not named type", typ) + continue + } + // determine underlying (possibly incomplete) type + // by following its forward chain + // TODO(gri) should this be part of Underlying()? + u := named.underlying + for { + n, _ := u.(*Named) + if n == nil { + break + } + u = n.underlying + } + if u == Typ[Invalid] { + continue + } + embed, _ := u.(*Interface) + if embed == nil { + check.errorf(pos, "%s is not an interface", named) + continue + } + iface.types = append(iface.types, named) + // collect embedded methods + for _, m := range embed.allMethods { + if check.declareInSet(&mset, pos, nil, m) { + iface.allMethods = append(iface.allMethods, m) } } } - return + // 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. + + // determine receiver type + var recv Type = iface + if def != nil { + def.underlying = iface + recv = def // use named receiver type if available + } + + for i, m := range iface.methods { + expr := signatures[i] + typ := check.typ(expr, nil, true) + if typ == Typ[Invalid] { + continue // keep method with empty method signature + } + sig, _ := typ.(*Signature) + if sig == nil { + check.invalidAST(expr.Pos(), "%s is not a method signature", typ) + continue // keep method with empty method signature + } + sig.recv = NewVar(m.pos, check.pkg, "", recv) + *m.typ.(*Signature) = *sig // update signature (don't replace it!) + } + + sort.Sort(byUniqueMethodName(iface.allMethods)) + + return iface } +// byUniqueMethodName method lists can be sorted by their unique method names. +type byUniqueMethodName []*Func + +func (a byUniqueMethodName) Len() int { return len(a) } +func (a byUniqueMethodName) Less(i, j int) bool { return a[i].Id() < a[j].Id() } +func (a byUniqueMethodName) Swap(i, j int) { a[i], a[j] = a[j], a[i] } + func (check *checker) tag(t *ast.BasicLit) string { if t != nil { if t.Kind == token.STRING { @@ -492,8 +583,9 @@ func (check *checker) collectFields(list *ast.FieldList, cycleOk bool) (fields [ } fld := NewField(pos, check.pkg, name, typ, anonymous) - check.declareFld(&fset, ident, fld) - fields = append(fields, fld) + if check.declareInSet(&fset, pos, ident, fld) { + fields = append(fields, fld) + } } for _, f := range list.List { diff --git a/go/types/universe.go b/go/types/universe.go index 049f7d6dbe..63a6be11f1 100644 --- a/go/types/universe.go +++ b/go/types/universe.go @@ -67,7 +67,7 @@ func defPredeclaredTypes() { res := NewVar(token.NoPos, nil, "", Typ[String]) sig := &Signature{results: NewTuple(res)} err := NewFunc(token.NoPos, nil, "Error", sig) - typ := &Named{underlying: &Interface{methods: []*Func{err}}, complete: true} + typ := &Named{underlying: NewInterface([]*Func{err}), complete: true} sig.recv = NewVar(token.NoPos, nil, "", typ) def(NewTypeName(token.NoPos, nil, "error", typ)) }