go.tools/go/types: print initialization cycles

Also: Always report cycle at first (in source order)
variable in the cycle.

R=adonovan, gri
CC=golang-dev
https://golang.org/cl/23370043

go/types/check_test.go

@@ -224,10 +224,9 @@ func checkFiles(t *testing.T, testfiles []string) {
 			t.Error(err)
 			return
 		}
-		// Ignore error messages containing "other declaration":
-		// They are follow-up error messages after a redeclaration
-		// error.
-		if !strings.Contains(err.Error(), "other declaration") {
+		// Ignore secondary error messages starting with "\t";
+		// they are clarifying messages for a primary error.
+		if !strings.Contains(err.Error(), ": \t") {
 			errlist = append(errlist, err)
 		}
 	}

go/types/resolver.go

@@ -19,7 +19,7 @@ func (check *checker) reportAltDecl(obj Object) {
 		// We use "other" rather than "previous" here because
 		// the first declaration seen may not be textually
 		// earlier in the source.
-		check.errorf(pos, "other declaration of %s", obj.Name())
+		check.errorf(pos, "\tother declaration of %s", obj.Name()) // secondary error, \t indented
 	}
 }
 
@@ -420,7 +420,8 @@ func (check *checker) resolveFiles(files []*ast.File) {
 	for _, obj := range objList {
 		if obj, _ := obj.(*Var); obj != nil {
 			if init := initMap[obj]; init != nil {
-				check.dependencies(obj, init, nil, 0)
+				// provide non-nil path for re-use of underlying array
+				check.dependencies(obj, init, make([]Object, 0, 8))
 			}
 		}
 	}
@@ -494,54 +495,72 @@ func (check *checker) resolveFiles(files []*ast.File) {
 // manner and appends the encountered variables in postorder to the Info.InitOrder list.
 // As a result, that list ends up being sorted topologically in the order of dependencies.
 //
-// The current node is represented by the pair (obj, init); last is the last variable seen
-// on the path to the current node; and n is the number of variables seen on the path before
-// reaching the current node.
+// The current node is represented by the pair (obj, init); and path contains all nodes
+// on the path to the current node (excluding the current node).
 //
 // To detect cyles, the nodes are marked as follows: Initially, all nodes are unmarked
-// (declInfo.mark == 0). On the way down, a node is marked with a value > 0 ("in progress"),
-// and on the way up, a node is marked with a value < 0 ("finished"). A cycle is detected
-// if traversal reaches a node that is marked "in progress".
+// (declInfo.mark == 0). On the way down, a node is appended to the path, and the node
+// is marked with a value > 0 ("in progress"). On the way up, a node is marked with a
+// value < 0 ("finished"). A cycle is detected if a node is reached that is marked as
+// "in progress".
 //
-// A cycle must contain at least one variable to be invalid (cycles containing only functions
-// are permitted). To detect such cycles, the "in progress" value is the number of variables
-// seen on the path to a specific node (including that node). That value is always > 0 because
-// the root nodes are variables. If the mark values are different when a cycle is detected,
-// the difference is the number of variables in the cycle, and the last variable seen on the
-// path must be a variable in the cycle.
+// A cycle must contain at least one variable to be invalid (cycles containing only
+// functions are permitted). To detect such a cycle, and in order to print it, the
+// mark value indicating "in progress" is the path length up to (and including) the
+// current node; i.e. the length of the path after appending the node. Naturally,
+// that value is > 0 as required for "in progress" marks. In other words, each node's
+// "in progress" mark value corresponds to the node's path index plus 1. Accordingly,
+// when the first node of a cycle is reached, that node's mark value indicates the
+// start of the cycle in the path. The tail of the path (path[mark-1:]) contains all
+// nodes of the cycle.
 //
-func (check *checker) dependencies(obj Object, init *declInfo, last *Var, n int) {
+func (check *checker) dependencies(obj Object, init *declInfo, path []Object) {
 	if init.mark < 0 {
 		return // finished
 	}
 
-	this, _ := obj.(*Var)
-	if this != nil {
-		last = this // this is the last variable seen on the path
-		n++         // count the variable
-	}
-
 	if init.mark > 0 {
-		// cycle detected
-		if init.mark != n {
-			// cycle contains variables - use the last one seen for error
-			check.errorf(last.pos, "initialization cycle for %s", last.name)
-			// TODO(gri) print the cycle
-			init.mark = -1 // avoid further errors
+		// cycle detected - find index of first variable in cycle, if any
+		first := -1
+		cycle := path[init.mark-1:]
+		for i, obj := range cycle {
+			if _, ok := obj.(*Var); ok {
+				first = i
+				break
+			}
 		}
+		// only report an error if there's at least one variable
+		if first >= 0 {
+			obj := cycle[first]
+			check.errorf(obj.Pos(), "initialization cycle for %s", obj.Name())
+			// print cycle
+			i := first
+			for _ = range cycle {
+				check.errorf(obj.Pos(), "\t%s refers to", obj.Name()) // secondary error, \t indented
+				i++
+				if i >= len(cycle) {
+					i = 0
+				}
+				obj = cycle[i]
+			}
+			check.errorf(obj.Pos(), "\t%s (cycle start)", obj.Name())
+
+		}
+		init.mark = -1 // avoid further errors
 		return
 	}
 
 	// init.mark == 0
 
-	init.mark = n // init.mark > 0
+	path = append(path, obj) // len(path) > 0
+	init.mark = len(path)    // init.mark > 0
 	for obj, dep := range init.deps {
-		check.dependencies(obj, dep, last, n)
+		check.dependencies(obj, dep, path)
 	}
 	init.mark = -1 // init.mark < 0
 
 	// record the init order for variables
-	if this != nil {
+	if this, _ := obj.(*Var); this != nil {
 		initLhs := init.lhs // possibly nil (see declInfo.lhs field comment)
 		if initLhs == nil {
 			initLhs = []*Var{this}

go/types/testdata/init0.src

@@ -9,6 +9,8 @@ package init0
 // type-checking, not initialization cycles (we don't know the types)
 // (avoid duplicate errors)
 var (
+	x0 /* ERROR cycle */ = x0
+
 	x1 /* ERROR cycle */ = y1
 	y1 = x1
 
@@ -20,6 +22,8 @@ var (
 
 // initialization cycles (we know the types)
 var (
+	x00 /* ERROR initialization cycle */ int = x00
+
 	x2 /* ERROR initialization cycle */ int = y2
 	y2 = x2
 
@@ -42,7 +46,7 @@ var x4 = x5
 var x5 /* ERROR initialization cycle */ = f1()
 func f1() int { return x5*10 }
 
-var x6, x7 /* ERROR initialization cycle */ = f2()
+var x6 /* ERROR initialization cycle */ , x7 = f2()
 var x8 = x7
 func f2() (int, int) { return f3() + f3(), 0 }
 func f3() int { return x8 }