From b52cce75f37150a1a154c3ab25d34fcec605e658 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 28 May 2013 11:39:15 -0700 Subject: [PATCH] adonovan: first round of cleanups: remove 'resolve' internal flag and respective dead code No other changes besides documentation updates for now. R=golang-dev, adonovan CC=golang-dev https://golang.org/cl/9780045 --- go/types/api.go | 3 - go/types/check.go | 432 ++------------------------------------ go/types/check_test.go | 10 +- go/types/expr.go | 91 +++----- go/types/objects.go | 41 +--- go/types/resolve.go | 182 ---------------- go/types/resolver_test.go | 39 +--- go/types/stdlib_test.go | 10 +- go/types/stmt.go | 69 +++--- go/types/types_test.go | 12 +- 10 files changed, 81 insertions(+), 808 deletions(-) delete mode 100644 go/types/resolve.go diff --git a/go/types/api.go b/go/types/api.go index 1dd46d43..87515a99 100644 --- a/go/types/api.go +++ b/go/types/api.go @@ -24,12 +24,9 @@ package types // BUG(gri): Switch statements don't check correct use of 'fallthrough'. // BUG(gri): Switch statements don't check duplicate cases for all types for which it is required. // BUG(gri): Some built-ins may not be callable if in statement-context. -// BUG(gri): Duplicate declarations in different files may not be reported. -// BUG(gri): The type-checker assumes that the input *ast.Files were created by go/parser. // The API is still slightly in flux and the following changes are considered: // -// API(gri): Provide position information for all objects. // API(gri): The GcImporter should probably be in its own package - it is only one of possible importers. import ( diff --git a/go/types/check.go b/go/types/check.go index 69f69e42..ccae8263 100644 --- a/go/types/check.go +++ b/go/types/check.go @@ -18,8 +18,6 @@ import ( const ( debug = true // leave on during development trace = false // turn on for detailed type resolution traces - // TODO(gri) remove this flag and clean up code under the assumption that resolve == true. - resolve = true // if set, resolve all identifiers in the type checker (don't use ast.Objects anymore) ) // exprInfo stores type and constant value for an untyped expression. @@ -35,23 +33,19 @@ type checker struct { fset *token.FileSet // lazily initialized - pkg *Package // current package - firsterr error // first error encountered - idents map[*ast.Ident]Object // maps identifiers to their unique object - objects map[*ast.Object]Object // maps *ast.Objects to their unique object - initspecs map[*ast.ValueSpec]*ast.ValueSpec // "inherited" type and initialization expressions for constant declarations - methods map[*TypeName]*Scope // maps type names to associated methods - conversions map[*ast.CallExpr]bool // set of type-checked conversions (to distinguish from calls) - untyped map[ast.Expr]exprInfo // map of expressions without final type + pkg *Package // current package + firsterr error // first error encountered + methods map[*TypeName]*Scope // maps type names to associated methods + conversions map[*ast.CallExpr]bool // set of type-checked conversions (to distinguish from calls) + untyped map[ast.Expr]exprInfo // map of expressions without final type + + objMap map[Object]*decl // if set we are in the package-global declaration phase (otherwise all objects seen must be declared) + topScope *Scope // topScope for lookups, non-global declarations // functions funclist []function // list of functions/methods with correct signatures and non-empty bodies funcsig *Signature // signature of currently typechecked function pos []token.Pos // stack of expr positions; debugging support, used if trace is set - - // these are only valid if resolve is set - objMap map[Object]*decl // if set we are in the package-global declaration phase (otherwise all objects seen must be declared) - topScope *Scope // topScope for lookups, non-global declarations } func (check *checker) openScope() { @@ -63,68 +57,23 @@ func (check *checker) closeScope() { } func (check *checker) register(id *ast.Ident, obj Object) { - if resolve { - // TODO(gri) Document how if an identifier can be registered more than once. - if f := check.ctxt.Ident; f != nil { - f(id, obj) - } - return - } - - // 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 + // TODO(gri) Document if an identifier can be registered more than once. 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 -// uses the checker.objects map. -// -// TODO(gri) Once identifier resolution is done entirely by -// the typechecker, only scopes are needed. Need -// to update the comment above. -// +// lookup returns the Object denoted by ident by looking up the +// identifier in the scope chain, starting with the top-most scope. +// If no object is found, lookup returns nil. func (check *checker) lookup(ident *ast.Ident) Object { - if resolve { - for scope := check.topScope; scope != nil; scope = scope.Outer { - if obj := scope.Lookup(ident.Name); obj != nil { - check.register(ident, obj) - return obj - } + for scope := check.topScope; scope != nil; scope = scope.Outer { + if obj := scope.Lookup(ident.Name); obj != nil { + check.register(ident, obj) + return obj } - return nil } - - // old code - obj := check.idents[ident] - astObj := ident.Obj - - if obj != nil { - assert(astObj == nil || check.objects[astObj] == nil || check.objects[astObj] == obj) - return obj - } - - if astObj == nil { - return nil - } - - if obj = check.objects[astObj]; obj == nil { - obj = newObj(check.pkg, astObj) - check.objects[astObj] = obj - } - check.register(ident, obj) - - return obj + return nil } type function struct { @@ -145,330 +94,6 @@ func (check *checker) later(f *Func, sig *Signature, body *ast.BlockStmt) { } } -func (check *checker) declareIdent(scope *Scope, ident *ast.Ident, obj Object) { - if resolve { - unreachable() - } - assert(check.lookup(ident) == nil) // identifier already declared or resolved - check.register(ident, obj) - if ident.Name != "_" { - if alt := scope.Insert(obj); alt != nil { - prevDecl := "" - if pos := alt.Pos(); pos.IsValid() { - prevDecl = fmt.Sprintf("\n\tprevious declaration at %s", check.fset.Position(pos)) - } - check.errorf(ident.Pos(), fmt.Sprintf("%s redeclared in this block%s", ident.Name, prevDecl)) - } - } -} - -func (check *checker) valueSpec(pos token.Pos, obj Object, lhs []*ast.Ident, spec *ast.ValueSpec, iota int) { - if resolve { - unreachable() - } - - if len(lhs) == 0 { - check.invalidAST(pos, "missing lhs in declaration") - return - } - - // determine type for all of lhs, if any - // (but only set it for the object we typecheck!) - var typ Type - if spec.Type != nil { - typ = check.typ(spec.Type, false) - } - - // len(lhs) > 0 - rhs := spec.Values - if len(lhs) == len(rhs) { - // check only lhs and rhs corresponding to obj - var l, r ast.Expr - for i, name := range lhs { - if check.lookup(name) == obj { - l = lhs[i] - r = rhs[i] - break - } - } - isConst := false - switch obj := obj.(type) { - case *Const: - obj.typ = typ - isConst = true - case *Var: - obj.typ = typ - default: - unreachable() - } - check.assign1to1(l, r, nil, true, iota, isConst) - return - } - - // there must be a type or initialization expressions - if typ == nil && len(rhs) == 0 { - check.invalidAST(pos, "missing type or initialization expression") - typ = Typ[Invalid] - } - - // if we have a type, mark all of lhs - if typ != nil { - for _, name := range lhs { - switch obj := check.lookup(name).(type) { - case *Const: - obj.typ = typ - case *Var: - obj.typ = typ - default: - unreachable() - } - } - } - - // check initial values, if any - if len(rhs) > 0 { - // TODO(gri) should try to avoid this conversion - lhx := make([]ast.Expr, len(lhs)) - for i, e := range lhs { - lhx[i] = e - } - _, isConst := obj.(*Const) - check.assignNtoM(lhx, rhs, true, iota, isConst) - } -} - -// object typechecks an object by assigning it a type. -// -func (check *checker) object(obj Object, cycleOk bool) { - if resolve { - unreachable() - } - - if obj.Type() != nil { - return // already checked - } - - switch obj := obj.(type) { - case *Package: - // nothing to do - if resolve { - unreachable() - } - - case *Const: - // The obj.Val field for constants is initialized to its respective - // iota value (type int) by the parser. - // If the object's type is Typ[Invalid], the object value is ignored. - // If the object's type is valid, the object value must be a legal - // constant value; it may be nil to indicate that we don't know the - // value of the constant (e.g., in: "const x = float32("foo")" we - // know that x is a constant and has type float32, but we don't - // have a value due to the error in the conversion). - if obj.visited { - check.errorf(obj.Pos(), "illegal cycle in initialization of constant %s", obj.name) - obj.typ = Typ[Invalid] - return - } - obj.visited = true - spec := obj.spec - iota, ok := exact.Int64Val(obj.val) - assert(ok) - obj.val = exact.MakeUnknown() - // determine spec for type and initialization expressions - init := spec - if len(init.Values) == 0 { - init = check.initspecs[spec] - } - check.valueSpec(spec.Pos(), obj, spec.Names, init, int(iota)) - - case *Var: - if obj.visited { - check.errorf(obj.Pos(), "illegal cycle in initialization of variable %s", obj.name) - obj.typ = Typ[Invalid] - return - } - obj.visited = true - switch d := obj.decl.(type) { - case *ast.Field: - unreachable() // function parameters are always typed when collected - case *ast.ValueSpec: - check.valueSpec(d.Pos(), obj, d.Names, d, 0) - case *ast.AssignStmt: - unreachable() // assign1to1 sets the type for failing short var decls - default: - unreachable() // see also function newObj - } - - case *TypeName: - typ := &Named{obj: obj} - obj.typ = typ // "mark" object so recursion terminates - typ.underlying = check.typ(obj.spec.Type, cycleOk).Underlying() - // typecheck associated method signatures - if scope := check.methods[obj]; scope != nil { - switch t := typ.underlying.(type) { - case *Struct: - // struct fields must not conflict with methods - for _, f := range t.fields { - if m := scope.Lookup(f.Name); m != nil { - check.errorf(m.Pos(), "type %s has both field and method named %s", obj.name, f.Name) - // ok to continue - } - } - case *Interface: - // methods cannot be associated with an interface type - for _, m := range scope.Entries { - recv := m.(*Func).decl.Recv.List[0].Type - check.errorf(recv.Pos(), "invalid receiver type %s (%s is an interface type)", obj.name, obj.name) - // ok to continue - } - } - // typecheck method signatures - var methods ObjSet - for _, obj := range scope.Entries { - m := obj.(*Func) - sig := check.typ(m.decl.Type, cycleOk).(*Signature) - params, _ := check.collectParams(sig.scope, m.decl.Recv, false) - sig.recv = params[0] // the parser/assocMethod ensure there is exactly one parameter - m.typ = sig - assert(methods.Insert(obj) == nil) - check.later(m, sig, m.decl.Body) - } - typ.methods = methods - delete(check.methods, obj) // we don't need this scope anymore - } - - case *Func: - fdecl := obj.decl - // methods are typechecked when their receivers are typechecked - if fdecl.Recv == nil { - sig := check.typ(fdecl.Type, cycleOk).(*Signature) - if obj.name == "init" && (sig.params.Len() > 0 || sig.results.Len() > 0) { - check.errorf(fdecl.Pos(), "func init must have no arguments and no return values") - // ok to continue - } - obj.typ = sig - check.later(obj, sig, fdecl.Body) - } - - default: - unreachable() - } -} - -// assocInitvals associates "inherited" initialization expressions -// with the corresponding *ast.ValueSpec in the check.initspecs map -// for constant declarations without explicit initialization expressions. -// -func (check *checker) assocInitvals(decl *ast.GenDecl) { - if resolve { - unreachable() - } - - var last *ast.ValueSpec - for _, s := range decl.Specs { - if s, ok := s.(*ast.ValueSpec); ok { - if len(s.Values) > 0 { - last = s - } else { - check.initspecs[s] = last - } - } - } - if last == nil { - check.invalidAST(decl.Pos(), "no initialization values provided") - } -} - -// assocMethod associates a method declaration with the respective -// receiver base type. meth.Recv must exist. -// -func (check *checker) assocMethod(meth *ast.FuncDecl) { - if resolve { - unreachable() - } - - // The receiver type is one of the following (enforced by parser): - // - *ast.Ident - // - *ast.StarExpr{*ast.Ident} - // - *ast.BadExpr (parser error) - typ := meth.Recv.List[0].Type - if ptr, ok := typ.(*ast.StarExpr); ok { - typ = ptr.X - } - // determine receiver base type name - ident, ok := typ.(*ast.Ident) - if !ok { - // not an identifier - parser reported error already - return // ignore this method - } - // determine receiver base type object - var tname *TypeName - if obj := check.lookup(ident); obj != nil { - obj, ok := obj.(*TypeName) - if !ok { - check.errorf(ident.Pos(), "%s is not a type", ident.Name) - return // ignore this method - } - if obj.spec == nil { - check.errorf(ident.Pos(), "cannot define method on non-local type %s", ident.Name) - return // ignore this method - } - tname = obj - } else { - // identifier not declared/resolved - parser reported error already - return // ignore this method - } - // declare method in receiver base type scope - scope := check.methods[tname] - if scope == nil { - scope = new(Scope) - check.methods[tname] = scope - } - check.declareIdent(scope, meth.Name, &Func{pkg: check.pkg, name: meth.Name.Name, decl: meth}) -} - -func (check *checker) decl(decl ast.Decl) { - if resolve { - unreachable() // during top-level type-checking - } - - switch d := decl.(type) { - case *ast.BadDecl: - // ignore - case *ast.GenDecl: - for _, spec := range d.Specs { - switch s := spec.(type) { - case *ast.ImportSpec: - // nothing to do (handled by check.resolve) - case *ast.ValueSpec: - for _, name := range s.Names { - check.object(check.lookup(name), false) - } - case *ast.TypeSpec: - check.object(check.lookup(s.Name), false) - default: - check.invalidAST(s.Pos(), "unknown ast.Spec node %T", s) - } - } - case *ast.FuncDecl: - // methods are checked when their respective base types are checked - if d.Recv != nil { - return - } - obj := check.lookup(d.Name) - // Initialization functions don't have an object associated with them - // since they are not in any scope. Create a dummy object for them. - if d.Name.Name == "init" { - assert(obj == nil) // all other functions should have an object - obj = &Func{pkg: check.pkg, name: d.Name.Name, decl: d} - check.register(d.Name, obj) - } - check.object(obj, false) - default: - check.invalidAST(d.Pos(), "unknown ast.Decl node %T", d) - } -} - // A bailout panic is raised to indicate early termination. type bailout struct{} @@ -484,9 +109,6 @@ func check(ctxt *Context, path string, fset *token.FileSet, files ...*ast.File) ctxt: ctxt, fset: fset, pkg: pkg, - idents: make(map[*ast.Ident]Object), - objects: make(map[*ast.Object]Object), - initspecs: make(map[*ast.ValueSpec]*ast.ValueSpec), methods: make(map[*TypeName]*Scope), conversions: make(map[*ast.CallExpr]bool), untyped: make(map[ast.Expr]exprInfo), @@ -532,24 +154,8 @@ func check(ctxt *Context, path string, fset *token.FileSet, files ...*ast.File) imp = GcImport } - if resolve { - check.resolveFiles(files, imp) - - } else { - methods := check.resolve(imp, files) - - // associate methods with types - for _, m := range methods { - check.assocMethod(m) - } - - // typecheck all declarations - for _, f := range files { - for _, d := range f.Decls { - check.decl(d) - } - } - } + // TODO(gri) resolveFiles needs to be split up and renamed (cleanup) + check.resolveFiles(files, imp) // typecheck all function/method bodies // (funclist may grow when checking statements - do not use range clause!) diff --git a/go/types/check_test.go b/go/types/check_test.go index fcab8d8b..dc2dabd1 100644 --- a/go/types/check_test.go +++ b/go/types/check_test.go @@ -83,11 +83,7 @@ func parseFiles(t *testing.T, testname string, filenames []string) ([]*ast.File, var files []*ast.File var errlist []error for _, filename := range filenames { - mode := parser.AllErrors - if !resolve { - mode |= parser.DeclarationErrors - } - file, err := parser.ParseFile(fset, filename, nil, mode) + file, err := parser.ParseFile(fset, filename, nil, parser.AllErrors) if file == nil { t.Fatalf("%s: could not parse file %s", testname, filename) } @@ -241,10 +237,6 @@ func TestCheck(t *testing.T) { return } - if resolve { - fmt.Println("*** Running new code: Identifiers are resolved by type checker. ***") - } - // Otherwise, run all the tests. for _, test := range tests { checkFiles(t, test.name, test.files) diff --git a/go/types/expr.go b/go/types/expr.go index d5cff4a4..768b0fa4 100644 --- a/go/types/expr.go +++ b/go/types/expr.go @@ -93,15 +93,10 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO if len(field.Names) > 0 { // named parameter for _, name := range field.Names { - var par *Var - if resolve { - par = &Var{pos: name.Pos(), pkg: check.pkg, name: name.Name, typ: typ, decl: field} - check.declare(scope, par) - check.register(name, par) - } else { - par = check.lookup(name).(*Var) - par.typ = typ - } + par := &Var{pos: name.Pos(), pkg: check.pkg, name: name.Name, typ: typ, decl: field} + check.declare(scope, par) + check.register(name, par) + last = par copy := *par params = append(params, ©) @@ -143,17 +138,14 @@ func (check *checker) collectMethods(scope *Scope, list *ast.FieldList) (methods // TODO(gri) with unified scopes (Scope, ObjSet) this can become // just a normal declaration obj := &Func{name.Pos(), check.pkg, nil, name.Name, sig, nil} - if alt := methods.Insert(obj); alt != nil && resolve { - // if !resolve, the parser complains + if alt := methods.Insert(obj); alt != nil { prevDecl := "" if pos := alt.Pos(); pos.IsValid() { prevDecl = fmt.Sprintf("\n\tprevious declaration at %s", check.fset.Position(pos)) } check.errorf(obj.Pos(), "%s redeclared in this block%s", obj.Name(), prevDecl) } - if resolve { - check.register(name, obj) - } + check.register(name, obj) } } else { // embedded interface @@ -200,13 +192,13 @@ func (check *checker) collectFields(scope *Scope, list *ast.FieldList, cycleOk b if tags != nil { tags = append(tags, tag) } - if resolve { - fld := &Var{pos: pos, pkg: check.pkg, name: name, typ: typ, decl: field} - check.declare(scope, fld) - if resolve && ident != nil { - check.register(ident, fld) - } + + fld := &Var{pos: pos, pkg: check.pkg, name: name, typ: typ, decl: field} + check.declare(scope, fld) + if ident != nil { + check.register(ident, fld) } + fields = append(fields, &Field{check.pkg, name, typ, isAnonymous}) } @@ -893,23 +885,6 @@ func (check *checker) index(arg ast.Expr, length int64, iota int) (i int64, ok b return -1, true } -// compositeLitKey resolves unresolved composite literal keys. -// For details, see comment in go/parser/parser.go, method parseElement. -func (check *checker) compositeLitKey(key ast.Expr) { - if resolve { - return - } - if ident, ok := key.(*ast.Ident); ok && ident.Obj == nil { - if obj := check.pkg.scope.Lookup(ident.Name); obj != nil { - check.register(ident, obj) - } else if obj := Universe.Lookup(ident.Name); obj != nil { - check.register(ident, obj) - } else { - check.errorf(ident.Pos(), "undeclared name: %s", ident.Name) - } - } -} - // indexElts checks the elements (elts) of an array or slice composite literal // against the literal's element type (typ), and the element indices against // the literal length if known (length >= 0). It returns the length of the @@ -923,7 +898,6 @@ func (check *checker) indexedElts(elts []ast.Expr, typ Type, length int64, iota validIndex := false eval := e if kv, _ := e.(*ast.KeyValueExpr); kv != nil { - check.compositeLitKey(kv.Key) if i, ok := check.index(kv.Key, length, iota); ok { if i >= 0 { index = i @@ -1072,37 +1046,29 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle goto Error // error was reported before case *ast.Ident: - if !resolve && e.Name == "_" { - check.invalidOp(e.Pos(), "cannot use _ as value or type") - goto Error - } obj := check.lookup(e) if obj == nil { - if resolve { - if e.Name == "_" { - check.invalidOp(e.Pos(), "cannot use _ as value or type") - } else { - // TODO(gri) anonymous function result parameters are - // not declared - this causes trouble when - // type-checking return statements - check.errorf(e.Pos(), "undeclared name: %s", e.Name) - } + if e.Name == "_" { + check.invalidOp(e.Pos(), "cannot use _ as value or type") + } else { + // TODO(gri) anonymous function result parameters are + // not declared - this causes trouble when + // type-checking return statements + check.errorf(e.Pos(), "undeclared name: %s", e.Name) } goto Error // error was reported before } - if resolve { - typ := obj.Type() - if check.objMap == nil { - if typ == nil { - check.dump("%s: %s not declared?", e.Pos(), e) - } - assert(typ != nil) - } else if typ == nil { - check.declareObject(obj, cycleOk) + + typ := obj.Type() + if check.objMap == nil { + if typ == nil { + check.dump("%s: %s not declared?", e.Pos(), e) } - } else { - check.object(obj, cycleOk) + assert(typ != nil) + } else if typ == nil { + check.declareObject(obj, cycleOk) } + switch obj := obj.(type) { case *Package: check.errorf(e.Pos(), "use of package %s not in selector", obj.Name) @@ -1271,7 +1237,6 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle check.errorf(e.Pos(), "missing key in map literal") continue } - check.compositeLitKey(kv.Key) check.expr(x, kv.Key, nil, iota) if !check.assignment(x, utyp.key) { if x.mode != invalid { diff --git a/go/types/objects.go b/go/types/objects.go index 02af57ea..ce2a3862 100644 --- a/go/types/objects.go +++ b/go/types/objects.go @@ -12,6 +12,8 @@ import ( ) // TODO(gri) provide a complete set of factory functions! +// TODO(gri) clean up the Pos functions. +// TODO(gri) clean up the internal links to decls/specs, etc. // An Object describes a named language entity such as a package, // constant, type, variable, function (incl. methods), or label. @@ -202,42 +204,3 @@ func (obj *Func) Pos() token.Pos { return token.NoPos } func (obj *Func) setOuter(s *Scope) { obj.outer = s } - -// newObj returns a new Object for a given *ast.Object. -// It does not canonicalize them (it always returns a new one). -// For canonicalization, see check.lookup. -// -// TODO(gri) Once we do identifier resolution completely in -// the typechecker, this functionality can go. -// -func newObj(pkg *Package, astObj *ast.Object) Object { - assert(pkg != nil) - name := astObj.Name - typ, _ := astObj.Type.(Type) - switch astObj.Kind { - case ast.Bad: - // ignore - case ast.Pkg: - unreachable() - case ast.Con: - iota := astObj.Data.(int) - return &Const{pkg: pkg, name: name, typ: typ, val: exact.MakeInt64(int64(iota)), spec: astObj.Decl.(*ast.ValueSpec)} - case ast.Typ: - return &TypeName{pkg: pkg, name: name, typ: typ, spec: astObj.Decl.(*ast.TypeSpec)} - case ast.Var: - switch astObj.Decl.(type) { - case *ast.Field: // function parameters - case *ast.ValueSpec: // proper variable declarations - case *ast.AssignStmt: // short variable declarations - default: - unreachable() // everything else is not ok - } - return &Var{pkg: pkg, name: name, typ: typ, decl: astObj.Decl} - case ast.Fun: - return &Func{pkg: pkg, name: name, typ: typ, decl: astObj.Decl.(*ast.FuncDecl)} - case ast.Lbl: - unreachable() // for now - } - unreachable() - return nil -} diff --git a/go/types/resolve.go b/go/types/resolve.go deleted file mode 100644 index e152e073..00000000 --- a/go/types/resolve.go +++ /dev/null @@ -1,182 +0,0 @@ -// Copyright 2013 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 ( - "fmt" - "go/ast" - "go/token" - "strconv" -) - -func (check *checker) declareObj(scope, altScope *Scope, obj Object, dotImport token.Pos) { - alt := scope.Insert(obj) - if alt == nil && altScope != nil { - // see if there is a conflicting declaration in altScope - alt = altScope.Lookup(obj.Name()) - } - if alt != nil { - prevDecl := "" - - // for dot-imports, local declarations are declared first - swap messages - if dotImport.IsValid() { - if pos := alt.Pos(); pos.IsValid() { - check.errorf(pos, fmt.Sprintf("%s redeclared in this block by import at %s", - obj.Name(), check.fset.Position(dotImport))) - return - } - - // get by w/o other position - check.errorf(dotImport, fmt.Sprintf("dot-import redeclares %s", obj.Name())) - return - } - - if pos := alt.Pos(); pos.IsValid() { - prevDecl = fmt.Sprintf("\n\tother declaration at %s", check.fset.Position(pos)) - } - check.errorf(obj.Pos(), fmt.Sprintf("%s redeclared in this block%s", obj.Name(), prevDecl)) - } -} - -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.register(ident, obj) - return true - } - } - return false -} - -func (check *checker) resolve(importer Importer, files []*ast.File) (methods []*ast.FuncDecl) { - pkg := check.pkg - - // complete package scope - for _, file := range files { - // the package identifier denotes the current package - check.register(file.Name, pkg) - - // insert top-level file objects in package scope - // (the parser took care of declaration errors in a single file, - // but not across multiple files - hence we need to check again) - for _, decl := range file.Decls { - switch d := decl.(type) { - case *ast.BadDecl: - // ignore - case *ast.GenDecl: - if d.Tok == token.CONST { - check.assocInitvals(d) - } - for _, spec := range d.Specs { - switch s := spec.(type) { - case *ast.ImportSpec: - // handled separately below - case *ast.ValueSpec: - for _, name := range s.Names { - if name.Name == "_" { - continue - } - check.declareObj(pkg.scope, nil, check.lookup(name), token.NoPos) - } - case *ast.TypeSpec: - if s.Name.Name == "_" { - continue - } - check.declareObj(pkg.scope, nil, check.lookup(s.Name), token.NoPos) - default: - check.invalidAST(s.Pos(), "unknown ast.Spec node %T", s) - } - } - case *ast.FuncDecl: - if d.Recv != nil { - // collect method - methods = append(methods, d) - continue - } - if d.Name.Name == "_" || d.Name.Name == "init" { - continue // blank (_) and init functions are inaccessible - } - check.declareObj(pkg.scope, nil, check.lookup(d.Name), token.NoPos) - default: - check.invalidAST(d.Pos(), "unknown ast.Decl node %T", d) - } - } - } - - // complete file scopes with imports and resolve identifiers - for _, file := range files { - // build file scope by processing all imports - importErrors := false - fileScope := &Scope{Outer: pkg.scope} - for _, spec := range file.Imports { - if importer == nil { - importErrors = true - continue - } - path, _ := strconv.Unquote(spec.Path.Value) - imp, err := importer(pkg.imports, path) - if err != nil { - check.errorf(spec.Path.Pos(), "could not import %s (%s)", path, err) - importErrors = true - continue - } - // TODO(gri) If a local package name != "." is provided, - // global identifier resolution could proceed even if the - // import failed. Consider adjusting the logic here a bit. - - // local name overrides imported package name - name := imp.name - if spec.Name != nil { - name = spec.Name.Name - } - - // add import to file scope - if name == "." { - // merge imported scope with file scope - for _, obj := range imp.scope.Entries { - // gcimported package scopes contain non-exported - // objects such as types used in partially exported - // objects - do not accept them - if ast.IsExported(obj.Name()) { - check.declareObj(fileScope, pkg.scope, obj, spec.Pos()) - } - } - // TODO(gri) consider registering the "." identifier - // if we have Context.Ident callbacks for say blank - // (_) identifiers - // check.register(spec.Name, pkg) - } else if name != "_" { - // declare imported package object in file scope - // (do not re-use imp in the file scope but create - // a new object instead; the Decl field is different - // for different files) - obj := &Package{name: name, scope: imp.scope, spec: spec} - check.declareObj(fileScope, pkg.scope, obj, token.NoPos) - } - } - - // resolve identifiers - if importErrors { - // don't use the universe scope without correct imports - // (objects in the universe may be shadowed by imports; - // with missing imports, identifiers might get resolved - // incorrectly to universe objects) - pkg.scope.Outer = nil - } - i := 0 - for _, ident := range file.Unresolved { - if !check.resolveIdent(fileScope, ident) { - check.errorf(ident.Pos(), "undeclared name: %s", ident.Name) - file.Unresolved[i] = ident - i++ - } - - } - file.Unresolved = file.Unresolved[0:i] - pkg.scope.Outer = Universe // reset outer scope (is nil if there were importErrors) - } - - return -} diff --git a/go/types/resolver_test.go b/go/types/resolver_test.go index 564cd865..0feda178 100644 --- a/go/types/resolver_test.go +++ b/go/types/resolver_test.go @@ -55,11 +55,7 @@ func TestResolveQualifiedIdents(t *testing.T) { fset := token.NewFileSet() var files []*ast.File for _, src := range sources { - mode := parser.AllErrors - if !resolve { - mode |= parser.DeclarationErrors - } - f, err := parser.ParseFile(fset, "", src, mode) + f, err := parser.ParseFile(fset, "", src, parser.DeclarationErrors) if err != nil { t.Fatal(err) } @@ -83,13 +79,7 @@ func TestResolveQualifiedIdents(t *testing.T) { } // check that there are no top-level unresolved identifiers - if !resolve { - for _, f := range files { - for _, x := range f.Unresolved { - t.Errorf("%s: unresolved global identifier %s", fset.Position(x.Pos()), x.Name) - } - } - } + // TODO(gri) add appropriate test code here // check that qualified identifiers are resolved for _, f := range files { @@ -113,32 +103,11 @@ func TestResolveQualifiedIdents(t *testing.T) { }) } - // Currently, the Check API doesn't call Ident for fields, methods, and composite literal keys. + // Currently, the Check API doesn't call Ident for composite literal keys. // Introduce them artifically so that we can run the check below. for _, f := range files { ast.Inspect(f, func(n ast.Node) bool { - switch x := n.(type) { - case *ast.StructType: - if resolve { - break // nothing to do - } - for _, list := range x.Fields.List { - for _, f := range list.Names { - assert(idents[f] == nil) - idents[f] = &Var{pkg: pkg, name: f.Name} - } - } - case *ast.InterfaceType: - if resolve { - break // nothing to do - } - for _, list := range x.Methods.List { - for _, f := range list.Names { - assert(idents[f] == nil) - idents[f] = &Func{pkg: pkg, name: f.Name} - } - } - case *ast.CompositeLit: + if x, ok := n.(*ast.CompositeLit); ok { for _, e := range x.Elts { if kv, ok := e.(*ast.KeyValueExpr); ok { if k, ok := kv.Key.(*ast.Ident); ok { diff --git a/go/types/stdlib_test.go b/go/types/stdlib_test.go index c6fcaae7..639d6bc3 100644 --- a/go/types/stdlib_test.go +++ b/go/types/stdlib_test.go @@ -30,10 +30,6 @@ var ( ) func TestStdlib(t *testing.T) { - if resolve { - fmt.Println("*** Running new code: Identifiers are resolved by type checker. ***") - } - walkDirs(t, filepath.Join(runtime.GOROOT(), "src/pkg")) if *verbose { fmt.Println(pkgCount, "packages typechecked in", time.Since(start)) @@ -52,11 +48,7 @@ func typecheck(t *testing.T, path string, filenames []string) { // parse package files var files []*ast.File for _, filename := range filenames { - mode := parser.AllErrors - if !resolve { - mode |= parser.DeclarationErrors - } - file, err := parser.ParseFile(fset, filename, nil, mode) + file, err := parser.ParseFile(fset, filename, nil, parser.AllErrors) if err != nil { // the parser error may be a list of individual errors; report them all if list, ok := err.(scanner.ErrorList); ok { diff --git a/go/types/stmt.go b/go/types/stmt.go index ee68fb53..3817d515 100644 --- a/go/types/stmt.go +++ b/go/types/stmt.go @@ -95,19 +95,13 @@ func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool, iota // set the object type to Typ[Invalid]. var obj Object var typ Type - - if resolve { - if isConst { - obj = &Const{pos: ident.Pos(), pkg: check.pkg, name: ident.Name} - } else { - obj = &Var{pos: ident.Pos(), pkg: check.pkg, name: ident.Name} - } - check.register(ident, obj) - defer check.declare(check.topScope, obj) - + if isConst { + obj = &Const{pos: ident.Pos(), pkg: check.pkg, name: ident.Name} } else { - obj = check.lookup(ident) + obj = &Var{pos: ident.Pos(), pkg: check.pkg, name: ident.Name} } + check.register(ident, obj) + defer check.declare(check.topScope, obj) switch obj := obj.(type) { default: @@ -176,6 +170,8 @@ func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool, iota } } +// TODO(gri) assignNtoM is only used in one place now. remove and consolidate with other assignment functions. + // assignNtoM typechecks a general assignment. If decl is set, the lhs expressions // must be identifiers; if their types are not set, they are deduced from the types // of the corresponding rhs expressions, or set to Typ[Invalid] in case of an error. @@ -243,17 +239,12 @@ Error: } var obj Object - if resolve { - if isConst { - obj = &Const{pos: ident.Pos(), pkg: check.pkg, name: ident.Name} - } else { - obj = &Var{pos: ident.Pos(), pkg: check.pkg, name: ident.Name} - } - defer check.declare(check.topScope, obj) - + if isConst { + obj = &Const{pos: ident.Pos(), pkg: check.pkg, name: ident.Name} } else { - obj = check.lookup(ident) + obj = &Var{pos: ident.Pos(), pkg: check.pkg, name: ident.Name} } + defer check.declare(check.topScope, obj) switch obj := obj.(type) { case *Const: @@ -318,20 +309,7 @@ func (check *checker) stmt(s ast.Stmt) { // ignore case *ast.DeclStmt: - if resolve { - check.declStmt(s.Decl) - return - } - - d, _ := s.Decl.(*ast.GenDecl) - if d == nil || (d.Tok != token.CONST && d.Tok != token.TYPE && d.Tok != token.VAR) { - check.invalidAST(token.NoPos, "const, type, or var declaration expected") - return - } - if d.Tok == token.CONST { - check.assocInitvals(d) - } - check.decl(d) + check.declStmt(s.Decl) case *ast.LabeledStmt: // TODO(gri) Declare label in the respectice label scope; define Label object. @@ -408,7 +386,7 @@ func (check *checker) stmt(s ast.Stmt) { check.invalidAST(s.Pos(), "missing lhs in assignment") return } - if resolve && s.Tok == token.DEFINE { + if s.Tok == token.DEFINE { // short variable declaration lhs := make([]Object, len(s.Lhs)) for i, x := range s.Lhs { @@ -488,8 +466,8 @@ func (check *checker) stmt(s ast.Stmt) { case *ast.ReturnStmt: sig := check.funcsig if n := sig.results.Len(); n > 0 { - if resolve { - // determine if the function has named results + // determine if the function has named results + { named := false lhs := make([]Object, len(sig.results.vars)) for i, res := range sig.results.vars { @@ -504,6 +482,7 @@ func (check *checker) stmt(s ast.Stmt) { return } } + // TODO(gri) should not have to compute lhs, named every single time - clean this up lhs := make([]ast.Expr, n) named := false // if set, function has named results @@ -635,19 +614,19 @@ func (check *checker) stmt(s ast.Stmt) { check.invalidAST(s.Pos(), "incorrect form of type switch guard") return } + ident, _ := guard.Lhs[0].(*ast.Ident) if ident == nil { check.invalidAST(s.Pos(), "incorrect form of type switch guard") return } - if resolve { - // TODO(gri) in the future, create one of these for each block with the correct type! - lhs = &Var{pkg: check.pkg, name: ident.Name} - check.register(ident, lhs) - } else { - lhs = check.lookup(ident).(*Var) - } + + // TODO(gri) in the future, create one of these for each block with the correct type! + lhs = &Var{pkg: check.pkg, name: ident.Name} + check.register(ident, lhs) + rhs = guard.Rhs[0] + default: check.invalidAST(s.Pos(), "incorrect form of type switch guard") return @@ -670,7 +649,7 @@ func (check *checker) stmt(s ast.Stmt) { return } - if resolve && lhs != nil { + if lhs != nil { check.declare(check.topScope, lhs) } diff --git a/go/types/types_test.go b/go/types/types_test.go index fdf49078..6b088722 100644 --- a/go/types/types_test.go +++ b/go/types/types_test.go @@ -16,11 +16,7 @@ import ( const filename = "" func makePkg(t *testing.T, src string) (*Package, error) { - mode := parser.AllErrors - if !resolve { - mode |= parser.DeclarationErrors - } - file, err := parser.ParseFile(fset, filename, src, mode) + file, err := parser.ParseFile(fset, filename, src, parser.DeclarationErrors) if err != nil { return nil, err } @@ -158,11 +154,7 @@ var testExprs = []testEntry{ func TestExprs(t *testing.T) { for _, test := range testExprs { src := "package p; var _ = " + test.src + "; var (x, y int; s []string; f func(int, float32) int; i interface{}; t interface { foo() })" - mode := parser.AllErrors - if !resolve { - mode |= parser.DeclarationErrors - } - file, err := parser.ParseFile(fset, filename, src, mode) + file, err := parser.ParseFile(fset, filename, src, parser.DeclarationErrors) if err != nil { t.Errorf("%s: %s", src, err) continue