From 7f18f8119257d42e5702b56c246c930904aac15a Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 14 Jan 2013 09:43:27 -0800 Subject: [PATCH] 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 | 9 ++- src/pkg/go/types/check.go | 26 +++++-- src/pkg/go/types/expr.go | 3 +- src/pkg/go/types/resolve.go | 2 +- src/pkg/go/types/resolver_test.go | 114 ++++++++++-------------------- src/pkg/go/types/stmt.go | 7 +- 6 files changed, 72 insertions(+), 89 deletions(-) diff --git a/src/pkg/go/types/api.go b/src/pkg/go/types/api.go index b7b5b90b62..502958000e 100644 --- a/src/pkg/go/types/api.go +++ b/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. diff --git a/src/pkg/go/types/check.go b/src/pkg/go/types/check.go index 07f0e861e4..33b9c7e2b4 100644 --- a/src/pkg/go/types/check.go +++ b/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: diff --git a/src/pkg/go/types/expr.go b/src/pkg/go/types/expr.go index 9f4cece20a..9a4b6c6051 100644 --- a/src/pkg/go/types/expr.go +++ b/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 diff --git a/src/pkg/go/types/resolve.go b/src/pkg/go/types/resolve.go index ef486c27ca..b314b7add4 100644 --- a/src/pkg/go/types/resolve.go +++ b/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 } } diff --git a/src/pkg/go/types/resolver_test.go b/src/pkg/go/types/resolver_test.go index d83ca753e0..40fe21fc4c 100644 --- a/src/pkg/go/types/resolver_test.go +++ b/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 }) - */ + } } diff --git a/src/pkg/go/types/stmt.go b/src/pkg/go/types/stmt.go index 492dfb6c67..f1d6704110 100644 --- a/src/pkg/go/types/stmt.go +++ b/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) -- 2.48.1