go/types: callback for *ast.Ident -> Object mapping

Also re-enabled resolver test.

R=adonovan
CC=golang-dev
https://golang.org/cl/7107043

src/pkg/go/types/api.go

@@ -20,9 +20,16 @@ type Context struct {
 	PtrSize int64 // size in bytes of pointers
 
 	// If Error is not nil, it is called with each error found
-	// during type checking.
+	// during type checking. Most error messages have accurate
+	// position information; those error strings are formatted
+	// filename:line:column: message.
 	Error func(err error)
 
+	// If Ident is not nil, it is called for each identifier
+	// id that is type-checked: obj is the object denoted by
+	// the identifier.
+	Ident func(id *ast.Ident, obj Object)
+
 	// If Expr is not nil, it is called for each expression x that is
 	// type-checked: typ is the expression type, and val is the value
 	// if x is constant, val is nil otherwise.

src/pkg/go/types/check.go

@@ -32,6 +32,21 @@ type checker struct {
 	pos       []token.Pos                       // stack of expr positions; debugging support, used if trace is set
 }
 
+func (check *checker) register(id *ast.Ident, obj Object) {
+	// When an expression is evaluated more than once (happens
+	// in rare cases, e.g. for statement expressions, see
+	// comment in stmt.go), the object has been registered
+	// before. Don't do anything in that case.
+	if alt := check.idents[id]; alt != nil {
+		assert(alt == obj)
+		return
+	}
+	check.idents[id] = obj
+	if f := check.ctxt.Ident; f != nil {
+		f(id, obj)
+	}
+}
+
 // lookup returns the unique Object denoted by the identifier.
 // For identifiers without assigned *ast.Object, it uses the
 // checker.idents map; for identifiers with an *ast.Object it
@@ -41,8 +56,8 @@ type checker struct {
 //           the typechecker, only the idents map is needed.
 //
 func (check *checker) lookup(ident *ast.Ident) Object {
-	astObj := ident.Obj
 	obj := check.idents[ident]
+	astObj := ident.Obj
 
 	if obj != nil {
 		assert(astObj == nil || check.objects[astObj] == nil || check.objects[astObj] == obj)
@@ -53,10 +68,9 @@ func (check *checker) lookup(ident *ast.Ident) Object {
 		return nil
 	}
 
-	obj = check.objects[astObj]
-	if obj == nil {
+	if obj = check.objects[astObj]; obj == nil {
 		obj = newObj(astObj)
-		check.idents[ident] = obj
+		check.register(ident, obj)
 		check.objects[astObj] = obj
 	}
 
@@ -82,7 +96,7 @@ func (check *checker) later(f *Func, sig *Signature, body *ast.BlockStmt) {
 
 func (check *checker) declareIdent(scope *Scope, ident *ast.Ident, obj Object) {
 	assert(check.lookup(ident) == nil) // identifier already declared or resolved
-	check.idents[ident] = obj
+	check.register(ident, obj)
 	if ident.Name != "_" {
 		if alt := scope.Insert(obj); alt != nil {
 			prevDecl := ""
@@ -364,7 +378,7 @@ func (check *checker) decl(decl ast.Decl) {
 		if d.Name.Name == "init" {
 			assert(obj == nil) // all other functions should have an object
 			obj = &Func{Name: d.Name.Name, decl: d}
-			check.idents[d.Name] = obj
+			check.register(d.Name, obj)
 		}
 		check.object(obj, false)
 	default:

src/pkg/go/types/expr.go

@@ -511,7 +511,7 @@ func (check *checker) index(index ast.Expr, length int64, iota int) int64 {
 func (check *checker) compositeLitKey(key ast.Expr) {
 	if ident, ok := key.(*ast.Ident); ok && ident.Obj == nil {
 		if obj := check.pkg.Scope.Lookup(ident.Name); obj != nil {
-			check.idents[ident] = obj
+			check.register(ident, obj)
 		} else {
 			check.errorf(ident.Pos(), "undeclared name: %s", ident.Name)
 		}
@@ -871,6 +871,7 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle
 					check.errorf(e.Sel.Pos(), "cannot refer to unexported %s", sel)
 					goto Error
 				}
+				check.register(e.Sel, exp)
 				// Simplified version of the code for *ast.Idents:
 				// - imported packages use types.Scope and types.Objects
 				// - imported objects are always fully initialized

src/pkg/go/types/resolve.go

@@ -29,7 +29,7 @@ func (check *checker) declareObj(scope, altScope *Scope, obj Object) {
 func (check *checker) resolveIdent(scope *Scope, ident *ast.Ident) bool {
 	for ; scope != nil; scope = scope.Outer {
 		if obj := scope.Lookup(ident.Name); obj != nil {
-			check.idents[ident] = obj
+			check.register(ident, obj)
 			return true
 		}
 	}

src/pkg/go/types/resolver_test.go

@@ -5,10 +5,8 @@
 package types
 
 import (
-	"fmt"
 	"go/ast"
-	//"go/parser"
-	"go/scanner"
+	"go/parser"
 	"go/token"
 	"testing"
 )
@@ -30,13 +28,9 @@ var sources = []string{
 	}
 	func g() (x int) { return }
 	`,
-	// TODO(gri) fix this
-	// cannot handle dot-import at the moment
-	/*
-		`package p
-		import . "go/parser"
-		func g() Mode { return ImportsOnly }`,
-	*/
+	`package p
+	import . "go/parser"
+	func g() Mode { return ImportsOnly }`,
 }
 
 var pkgnames = []string{
@@ -44,88 +38,52 @@ var pkgnames = []string{
 	"math",
 }
 
-// ResolveQualifiedIdents resolves the selectors of qualified
-// identifiers by associating the correct ast.Object with them.
-// TODO(gri): Eventually, this functionality should be subsumed
-//            by Check.
-//
-func ResolveQualifiedIdents(fset *token.FileSet, pkg *ast.Package) error {
-	var errors scanner.ErrorList
-
-	findObj := func(pkg *ast.Object, name *ast.Ident) *ast.Object {
-		scope := pkg.Data.(*ast.Scope)
-		obj := scope.Lookup(name.Name)
-		if obj == nil {
-			errors.Add(fset.Position(name.Pos()), fmt.Sprintf("no %s in package %s", name.Name, pkg.Name))
-		}
-		return obj
-	}
-
-	ast.Inspect(pkg, func(n ast.Node) bool {
-		if s, ok := n.(*ast.SelectorExpr); ok {
-			if x, ok := s.X.(*ast.Ident); ok && x.Obj != nil && x.Obj.Kind == ast.Pkg {
-				// find selector in respective package
-				s.Sel.Obj = findObj(x.Obj, s.Sel)
-			}
-			return false
-		}
-		return true
-	})
-
-	return errors.Err()
-}
-
 func TestResolveQualifiedIdents(t *testing.T) {
-	return
-	// disabled for now
-	/*
-		// parse package files
-		fset := token.NewFileSet()
-		files := make([]*ast.File, len(sources))
-		for i, src := range sources {
-			f, err := parser.ParseFile(fset, "", src, parser.DeclarationErrors)
-			if err != nil {
-				t.Fatal(err)
-			}
-			files[i] = f
-		}
-
-		// resolve package AST
-		astpkg, pkg, err := Check(fset, files)
+	// parse package files
+	fset := token.NewFileSet()
+	var files []*ast.File
+	for _, src := range sources {
+		f, err := parser.ParseFile(fset, "", src, parser.DeclarationErrors)
 		if err != nil {
 			t.Fatal(err)
 		}
+		files = append(files, f)
+	}
 
-		// check that all packages were imported
-		for _, name := range pkgnames {
-			if pkg.Imports[name] == nil {
-				t.Errorf("package %s not imported", name)
-			}
-		}
+	// resolve and type-check package AST
+	idents := make(map[*ast.Ident]Object)
+	ctxt := Default
+	ctxt.Ident = func(id *ast.Ident, obj Object) { idents[id] = obj }
+	pkg, err := ctxt.Check(fset, files)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-		// TODO(gri) fix this
-		// unresolved identifiers are not collected at the moment
-		// check that there are no top-level unresolved identifiers
-		for _, f := range astpkg.Files {
-			for _, x := range f.Unresolved {
-				t.Errorf("%s: unresolved global identifier %s", fset.Position(x.Pos()), x.Name)
-			}
+	// check that all packages were imported
+	for _, name := range pkgnames {
+		if pkg.Imports[name] == nil {
+			t.Errorf("package %s not imported", name)
 		}
+	}
 
-		// resolve qualified identifiers
-		if err := ResolveQualifiedIdents(fset, astpkg); err != nil {
-			t.Error(err)
+	// check that there are no top-level unresolved identifiers
+	for _, f := range files {
+		for _, x := range f.Unresolved {
+			t.Errorf("%s: unresolved global identifier %s", fset.Position(x.Pos()), x.Name)
 		}
+	}
 
-		// check that qualified identifiers are resolved
-		ast.Inspect(astpkg, func(n ast.Node) bool {
+	// check that qualified identifiers are resolved
+	for _, f := range files {
+		ast.Inspect(f, func(n ast.Node) bool {
 			if s, ok := n.(*ast.SelectorExpr); ok {
 				if x, ok := s.X.(*ast.Ident); ok {
-					if x.Obj == nil {
+					obj := idents[x]
+					if obj == nil {
 						t.Errorf("%s: unresolved qualified identifier %s", fset.Position(x.Pos()), x.Name)
 						return false
 					}
-					if x.Obj.Kind == ast.Pkg && s.Sel != nil && s.Sel.Obj == nil {
+					if _, ok := obj.(*Package); ok && idents[s.Sel] == nil {
 						t.Errorf("%s: unresolved selector %s", fset.Position(s.Sel.Pos()), s.Sel.Name)
 						return false
 					}
@@ -135,5 +93,5 @@ func TestResolveQualifiedIdents(t *testing.T) {
 			}
 			return true
 		})
-	*/
+	}
 }

src/pkg/go/types/stmt.go

@@ -307,6 +307,9 @@ func (check *checker) stmt(s ast.Stmt) {
 			// function calls are permitted
 			used = true
 			// but some builtins are excluded
+			// (Caution: This evaluates e.Fun twice, once here and once
+			//           below as part of s.X. This has consequences for
+			//           check.register. Perhaps this can be avoided.)
 			check.expr(&x, e.Fun, nil, -1)
 			if x.mode != invalid {
 				if b, ok := x.typ.(*builtin); ok && !b.isStatement {
@@ -431,7 +434,7 @@ func (check *checker) stmt(s ast.Stmt) {
 				}
 				name := ast.NewIdent(res.Name)
 				name.NamePos = s.Pos()
-				check.idents[name] = &Var{Name: res.Name, Type: res.Type}
+				check.register(name, &Var{Name: res.Name, Type: res.Type})
 				lhs[i] = name
 			}
 			if len(s.Results) > 0 || !named {
@@ -465,7 +468,7 @@ func (check *checker) stmt(s ast.Stmt) {
 		if tag == nil {
 			// use fake true tag value and position it at the opening { of the switch
 			ident := &ast.Ident{NamePos: s.Body.Lbrace, Name: "true"}
-			check.idents[ident] = Universe.Lookup("true")
+			check.register(ident, Universe.Lookup("true"))
 			tag = ident
 		}
 		check.expr(&x, tag, nil, -1)