]> Cypherpunks repositories - gostls13.git/commitdiff
go/types: various minor fixes
authorRobert Griesemer <gri@golang.org>
Mon, 14 Jan 2013 23:19:32 +0000 (15:19 -0800)
committerRobert Griesemer <gri@golang.org>
Mon, 14 Jan 2013 23:19:32 +0000 (15:19 -0800)
- always set the Pkg field in QualifiedIdents
- call Context.Ident for all identifiers in the AST that denote
  a types.Object (bug fix)
- added test that Context.Ident is called for all such identifiers

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

src/pkg/go/types/api.go
src/pkg/go/types/check.go
src/pkg/go/types/expr.go
src/pkg/go/types/resolve.go
src/pkg/go/types/resolver_test.go
src/pkg/go/types/types.go
src/pkg/go/types/universe.go

index 502958000e9671d9c40f05660b5ee5f50a892a69..e5b6aa12c011d3028ada393a46cf0e3cf75e35dc 100644 (file)
@@ -5,7 +5,7 @@
 // Package types declares the data structures for representing
 // Go types and implements typechecking of package files.
 //
-// WARNING: THE TYPES API IS SUBJECT TO SIGNIFICANT CHANGE.
+// WARNING: THE TYPES API IS SUBJECT TO CHANGE.
 //
 package types
 
