From 56b9e46247a88d0efe316362880daabe96ecb555 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 6 Aug 2013 11:27:05 -0700 Subject: [PATCH] go.tools/go/types: fewer guarantees in case of type errors Provide fewer guarantees regarding the collected result type information in types.Info if there are type errors. Specifically, don't record nil objects in the Objects map in case of a declaration error; instead, simply omit them. R=adonovan TBR=adonovan CC=golang-dev https://golang.org/cl/12553043 --- go/types/api.go | 34 +++++++++++++++------------------- go/types/assignments.go | 4 +++- go/types/call.go | 2 +- go/types/gcimporter.go | 1 - go/types/lookup.go | 2 +- go/types/resolver.go | 26 +++++++++----------------- go/types/typexpr.go | 2 +- 7 files changed, 30 insertions(+), 41 deletions(-) diff --git a/go/types/api.go b/go/types/api.go index 6c8f7648..c49a79c7 100644 --- a/go/types/api.go +++ b/go/types/api.go @@ -89,29 +89,28 @@ type Config struct { Sizeof func(Type) int64 } -// Info holds result type information for a package. +// Info holds result type information for a type-checked package. +// Only the information for which a map is provided is collected. +// If the package has type errors, the collected information may +// be incomplete. type Info struct { - // If Types != nil, it records the expression and corresponding type - // for each expression that is type-checked. Identifiers declaring a - // variable are recorded in Objects, not Types. + // Types maps expressions to their types. Identifiers on the + // lhs of declarations are collected in Objects, not Types. Types map[ast.Expr]Type - // If Values != nil, it records the expression and corresponding value - // for each constant expression that is type-checked. + // Values maps constant expressions to their values. Values map[ast.Expr]exact.Value - // If Objects != nil, it records the identifier and corresponding object - // for each identifier that is type-checked (including package names, - // dots "." of dot-imports, and blank "_" identifiers). For identifiers - // that are not declared (due to an error, or because they are symbolic - // as the t in t := x.(type) of a type switch header), the corresponding - // object is nil. + // Objects maps identifiers to their corresponding objects (including + // package names, dots "." of dot-imports, and blank "_" identifiers). + // For identifiers that do not denote objects (e.g., blank identifiers + // on the lhs of assignments, or symbolic variables t in t := x.(type) + // of type switch headers), the corresponding objects are nil. // BUG(gri) Label identifiers in break, continue, or goto statements - // are not recorded. + // are not yet mapped. Objects map[*ast.Ident]Object - // If Implicits != nil, it records the node and corresponding object for - // each node that is type-checked and that implicitly declared an object. + // Implicits maps nodes to their implicitly declared objects, if any. // The following node and object types may appear: // // node declared object @@ -122,10 +121,7 @@ type Info struct { // Implicits map[ast.Node]Object - // If Selections != nil, it records the selector expression and corresponding - // selection, i.e., package object (qualified identifier), struct field (field - // selector), or method (method expression or value) for each selector expression - // that is type-checked. + // Selections maps selector expressions to their corresponding selections. Selections map[*ast.SelectorExpr]*Selection } diff --git a/go/types/assignments.go b/go/types/assignments.go index d7912539..569d46fe 100644 --- a/go/types/assignments.go +++ b/go/types/assignments.go @@ -306,7 +306,9 @@ func (check *checker) shortVarDecl(lhs, rhs []ast.Expr) { // declare new variable obj = NewVar(ident.Pos(), check.pkg, ident.Name, nil) } - check.recordObject(ident, obj) // obj may be nil + if obj != nil { + check.recordObject(ident, obj) + } } else { check.errorf(lhs.Pos(), "cannot declare %s", lhs) } diff --git a/go/types/call.go b/go/types/call.go index 6b914699..d98964fe 100644 --- a/go/types/call.go +++ b/go/types/call.go @@ -169,7 +169,7 @@ func (check *checker) selector(x *operand, e *ast.SelectorExpr) { // can only appear in qualified identifiers which are mapped to // selector expressions. if ident, ok := e.X.(*ast.Ident); ok { - if pkg, ok := check.topScope.LookupParent(ident.Name).(*Package); ok { + if pkg, _ := check.topScope.LookupParent(ident.Name).(*Package); pkg != nil { check.recordObject(ident, pkg) exp := pkg.scope.Lookup(sel) if exp == nil { diff --git a/go/types/gcimporter.go b/go/types/gcimporter.go index f4b70d0a..fa76d46f 100644 --- a/go/types/gcimporter.go +++ b/go/types/gcimporter.go @@ -107,7 +107,6 @@ func GcImportData(imports map[string]*Package, filename, id string, data *bufio. // corresponding package object to the imports map, and returns the object. // Local import paths are interpreted relative to the current working directory. // The imports map must contains all packages already imported. -// GcImport satisfies the ast.Importer signature. // func GcImport(imports map[string]*Package, path string) (pkg *Package, err error) { if path == "unsafe" { diff --git a/go/types/lookup.go b/go/types/lookup.go index d7ebd38b..048ac477 100644 --- a/go/types/lookup.go +++ b/go/types/lookup.go @@ -236,7 +236,7 @@ func MissingMethod(typ Type, T *Interface, static bool) (method *Func, wrongType return } - // TODO(gri) Consider using methods sets here. Might be more efficient. + // TODO(gri) Consider using method sets here. Might be more efficient. if ityp, _ := typ.Underlying().(*Interface); ityp != nil { for _, m := range T.methods { diff --git a/go/types/resolver.go b/go/types/resolver.go index 290d7cbc..7846694e 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -27,7 +27,7 @@ func (check *checker) declareObj(scope *Scope, id *ast.Ident, obj Object) { if alt := scope.Insert(obj); alt != nil { check.errorf(obj.Pos(), "%s redeclared in this block", obj.Name()) check.reportAltDecl(alt) - obj = nil + return } if id != nil { check.recordObject(id, obj) @@ -38,7 +38,7 @@ func (check *checker) declareFld(oset *objset, id *ast.Ident, obj Object) { if alt := oset.insert(obj); alt != nil { check.errorf(obj.Pos(), "%s redeclared", obj.Name()) check.reportAltDecl(alt) - obj = nil + return } if id != nil { check.recordObject(id, obj) @@ -131,7 +131,6 @@ func (check *checker) resolveFiles(files []*ast.File) { // spec: "A package-scope or file-scope identifier with name init // may only be declared to be a function with this (func()) signature." if ident.Name == "init" { - check.recordObject(ident, nil) check.errorf(ident.Pos(), "cannot declare init - must be func") return } @@ -529,8 +528,6 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, cycleOk // check each method for _, m := range methods { - ident := check.objMap[m].fdecl.Name - if alt := mset.insert(m); alt != nil { switch alt.(type) { case *Var: @@ -541,19 +538,14 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, cycleOk unreachable() } check.reportAltDecl(alt) - m = nil + continue } - - // If the method is valid, type-check its signature, - // and collect it with the named base type. - if m != nil { - check.recordObject(ident, m) - check.objDecl(m, nil, true) - // Methods with blank _ names cannot be found. - // Don't add them to the method list. - if m.name != "_" { - named.methods = append(named.methods, m) - } + check.recordObject(check.objMap[m].fdecl.Name, m) + check.objDecl(m, nil, true) + // Methods with blank _ names cannot be found. + // Don't add them to the method list. + if m.name != "_" { + named.methods = append(named.methods, m) } } diff --git a/go/types/typexpr.go b/go/types/typexpr.go index cc480668..cde211d4 100644 --- a/go/types/typexpr.go +++ b/go/types/typexpr.go @@ -23,7 +23,6 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool) x.expr = e obj := check.topScope.LookupParent(e.Name) - check.recordObject(e, obj) if obj == nil { if e.Name == "_" { check.errorf(e.Pos(), "cannot use _ as value or type") @@ -32,6 +31,7 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool) } return } + check.recordObject(e, obj) typ := obj.Type() if typ == nil {