exp/types: enable cycle checks again

Process a package's object in a reproducible
order (rather then in map order) so that we
get error messages in reproducible order.

R=r
CC=golang-dev
https://golang.org/cl/6449076

diff --git a/src/pkg/exp/types/check.go b/src/pkg/exp/types/check.go
index 9d7474a446909bd35811c690ac4b5b3b19ff4c9a..bd947a1639f9c68568ca754a6293ca77c54cb70c 100644 (file)
--- a/src/pkg/exp/types/check.go
+++ b/src/pkg/exp/types/check.go
@@ -30,6 +30,7 @@ func (c *checker) errorf(pos token.Pos, format string, args ...interface{}) stri
 
 // collectFields collects struct fields tok = token.STRUCT), interface methods
 // (tok = token.INTERFACE), and function arguments/results (tok = token.FUNC).
+//
 func (c *checker) collectFields(tok token.Token, list *ast.FieldList, cycleOk bool) (fields ObjList, tags []string, isVariadic bool) {
        if list != nil {
                for _, field := range list.List {
@@ -104,7 +105,7 @@ func (c *checker) makeType(x ast.Expr, cycleOk bool) (typ Type) {
                obj := t.Obj
                if obj == nil {
                        // unresolved identifier (error has been reported before)
-                       return &Bad{Msg: "unresolved identifier"}
+                       return &Bad{Msg: fmt.Sprintf("%s is unresolved", t.Name)}
                }
                if obj.Kind != ast.Typ {
                        msg := c.errorf(t.Pos(), "%s is not a type", t.Name)
@@ -112,10 +113,7 @@ func (c *checker) makeType(x ast.Expr, cycleOk bool) (typ Type) {
                }
                c.checkObj(obj, cycleOk)
                if !cycleOk && obj.Type.(*Name).Underlying == nil {
-                       // TODO(gri) Enable this message again once its position
-                       // is independent of the underlying map implementation.
-                       // msg := c.errorf(obj.Pos(), "illegal cycle in declaration of %s", obj.Name)
-                       msg := "illegal cycle"
+                       msg := c.errorf(obj.Pos(), "illegal cycle in declaration of %s", obj.Name)
                        return &Bad{Msg: msg}
                }
                return obj.Type.(Type)
@@ -227,11 +225,25 @@ func (c *checker) checkObj(obj *ast.Object, ref bool) {
 // there are errors.
 //
 func Check(fset *token.FileSet, pkg *ast.Package) (types map[ast.Expr]Type, err error) {
+       // Sort objects so that we get reproducible error
+       // positions (this is only needed for testing).
+       // TODO(gri): Consider ast.Scope implementation that
+       // provides both a list and a map for fast lookup.
+       // Would permit the use of scopes instead of ObjMaps
+       // elsewhere.
+       list := make(ObjList, len(pkg.Scope.Objects))
+       i := 0
+       for _, obj := range pkg.Scope.Objects {
+               list[i] = obj
+               i++
+       }
+       list.Sort()
+
        var c checker
        c.fset = fset
        c.types = make(map[ast.Expr]Type)
 
-       for _, obj := range pkg.Scope.Objects {
+       for _, obj := range list {
                c.checkObj(obj, false)
        }
 

diff --git a/src/pkg/exp/types/testdata/test0.src b/src/pkg/exp/types/testdata/test0.src
index 876573481b96c0c4f9c6b074d17b34860457c244..a770a19b803a5b7e92798f65185b954981d7d45d 100644 (file)
--- a/src/pkg/exp/types/testdata/test0.src
+++ b/src/pkg/exp/types/testdata/test0.src
@@ -44,15 +44,15 @@ type (
 type (
        Pi pi /* ERROR "not a type" */
 
-       a /* DISABLED "illegal cycle" */ a
+       a /* ERROR "illegal cycle" */ a
        a /* ERROR "redeclared" */ int
 
        // where the cycle error appears depends on the
        // order in which declarations are processed
        // (which depends on the order in which a map
        // is iterated through)
-       b c
-       c /* DISABLED "illegal cycle" */ d
+       b /* ERROR "illegal cycle" */ c
+       c d
        d e
        e b
 
@@ -79,13 +79,13 @@ type (
        S3 struct {
                x S2
        }
-       S4/* DISABLED "illegal cycle" */ struct {
+       S4/* ERROR "illegal cycle" */ struct {
                S4
        }
-       S5 struct {
+       S5 /* ERROR "illegal cycle" */ struct {
                S6
        }
-       S6 /* DISABLED "illegal cycle" */ struct {
+       S6 struct {
                field S7
        }
        S7 struct {
@@ -96,8 +96,8 @@ type (
        L2 []int
 
        A1 [10]int
-       A2 /* DISABLED "illegal cycle" */ [10]A2
-       A3 /* DISABLED "illegal cycle" */ [10]struct {
+       A2 /* ERROR "illegal cycle" */ [10]A2
+       A3 /* ERROR "illegal cycle" */ [10]struct {
                x A4
        }
        A4 [10]A3
@@ -132,17 +132,21 @@ type (
                I1
                I1
        }
-       I8 /* DISABLED "illegal cycle" */ interface {
+       I8 /* ERROR "illegal cycle" */ interface {
                I8
        }
-       I9 /* DISABLED "illegal cycle" */ interface {
+       // 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 {
                I10
        }
        I10 interface {
                I11
        }
        I11 interface {
-               I9
+               I09
        }
 
        C1 chan int