@@ -25,9 +25,14 @@ type Context struct {
        // 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.
+       // If Ident is not nil, it is called for each identifier id
+       // denoting an Object in the files provided to Check, and
+       // obj is the denoted object.
+       // Ident is not called for fields and methods in struct or
+       // interface types or composite literals, or for blank (_)
+       // or dot (.) identifiers in dot-imports.
+       // TODO(gri) Consider making Fields and Methods ordinary
+       // Objects - than we could lift this restriction.
        Ident func(id *ast.Ident, obj Object)
 
        // If Expr is not nil, it is called for each expression x that is
index 3b13b0fcd30a59fe4dbf46167b010a1e0de4c33f..fb9d7573d5792147e1a47bf959fe26277e6b8936 100644 (file)
@@ -70,9 +70,9 @@ func (check *checker) lookup(ident *ast.Ident) Object {
 
        if obj = check.objects[astObj]; obj == nil {
                obj = newObj(astObj)
-               check.register(ident, obj)
                check.objects[astObj] = obj
        }
+       check.register(ident, obj)
 
        return obj
 }
@@ -256,7 +256,7 @@ func (check *checker) object(obj Object, cycleOk bool) {
                                params, _ := check.collectParams(m.decl.Recv, false)
                                sig.Recv = params[0] // the parser/assocMethod ensure there is exactly one parameter
                                m.Type = sig
-                               methods = append(methods, &Method{QualifiedName{nil, m.Name}, sig})
+                               methods = append(methods, &Method{QualifiedName{check.pkg, m.Name}, sig})
                                check.later(m, sig, m.decl.Body)
                        }
                        typ.Methods = methods
index 9a4b6c605165d3d8561c10b7e2795e658145a181..f475cacc5a0d3ef89728effe687a7c0d123ae01a 100644 (file)
@@ -84,7 +84,7 @@ func (check *checker) collectMethods(list *ast.FieldList) (methods []*Method) {
                                continue
                        }
                        for _, name := range f.Names {
-                               methods = append(methods, &Method{QualifiedName{nil, name.Name}, sig})
+                               methods = append(methods, &Method{QualifiedName{check.pkg, name.Name}, sig})
                        }
                } else {
                        // embedded interface
@@ -137,15 +137,15 @@ func (check *checker) collectFields(list *ast.FieldList, cycleOk bool) (fields [
                if len(f.Names) > 0 {
                        // named fields
                        for _, name := range f.Names {
-                               fields = append(fields, &Field{QualifiedName{nil, name.Name}, typ, tag, false})
+                               fields = append(fields, &Field{QualifiedName{check.pkg, name.Name}, typ, tag, false})
                        }
                } else {
                        // anonymous field
                        switch t := deref(typ).(type) {
                        case *Basic:
-                               fields = append(fields, &Field{QualifiedName{nil, t.Name}, typ, tag, true})
+                               fields = append(fields, &Field{QualifiedName{check.pkg, t.Name}, typ, tag, true})
                        case *NamedType:
-                               fields = append(fields, &Field{QualifiedName{nil, t.Obj.GetName()}, typ, tag, true})
+                               fields = append(fields, &Field{QualifiedName{check.pkg, t.Obj.GetName()}, typ, tag, true})
                        default:
                                if typ != Typ[Invalid] {
                                        check.invalidAST(f.Type.Pos(), "anonymous field type %s must be named", typ)
@@ -902,7 +902,7 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle
                if x.mode == invalid {
                        goto Error
                }
-               mode, typ := lookupField(x.typ, QualifiedName{nil, sel})
+               mode, typ := lookupField(x.typ, QualifiedName{check.pkg, sel})
                if mode == invalid {
                        check.invalidOp(e.Pos(), "%s has no single field or method %s", x, sel)
                        goto Error
index b314b7add4b6ae77471c5490e76bfb81363ea4e0..112818f79047c2c99e792df8d88a94a0a4535e5c 100644 (file)
@@ -37,18 +37,17 @@ func (check *checker) resolveIdent(scope *Scope, ident *ast.Ident) bool {
 }
 
 func (check *checker) resolve(importer Importer) (pkg *Package, methods []*ast.FuncDecl) {
-       // complete package scope
-       pkgName := ""
-       pkgScope := &Scope{Outer: Universe}
+       pkg = &Package{Scope: &Scope{Outer: Universe}, Imports: make(map[string]*Package)}
 
+       // complete package scope
        i := 0
        for _, file := range check.files {
                // package names must match
                switch name := file.Name.Name; {
-               case pkgName == "":
-                       pkgName = name
-               case name != pkgName:
-                       check.errorf(file.Package, "package %s; expected %s", name, pkgName)
+               case pkg.Name == "":
+                       pkg.Name = name
+               case name != pkg.Name:
+                       check.errorf(file.Package, "package %s; expected %s", name, pkg.Name)
                        continue // ignore this file
                }
 
@@ -56,6 +55,9 @@ func (check *checker) resolve(importer Importer) (pkg *Package, methods []*ast.F
                check.files[i] = file
                i++
 
+               // 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)
                for _, decl := range file.Decls {
@@ -75,13 +77,13 @@ func (check *checker) resolve(importer Importer) (pkg *Package, methods []*ast.F
                                                        if name.Name == "_" {
                                                                continue
                                                        }
-                                                       pkgScope.Insert(check.lookup(name))
+                                                       pkg.Scope.Insert(check.lookup(name))
                                                }
                                        case *ast.TypeSpec:
                                                if s.Name.Name == "_" {
                                                        continue
                                                }
-                                               pkgScope.Insert(check.lookup(s.Name))
+                                               pkg.Scope.Insert(check.lookup(s.Name))
                                        default:
                                                check.invalidAST(s.Pos(), "unknown ast.Spec node %T", s)
                                        }
@@ -95,7 +97,7 @@ func (check *checker) resolve(importer Importer) (pkg *Package, methods []*ast.F
                                if d.Name.Name == "_" || d.Name.Name == "init" {
                                        continue // blank (_) and init functions are inaccessible
                                }
-                               pkgScope.Insert(check.lookup(d.Name))
+                               pkg.Scope.Insert(check.lookup(d.Name))
                        default:
                                check.invalidAST(d.Pos(), "unknown ast.Decl node %T", d)
                        }
@@ -103,21 +105,18 @@ func (check *checker) resolve(importer Importer) (pkg *Package, methods []*ast.F
        }
        check.files = check.files[0:i]
 
-       // package global mapping of imported package ids to package objects
-       imports := make(map[string]*Package)
-
        // complete file scopes with imports and resolve identifiers
        for _, file := range check.files {
                // build file scope by processing all imports
                importErrors := false
-               fileScope := &Scope{Outer: pkgScope}
+               fileScope := &Scope{Outer: pkg.Scope}
                for _, spec := range file.Imports {
                        if importer == nil {
                                importErrors = true
                                continue
                        }
                        path, _ := strconv.Unquote(spec.Path.Value)
-                       pkg, err := importer(imports, path)
+                       imp, err := importer(pkg.Imports, path)
                        if err != nil {
                                check.errorf(spec.Path.Pos(), "could not import %s (%s)", path, err)
                                importErrors = true
@@ -128,7 +127,7 @@ func (check *checker) resolve(importer Importer) (pkg *Package, methods []*ast.F
                        // import failed. Consider adjusting the logic here a bit.
 
                        // local name overrides imported package name
-                       name := pkg.Name
+                       name := imp.Name
                        if spec.Name != nil {
                                name = spec.Name.Name
                        }
@@ -136,16 +135,20 @@ func (check *checker) resolve(importer Importer) (pkg *Package, methods []*ast.F
                        // add import to file scope
                        if name == "." {
                                // merge imported scope with file scope
-                               for _, obj := range pkg.Scope.Entries {
-                                       check.declareObj(fileScope, pkgScope, obj)
+                               for _, obj := range imp.Scope.Entries {
+                                       check.declareObj(fileScope, pkg.Scope, obj)
                                }
+                               // 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 pkg in the file scope but create
+                               // (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: pkg.Scope, spec: spec}
-                               check.declareObj(fileScope, pkgScope, obj)
+                               obj := &Package{Name: name, Scope: imp.Scope, spec: spec}
+                               check.declareObj(fileScope, pkg.Scope, obj)
                        }
                }
 
@@ -155,7 +158,7 @@ func (check *checker) resolve(importer Importer) (pkg *Package, methods []*ast.F
                        // (objects in the universe may be shadowed by imports;
                        // with missing imports, identifiers might get resolved
                        // incorrectly to universe objects)
-                       pkgScope.Outer = nil
+                       pkg.Scope.Outer = nil
                }
                i := 0
                for _, ident := range file.Unresolved {
@@ -167,8 +170,8 @@ func (check *checker) resolve(importer Importer) (pkg *Package, methods []*ast.F
 
                }
                file.Unresolved = file.Unresolved[0:i]
-               pkgScope.Outer = Universe // reset outer scope
+               pkg.Scope.Outer = Universe // reset outer scope (is nil if there were importErrors)
        }
 
-       return &Package{Name: pkgName, Scope: pkgScope, Imports: imports}, methods
+       return
 }
index 40fe21fc4cad627e523af6fbea9222d605c9d5c1..985d9a7c04dd085f1c709f92d2419d2426e6ea88 100644 (file)
@@ -12,7 +12,8 @@ import (
 )
 
 var sources = []string{
-       `package p
+       `
+       package p
        import "fmt"
        import "math"
        const pi = math.Pi
@@ -21,16 +22,27 @@ var sources = []string{
        }
        var Println = fmt.Println
        `,
-       `package p
+       `
+       package p
        import "fmt"
        func f() string {
+               _ = "foo"
                return fmt.Sprintf("%d", g())
        }
        func g() (x int) { return }
        `,
-       `package p
+       `
+       package p
        import . "go/parser"
-       func g() Mode { return ImportsOnly }`,
+       import "sync"
+       func g() Mode { return ImportsOnly }
+       var _, x int = 1, 2
+       func init() {}
+       type T struct{ sync.Mutex; a, b, c int}
+       type I interface{ m() }
+       var _ = T{a: 1, b: 2, c: 3}
+       func (_ T) m() {}
+       `,
 }
 
 var pkgnames = []string{
@@ -94,4 +106,62 @@ func TestResolveQualifiedIdents(t *testing.T) {
                        return true
                })
        }
+
+       // Currently, the Check API doesn't call Ident for fields, methods, and 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:
+                               for _, list := range x.Fields.List {
+                                       for _, f := range list.Names {
+                                               assert(idents[f] == nil)
+                                               idents[f] = &Var{Name: f.Name}
+                                       }
+                               }
+                       case *ast.InterfaceType:
+                               for _, list := range x.Methods.List {
+                                       for _, f := range list.Names {
+                                               assert(idents[f] == nil)
+                                               idents[f] = &Func{Name: f.Name}
+                                       }
+                               }
+                       case *ast.CompositeLit:
+                               for _, e := range x.Elts {
+                                       if kv, ok := e.(*ast.KeyValueExpr); ok {
+                                               if k, ok := kv.Key.(*ast.Ident); ok {
+                                                       assert(idents[k] == nil)
+                                                       idents[k] = &Var{Name: k.Name}
+                                               }
+                                       }
+                               }
+                       }
+                       return true
+               })
+       }
+
+       // check that each identifier in the source is enumerated by the Context.Ident callback
+       for _, f := range files {
+               ast.Inspect(f, func(n ast.Node) bool {
+                       if x, ok := n.(*ast.Ident); ok && x.Name != "_" && x.Name != "." {
+                               obj := idents[x]
+                               if obj == nil {
+                                       t.Errorf("%s: unresolved identifier %s", fset.Position(x.Pos()), x.Name)
+                               } else {
+                                       delete(idents, x)
+                               }
+                               return false
+                       }
+                       return true
+               })
+       }
+
+       // TODO(gri) enable code below
+       // At the moment, the type checker introduces artifical identifiers which are not
+       // present in the source. Once it doesn't do that anymore, enable the checks below.
+       /*
+               for x := range idents {
+                       t.Errorf("%s: identifier %s not present in source", fset.Position(x.Pos()), x.Name)
+               }
+       */
 }
index 3894825b2b99239b85b147153fbfdb00141b5a25..65daad5cfa6d7c169026b99e1fc37cde147bcc26 100644 (file)
@@ -90,7 +90,7 @@ type Slice struct {
 
 // A QualifiedName is a name qualified with the package that declared the name.
 type QualifiedName struct {
-       Pkg  *Package // nil for current (non-imported) package
+       Pkg  *Package // nil only for predeclared error.Error
        Name string   // unqualified type name for anonymous fields
 }
 
@@ -104,19 +104,7 @@ func (p QualifiedName) IsSame(q QualifiedName) bool {
                return false
        }
        // p.Name == q.Name
-       if !ast.IsExported(p.Name) {
-               // TODO(gri) just compare packages once we guarantee that they are canonicalized
-               pp := ""
-               if p.Pkg != nil {
-                       pp = p.Pkg.Path
-               }
-               qp := ""
-               if q.Pkg != nil {
-                       qp = q.Pkg.Path
-               }
-               return pp == qp
-       }
-       return true
+       return ast.IsExported(p.Name) || p.Pkg == q.Pkg
 }
 
 // A Field represents a field of a struct.
index 9668aa8a34d4a01ed146ca091cee91c6337a38fd..8e9f6aaa09f4ddca3d88aecaac374df684886348 100644 (file)
@@ -97,7 +97,8 @@ func init() {
 
        // error type
        {
-               err := &Method{QualifiedName{Name: "Error"}, &Signature{Results: []*Var{{Name: "", Type: Typ[String]}}}}
+               // Error has a nil package in its qualified name since it is in no package
+               err := &Method{QualifiedName{nil, "Error"}, &Signature{Results: []*Var{{Name: "", Type: Typ[String]}}}}
                def(&TypeName{Name: "error", Type: &NamedType{Underlying: &Interface{Methods: []*Method{err}}}})
        }