go.tools/go/types: init order depends on source order

Apply sorting consistently to object->decl maps.

Fixes golang/go#7131.

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

go/types/api_test.go

@@ -349,6 +349,21 @@ func TestInitOrder(t *testing.T) {
 		{`package p9; type T struct{}; func (T) m() int { _ = y; return 0 }; var x, y = T.m, 1`, []string{
 			"y = 1", "x = T.m",
 		}},
+		// test case for issue 7131
+		{`package main
+
+		var counter int
+		func next() int { counter++; return counter }
+
+		var _ = makeOrder()
+		func makeOrder() []int { return []int{f, b, d, e, c, a} }
+
+		var a       = next()
+		var b, c    = next(), next()
+		var d, e, f = next(), next(), next()
+		`, []string{
+			"a = next()", "b = next()", "c = next()", "d = next()", "e = next()", "f = next()", "_ = makeOrder()",
+		}},
 	}
 
 	for _, test := range tests {

go/types/resolver.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
+	"sort"
 	"strconv"
 	"strings"
 	"unicode"
@@ -114,13 +115,12 @@ func (check *checker) resolveFiles(files []*ast.File) {
 	//          base type names.
 
 	var (
-		objList []Object                     // list of package-level objects, in source order
 		objMap  = make(map[Object]*declInfo) // corresponding declaration info
 		initMap = make(map[Object]*declInfo) // declaration info for variables with initializers
 	)
 
 	// declare declares obj in the package scope, records its ident -> obj mapping,
-	// and updates objList and objMap. The object must not be a function or method.
+	// and updates objMap. The object must not be a function or method.
 	declare := func(ident *ast.Ident, obj Object, d *declInfo) {
 		assert(ident.Name == obj.Name())
 
@@ -132,7 +132,6 @@ func (check *checker) resolveFiles(files []*ast.File) {
 		}
 
 		check.declare(pkg.scope, ident, obj)
-		objList = append(objList, obj)
 		objMap[obj] = d
 	}
 
@@ -356,7 +355,6 @@ func (check *checker) resolveFiles(files []*ast.File) {
 						}
 					}
 				}
-				objList = append(objList, obj)
 				info := &declInfo{file: fileScope, fdecl: d}
 				objMap[obj] = info
 				// remember obj if it has a body (= initializer)
@@ -381,16 +379,15 @@ func (check *checker) resolveFiles(files []*ast.File) {
 		}
 	}
 
-	// Phase 3: Typecheck all objects in objList, but not function bodies.
+	// Phase 3: Typecheck all objects in objMap, but not function bodies.
 
 	check.objMap = objMap // indicate that we are checking package-level declarations (objects may not have a type yet)
 	check.initMap = initMap
-	for _, obj := range objList {
+	for _, obj := range objectsOf(check.objMap) {
 		if obj.Type() == nil {
 			check.objDecl(obj, nil, false)
 		}
 	}
-	check.objMap = nil // done with package-level declarations
 
 	// At this point we may have a non-empty check.methods map; this means that not all
 	// entries were deleted at the end of typeDecl because the respective receiver base
@@ -409,9 +406,7 @@ func (check *checker) resolveFiles(files []*ast.File) {
 	// Note: must happen after checking all functions because function bodies
 	// may introduce dependencies
 
-	// For source order and reproducible error messages,
+	for _, obj := range objectsOf(initMap) {
-	// iterate through objList rather than initMap.
-	for _, obj := range objList {
 		if obj, _ := obj.(*Var); obj != nil {
 			if init := initMap[obj]; init != nil {
 				// provide non-nil path for re-use of underlying array
@@ -419,7 +414,6 @@ func (check *checker) resolveFiles(files []*ast.File) {
 			}
 		}
 	}
-	objList = nil       // not needed anymore
 	check.initMap = nil // not needed anymore
 
 	// Phase 6: Check for declared but not used packages and function variables.
@@ -467,6 +461,30 @@ func (check *checker) resolveFiles(files []*ast.File) {
 	}
 }
 
+// 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.
+
+// objectsOf returns the keys of m in source order.
+func objectsOf(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
+}
+
+// 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.
@@ -530,7 +548,8 @@ func (check *checker) dependencies(obj Object, init *declInfo, path []Object) {
 
 	path = append(path, obj) // len(path) > 0
 	init.mark = len(path)    // init.mark > 0
-	for obj, dep := range init.deps {
+	for _, obj := range objectsOf(init.deps) {
+		dep := init.deps[obj]
 		check.dependencies(obj, dep, path)
 	}
 	init.mark = -1 // init.mark < 0