go.tools/go/types: compute correct initialization order

- Replaced check.initDependencies with check.initOrder;
  this is the only semantic change, it affects only the
  value of Info.InitOrder.
- Added additional init order test cases and adjusted
  existing tests.
- Moved orderedSetObjects from resolver.go to ordering.go.

Fixes golang/go#7964.

LGTM=adonovan
R=adonovan
CC=golang-codereviews
https://golang.org/cl/91450043

go/ssa/interp/testdata/coverage.go

@@ -354,7 +354,7 @@ var a, b = create(1), create(2)
 
 // Initialization order of package-level value specs.
 func init() {
-	if x := fmt.Sprint(order); x != "[2 3 1]" {
+	if x := fmt.Sprint(order); x != "[1 2 3]" {
 		panic(x)
 	}
 	if c != 3 {

go/ssa/interp/testdata/initorder.go

@@ -26,9 +26,9 @@ func main() {
 	// - {f,b,c/d,e} < order  (ref graph traversal)
 	// - order < {a}          (lexical order)
 	// - b < c/d < e < f      (lexical order)
-	// Solution: b c/d e f a
+	// Solution: a b c/d e f
 	abcdef := [6]int{a, b, c, d, e, f}
-	if abcdef != [6]int{5, 0, 1, 2, 3, 4} {
+	if abcdef != [6]int{0, 1, 2, 3, 4, 5} {
 		panic(abcdef)
 	}
 }

go/types/api_test.go

@@ -406,7 +406,31 @@ func TestInitOrderInfo(t *testing.T) {
 			"y = 1", "x = T.m",
 		}},
 		{`package p10; var (d = c + b; a = 0; b = 0; c = 0)`, []string{
-			"b = 0", "c = 0", "d = c + b", "a = 0",
+			"a = 0", "b = 0", "c = 0", "d = c + b",
+		}},
+		{`package p11; var (a = e + c; b = d + c; c = 0; d = 0; e = 0)`, []string{
+			"c = 0", "d = 0", "b = d + c", "e = 0", "a = e + c",
+		}},
+		// emit an initializer for n:1 initializations only once (not for each node
+		// on the lhs which may appear in different order in the dependency graph)
+		{`package p12; var (a = x; b = 0; x, y = m[0]; m map[int]int)`, []string{
+			"b = 0", "x, y = m[0]", "a = x",
+		}},
+		// test case from spec section on package initialization
+		{`package p12
+
+		var (
+			a = c + b
+			b = f()
+			c = f()
+			d = 3
+		)
+
+		func f() int {
+			d++
+			return d
+		}`, []string{
+			"d = 3", "b = f()", "c = f()", "a = c + b",
 		}},
 		// test case for issue 7131
 		{`package main

go/types/check.go

@@ -77,15 +77,14 @@ type checker struct {
 	indent int // indentation for tracing
 }
 
-// addDeclDep adds the dependency edge (check.decl -> to)
-// if check.decl exists and to has an initializer.
+// addDeclDep adds the dependency edge (check.decl -> to) if check.decl exists
 func (check *checker) addDeclDep(to Object) {
 	from := check.decl
 	if from == nil {
 		return // not in a package-level init expression
 	}
-	if decl := check.objMap[to]; decl == nil || !decl.hasInitializer() {
-		return // to is not a package-level object or has no initializer
+	if _, found := check.objMap[to]; !found {
+		return // to is not a package-level object
 	}
 	from.addDep(to)
 }
@@ -189,7 +188,7 @@ func (check *checker) initFiles(files []*ast.File) {
 	}
 }
 
-// A bailout panic is raised to indicate early termination.
+// A bailout panic is used for early termination.
 type bailout struct{}
 
 func (check *checker) handleBailout(err *error) {
@@ -211,13 +210,11 @@ func (check *checker) Files(files []*ast.File) (err error) {
 
 	check.collectObjects()
 
-	objList := check.resolveOrder()
-
-	check.packageObjects(objList)
+	check.packageObjects(check.resolveOrder())
 
 	check.functionBodies()
 
-	check.initDependencies(objList)
+	check.initOrder()
 
 	check.unusedImports()
 

go/types/initorder.go

@@ -0,0 +1,218 @@
+// 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.
+
+package types
+
+import (
+	"container/heap"
+	"fmt"
+)
+
+// initOrder computes the Info.InitOrder for package variables.
+func (check *checker) initOrder() {
+	// compute the object dependency graph and
+	// initialize a priority queue with the list
+	// of graph nodes
+	pq := nodeQueue(dependencyGraph(check.objMap))
+	heap.Init(&pq)
+
+	const debug = false
+	if debug {
+		fmt.Printf("package %s: object dependency graph\n", check.pkg.Name())
+		for _, n := range pq {
+			for _, o := range n.out {
+				fmt.Printf("\t%s -> %s\n", n.obj.Name(), o.obj.Name())
+			}
+		}
+		fmt.Println()
+		fmt.Printf("package %s: initialization order\n", check.pkg.Name())
+	}
+
+	// determine initialization order by removing the highest priority node
+	// (the one with the fewest dependencies) and its edges from the graph,
+	// repeatedly, until there are no nodes left.
+	// In a valid Go program, those nodes always have zero dependencies (after
+	// removing all incoming dependencies), otherwise there are initialization
+	// cycles.
+	mark := 0
+	emitted := make(map[*declInfo]bool)
+	for len(pq) > 0 {
+		// get the next node
+		n := heap.Pop(&pq).(*objNode)
+
+		// if n still depends on other nodes, we have a cycle
+		if n.in > 0 {
+			mark++ // mark nodes using a different value each time
+			cycle := findPath(n, n, mark)
+			if i := valIndex(cycle); i >= 0 {
+				check.reportCycle(cycle, i)
+			}
+			// ok to continue, but the variable initialization order
+			// will be incorrect at this point since it assumes no
+			// cycle errors
+		}
+
+		// reduce dependency count of all dependent nodes
+		// and update priority queue
+		for _, out := range n.out {
+			out.in--
+			heap.Fix(&pq, out.index)
+		}
+
+		// record the init order for variables with initializers only
+		v, _ := n.obj.(*Var)
+		info := check.objMap[v]
+		if v == nil || !info.hasInitializer() {
+			continue
+		}
+
+		// n:1 variable declarations such as: a, b = f()
+		// introduce a node for each lhs variable (here: a, b);
+		// but they all have the same initializer - emit only
+		// one, for the first variable seen
+		if emitted[info] {
+			continue // initializer already emitted, if any
+		}
+		emitted[info] = true
+
+		infoLhs := info.lhs // possibly nil (see declInfo.lhs field comment)
+		if infoLhs == nil {
+			infoLhs = []*Var{v}
+		}
+		init := &Initializer{infoLhs, info.init}
+		check.Info.InitOrder = append(check.Info.InitOrder, init)
+
+		if debug {
+			fmt.Printf("\t%s\n", init)
+		}
+	}
+
+	if debug {
+		fmt.Println()
+	}
+}
+
+// findPath returns the (reversed) list of nodes z, ... c, b, a,
+// such that there is a path (list of edges) from a to z.
+// If there is no such path, the result is nil.
+// Nodes marked with the value mark are considered "visited";
+// unvisited nodes are marked during the graph search.
+func findPath(a, z *objNode, mark int) []*objNode {
+	if a.mark == mark {
+		return nil // node already seen
+	}
+	a.mark = mark
+
+	for _, n := range a.out {
+		if n == z {
+			return []*objNode{z}
+		}
+		if P := findPath(n, z, mark); P != nil {
+			return append(P, n)
+		}
+	}
+
+	return nil
+}
+
+// valIndex returns the index of the first constant or variable in a,
+// if any; or a value < 0.
+func valIndex(a []*objNode) int {
+	for i, n := range a {
+		switch n.obj.(type) {
+		case *Const, *Var:
+			return i
+		}
+	}
+	return -1
+}
+
+// reportCycle reports an error for the cycle starting at i.
+func (check *checker) reportCycle(cycle []*objNode, i int) {
+	obj := cycle[i].obj
+	check.errorf(obj.Pos(), "initialization cycle for %s", obj.Name())
+	// print cycle
+	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].obj
+	}
+	check.errorf(obj.Pos(), "\t%s", obj.Name())
+}
+
+// An objNode represents a node in the object dependency graph.
+// Each node b in a.out represents an edge a->b indicating that
+// b depends on a.
+// Nodes may be marked for cycle detection. A node n is marked
+// if n.mark corresponds to the current mark value.
+type objNode struct {
+	obj   Object     // object represented by this node
+	in    int        // number of nodes this node depends on
+	out   []*objNode // list of nodes that depend on this node
+	index int        // node index in list of nodes
+	mark  int        // for cycle detection
+}
+
+// dependencyGraph computes the transposed object dependency graph
+// from the given objMap. The transposed graph is returned as a list
+// of nodes; an edge d->n indicates that node n depends on node d.
+func dependencyGraph(objMap map[Object]*declInfo) []*objNode {
+	// M maps each object to its corresponding node
+	M := make(map[Object]*objNode, len(objMap))
+	for obj := range objMap {
+		M[obj] = &objNode{obj: obj}
+	}
+
+	// G is the graph of nodes n
+	G := make([]*objNode, len(M))
+	i := 0
+	for obj, n := range M {
+		deps := objMap[obj].deps
+		n.in = len(deps)
+		for d := range deps {
+			d := M[d]                // node n depends on node d
+			d.out = append(d.out, n) // add edge d->n
+		}
+
+		G[i] = n
+		n.index = i
+		i++
+	}
+
+	return G
+}
+
+// nodeQueue implements the container/heap interface;
+// a nodeQueue may be used as a priority queue.
+type nodeQueue []*objNode
+
+func (a nodeQueue) Len() int { return len(a) }
+
+func (a nodeQueue) Swap(i, j int) {
+	x, y := a[i], a[j]
+	a[i], a[j] = y, x
+	x.index, y.index = j, i
+}
+
+func (a nodeQueue) Less(i, j int) bool {
+	x, y := a[i], a[j]
+	// nodes are prioritized by number of incoming dependencies (1st key)
+	// and source positions (2nd key)
+	return x.in < y.in || x.in == y.in && x.obj.Pos() < y.obj.Pos()
+}
+
+func (a *nodeQueue) Push(x interface{}) {
+	panic("unreachable")
+}
+
+func (a *nodeQueue) Pop() interface{} {
+	n := len(*a)
+	x := (*a)[n-1]
+	x.index = -1 // for safety
+	*a = (*a)[:n-1]
+	return x
+}

go/types/ordering.go

@@ -106,3 +106,22 @@ func (check *checker) appendInPostOrder(order *[]Object, obj Object) {
 
 	*order = append(*order, obj)
 }
+
+func orderedSetObjects(set map[Object]bool) []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].Pos() < a[j].Pos() }
+func (a inSourceOrder) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }

go/types/resolver.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
-	"sort"
 	"strconv"
 	"strings"
 	"unicode"
@@ -350,7 +349,7 @@ func (check *checker) collectObjects() {
 	}
 }
 
-// packageObjects typechecks all package objects in check.objMap, but not function bodies.
+// packageObjects typechecks all package objects in objList, but not function bodies.
 func (check *checker) packageObjects(objList []Object) {
 	// add new methods to already type-checked types (from a prior Checker.Files call)
 	for _, obj := range objList {
@@ -380,21 +379,6 @@ func (check *checker) functionBodies() {
 	}
 }
 
-// initDependencies computes initialization dependencies.
-func (check *checker) initDependencies(objList []Object) {
-	// pre-allocate space for initialization paths so that the underlying array is reused
-	initPath := make([]Object, 0, 8)
-
-	for _, obj := range objList {
-		switch obj.(type) {
-		case *Const, *Var:
-			if check.objMap[obj].hasInitializer() {
-				check.dependencies(obj, initPath)
-			}
-		}
-	}
-}
-
 // unusedImports checks for unused imports.
 func (check *checker) unusedImports() {
 	// if function bodies are not checked, packages' uses are likely missing - don't check
@@ -436,102 +420,3 @@ func (check *checker) unusedImports() {
 		}
 	}
 }
-
-func orderedSetObjects(set map[Object]bool) []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].Pos() < a[j].Pos() }
-func (a inSourceOrder) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
-
-// dependencies recursively traverses the initialization dependency graph in a depth-first
-// 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.
-//
-// Path contains all nodes on the path to the current node obj (excluding obj).
-//
-// To detect cyles, the nodes are marked as follows: Initially, all nodes are unmarked
-// (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 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, path []Object) {
-	init := check.objMap[obj]
-	if init.mark < 0 {
-		return // finished
-	}
-
-	if init.mark > 0 {
-		// cycle detected - find index of first constant or variable in cycle, if any
-		first := -1
-		cycle := path[init.mark-1:]
-	L:
-		for i, obj := range cycle {
-			switch obj.(type) {
-			case *Const, *Var:
-				first = i
-				break L
-			}
-		}
-		// only report an error if there's at least one constant or 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", obj.Name())
-
-		}
-		init.mark = -1 // avoid further errors
-		return
-	}
-
-	// init.mark == 0
-
-	path = append(path, obj) // len(path) > 0
-	init.mark = len(path)    // init.mark > 0
-	for _, obj := range orderedSetObjects(init.deps) {
-		check.dependencies(obj, path)
-	}
-	init.mark = -1 // init.mark < 0
-
-	// record the init order for variables only
-	if this, _ := obj.(*Var); this != nil {
-		initLhs := init.lhs // possibly nil (see declInfo.lhs field comment)
-		if initLhs == nil {
-			initLhs = []*Var{this}
-		}
-		check.Info.InitOrder = append(check.Info.InitOrder, &Initializer{initLhs, init.init})
-	}
-}

go/types/testdata/init0.src

@@ -70,7 +70,7 @@ var x4 = x5
 var x5 /* ERROR initialization cycle */ = f1()
 func f1() int { return x5*10 }
 
-var x6 /* ERROR initialization cycle */ , x7 = f2()
+var x6, x7 /* ERROR initialization cycle */ = f2()
 var x8 = x7
 func f2() (int, int) { return f3() + f3(), 0 }
 func f3() int { return x8 }