From faef64e27194a95a926689d6010f008f3ad3bf21 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 30 May 2013 09:50:44 -0700 Subject: [PATCH] go.tools/go/types: support for Context.ImplicitObj callbacks Also: - more cleanup of new identifier resolution code - removed residue Object.Pos() code - two separate, equally formatted error messages for redeclaration errors for easier tool support - initial support for labels - enabled several disabled tests Thic CL will break go.tools/ssa/interp, but the pending CL 9863045 fixes that. Fixes golang/go#5504. R=adonovan CC=golang-dev https://golang.org/cl/9839045 --- go/types/api.go | 55 ++++++++++--- go/types/check.go | 10 ++- go/types/check_test.go | 10 ++- go/types/expr.go | 36 +++++---- go/types/objects.go | 145 +++++++++++------------------------ go/types/resolver.go | 142 ++++++++++++++-------------------- go/types/resolver_test.go | 56 +++++--------- go/types/stmt.go | 75 ++++++++---------- go/types/testdata/const0.src | 19 ++++- go/types/testdata/decls0.src | 8 +- go/types/testdata/decls1.src | 12 ++- go/types/testdata/stmt0.src | 23 +++++- go/types/types.go | 9 ++- 13 files changed, 289 insertions(+), 311 deletions(-) diff --git a/go/types/api.go b/go/types/api.go index 87515a998c8..32761144762 100644 --- a/go/types/api.go +++ b/go/types/api.go @@ -2,8 +2,27 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// Package types declares the data structures for representing -// Go types and implements typechecking of package files. +// Package types declares the data types and implements the algorithms for +// name resolution, source-level constant folding, and type checking of Go +// packages. +// +// Name resolution maps each identifier (ast.Ident) in the program to the +// language object (Object) it denotes. +// +// Constant folding computes the exact constant value (exact.Value) for +// every expression (ast.Expr) that is a compile-time constant. +// +// Type checking computes the type (Type) of every expression (ast.Expr) +// and checks for compliance with the language specification. +// +// The results of the various computations are delivered to a client via +// Context callback functions: +// +// Context callback: Information delivered to client: +// +// Ident, ImplicitObj results of name resolution +// Expr results of constant folding and type checking +// Error errors // // WARNING: THE TYPES API IS SUBJECT TO CHANGE. // @@ -45,15 +64,29 @@ type Context struct { // filename:line:column: message Error func(err error) - // If Ident != 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. + // If Ident != nil, it is called for each identifier id in the AST + // (including package names, dots "." of dot-imports, and blank "_" + // identifiers), and obj is the object denoted by ident. The object + // is nil if the identifier was not declared. Ident may be called + // multiple times for the same identifier (e.g., for typed variable + // declarations with multiple initialization statements); but Ident + // will report the same obj for a given id in this case. Ident func(id *ast.Ident, obj Object) + // TODO(gri) Can we make this stronger, so that Ident is called + // always exactly once (w/o resorting to a map, internally)? + // TODO(gri) At the moment, Ident is not called for label identifiers + // in break, continue, or goto statements. + + // If ImplicitObj != nil, it is called exactly once for each node + // that declares an object obj implicitly. The following nodes may + // appear: + // + // node obj + // *ast.ImportSpec *Package (imports w/o renames), or imported objects (dot-imports) + // *ast.CaseClause type-specific variable introduced for each single-type type switch clause + // *ast.Field anonymous struct field or parameter, embedded interface + // + ImplicitObj func(node ast.Node, obj Object) // If Expr != nil, it is called exactly once for each expression x // that is type-checked: typ is the expression type, and val is the @@ -62,8 +95,8 @@ type Context struct { // If x is a literal value (constant, composite literal), typ is always // the dynamic type of x (never an interface type). Otherwise, typ is x's // static type (possibly an interface type). - // TODO(gri): Should this hold for all constant expressions? Expr func(x ast.Expr, typ Type, val exact.Value) + // TODO(gri): Should this hold for all constant expressions? // If Import != nil, it is called for each imported package. // Otherwise, GcImporter is called. diff --git a/go/types/check.go b/go/types/check.go index ccae8263d8d..ca5a2c5fdfe 100644 --- a/go/types/check.go +++ b/go/types/check.go @@ -56,20 +56,24 @@ func (check *checker) closeScope() { check.topScope = check.topScope.Outer } -func (check *checker) register(id *ast.Ident, obj Object) { - // TODO(gri) Document if an identifier can be registered more than once. +func (check *checker) callIdent(id *ast.Ident, obj Object) { if f := check.ctxt.Ident; f != nil { f(id, obj) } } +func (check *checker) callImplicitObj(node ast.Node, obj Object) { + if f := check.ctxt.ImplicitObj; f != nil { + f(node, obj) + } +} + // 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 { for scope := check.topScope; scope != nil; scope = scope.Outer { if obj := scope.Lookup(ident.Name); obj != nil { - check.register(ident, obj) return obj } } diff --git a/go/types/check_test.go b/go/types/check_test.go index dc2dabd1f4b..0fde5ef6781 100644 --- a/go/types/check_test.go +++ b/go/types/check_test.go @@ -32,6 +32,7 @@ import ( "io/ioutil" "os" "regexp" + "strings" "testing" ) @@ -188,7 +189,14 @@ func checkFiles(t *testing.T, testname string, testfiles []string) { // typecheck and collect typechecker errors var ctxt Context - ctxt.Error = func(err error) { errlist = append(errlist, err) } + ctxt.Error = func(err error) { + // Ignore error messages containing "previous declaration": + // They are follow-up error messages after a redeclaration + // error. + if !strings.Contains(err.Error(), "previous declaration") { + errlist = append(errlist, err) + } + } ctxt.Check(testname, fset, files...) if *listErrors { diff --git a/go/types/expr.go b/go/types/expr.go index 768b0fa4be4..397bc5cfe5f 100644 --- a/go/types/expr.go +++ b/go/types/expr.go @@ -7,7 +7,6 @@ package types import ( - "fmt" "go/ast" "go/token" "strconv" @@ -93,9 +92,8 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO if len(field.Names) > 0 { // named parameter for _, name := range field.Names { - par := &Var{pos: name.Pos(), pkg: check.pkg, name: name.Name, typ: typ, decl: field} - check.declare(scope, par) - check.register(name, par) + par := &Var{pos: name.Pos(), pkg: check.pkg, name: name.Name, typ: typ} + check.declare(scope, name, par) last = par copy := *par @@ -103,7 +101,9 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO } } else { // anonymous parameter - par := &Var{pkg: check.pkg, typ: typ, decl: field} + par := &Var{pkg: check.pkg, typ: typ} + check.callImplicitObj(field, par) + last = nil // not accessible inside function params = append(params, par) } @@ -139,13 +139,13 @@ func (check *checker) collectMethods(scope *Scope, list *ast.FieldList) (methods // just a normal declaration obj := &Func{name.Pos(), check.pkg, nil, name.Name, sig, nil} if alt := methods.Insert(obj); alt != nil { - prevDecl := "" + check.errorf(obj.Pos(), "%s redeclared in this block", obj.Name()) if pos := alt.Pos(); pos.IsValid() { - prevDecl = fmt.Sprintf("\n\tprevious declaration at %s", check.fset.Position(pos)) + check.errorf(pos, "previous declaration of %s", obj.Name()) } - check.errorf(obj.Pos(), "%s redeclared in this block%s", obj.Name(), prevDecl) + obj = nil // for callIdent, below } - check.register(name, obj) + check.callIdent(name, obj) } } else { // embedded interface @@ -154,7 +154,9 @@ func (check *checker) collectMethods(scope *Scope, list *ast.FieldList) (methods for _, obj := range ityp.methods.entries { if alt := methods.Insert(obj); alt != nil { check.errorf(list.Pos(), "multiple methods named %s", obj.Name()) + obj = nil // for callImplicit, below } + check.callImplicitObj(f, obj) } } else if utyp != Typ[Invalid] { // if utyp is invalid, don't complain (the root cause was reported before) @@ -193,13 +195,10 @@ func (check *checker) collectFields(scope *Scope, list *ast.FieldList, cycleOk b tags = append(tags, tag) } - fld := &Var{pos: pos, pkg: check.pkg, name: name, typ: typ, decl: field} - check.declare(scope, fld) - if ident != nil { - check.register(ident, fld) - } + fld := &Var{pos: pos, pkg: check.pkg, name: name, typ: typ} + check.declare(scope, ident, fld) - fields = append(fields, &Field{check.pkg, name, typ, isAnonymous}) + fields = append(fields, &Field{check.pkg, name, typ, isAnonymous, fld}) } for _, f := range list.List { @@ -1047,6 +1046,7 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle case *ast.Ident: obj := check.lookup(e) + check.callIdent(e, obj) if obj == nil { if e.Name == "_" { check.invalidOp(e.Pos(), "cannot use _ as value or type") @@ -1090,7 +1090,7 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle case *TypeName: x.mode = typexpr if !cycleOk && obj.typ.Underlying() == nil { - check.errorf(obj.spec.Pos(), "illegal cycle in declaration of %s", obj.name) + check.errorf(obj.pos, "illegal cycle in declaration of %s", obj.name) x.expr = e x.typ = Typ[Invalid] return // don't goto Error - need x.mode == typexpr @@ -1177,6 +1177,7 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle check.errorf(kv.Pos(), "unknown field %s in struct literal", key.Name) continue } + check.callIdent(key, fields[i].obj) // 0 <= i < len(fields) if visited[i] { check.errorf(kv.Pos(), "duplicate field name %s in struct literal", key.Name) @@ -1279,6 +1280,7 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle // selector expressions. if ident, ok := e.X.(*ast.Ident); ok { if pkg, ok := check.lookup(ident).(*Package); ok { + check.callIdent(ident, pkg) exp := pkg.scope.Lookup(sel) if exp == nil { check.errorf(e.Pos(), "%s not declared by package %s", sel, ident) @@ -1290,7 +1292,7 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle check.errorf(e.Pos(), "%s not exported by package %s", sel, ident) goto Error } - check.register(e.Sel, exp) + check.callIdent(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/go/types/objects.go b/go/types/objects.go index ce2a3862f64..de8a9ab07f8 100644 --- a/go/types/objects.go +++ b/go/types/objects.go @@ -12,15 +12,14 @@ 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. +// TODO(gri) All objects looks very similar now. Maybe just have single Object struct with an object kind? // An Object describes a named language entity such as a package, // constant, type, variable, function (incl. methods), or label. // All objects implement the Object interface. // type Object interface { - Pkg() *Package // nil for objects in the Universe scope + Pkg() *Package // nil for objects in the Universe scope and labels Outer() *Scope // the scope in which this object is declared Name() string Type() Type @@ -39,28 +38,18 @@ type Package struct { scope *Scope // package-level scope imports map[string]*Package // map of import paths to imported packages complete bool // if set, this package was imported completely - - spec *ast.ImportSpec } func NewPackage(path, name string) *Package { return &Package{name: name, path: path, complete: true} } -func (obj *Package) Pkg() *Package { return obj } -func (obj *Package) Outer() *Scope { return obj.outer } -func (obj *Package) Scope() *Scope { return obj.scope } -func (obj *Package) Name() string { return obj.name } -func (obj *Package) Type() Type { return Typ[Invalid] } -func (obj *Package) Pos() token.Pos { - if obj.pos.IsValid() { - return obj.pos - } - if obj.spec != nil { - return obj.spec.Pos() - } - return token.NoPos -} +func (obj *Package) Pkg() *Package { return obj } +func (obj *Package) Outer() *Scope { return obj.outer } +func (obj *Package) Scope() *Scope { return obj.scope } +func (obj *Package) Name() string { return obj.name } +func (obj *Package) Type() Type { return Typ[Invalid] } +func (obj *Package) Pos() token.Pos { return obj.pos } func (obj *Package) Path() string { return obj.path } func (obj *Package) Imports() map[string]*Package { return obj.imports } func (obj *Package) Complete() bool { return obj.complete } @@ -77,27 +66,13 @@ type Const struct { val exact.Value visited bool // for initialization cycle detection - spec *ast.ValueSpec } -func (obj *Const) Pkg() *Package { return obj.pkg } -func (obj *Const) Outer() *Scope { return obj.outer } -func (obj *Const) Name() string { return obj.name } -func (obj *Const) Type() Type { return obj.typ } -func (obj *Const) Pos() token.Pos { - if obj.pos.IsValid() { - return obj.pos - } - if obj.spec == nil { - return token.NoPos - } - for _, n := range obj.spec.Names { - if n.Name == obj.name { - return n.Pos() - } - } - return token.NoPos -} +func (obj *Const) Pkg() *Package { return obj.pkg } +func (obj *Const) Outer() *Scope { return obj.outer } +func (obj *Const) Name() string { return obj.name } +func (obj *Const) Type() Type { return obj.typ } +func (obj *Const) Pos() token.Pos { return obj.pos } func (obj *Const) Val() exact.Value { return obj.val } func (obj *Const) setOuter(s *Scope) { obj.outer = s } @@ -108,27 +83,17 @@ type TypeName struct { outer *Scope name string typ Type // *Named or *Basic - - spec *ast.TypeSpec } func NewTypeName(pkg *Package, name string, typ Type) *TypeName { - return &TypeName{token.NoPos, pkg, nil, name, typ, nil} + return &TypeName{token.NoPos, pkg, nil, name, typ} } -func (obj *TypeName) Pkg() *Package { return obj.pkg } -func (obj *TypeName) Outer() *Scope { return obj.outer } -func (obj *TypeName) Name() string { return obj.name } -func (obj *TypeName) Type() Type { return obj.typ } -func (obj *TypeName) Pos() token.Pos { - if obj.pos.IsValid() { - return obj.pos - } - if obj.spec == nil { - return token.NoPos - } - return obj.spec.Pos() -} +func (obj *TypeName) Pkg() *Package { return obj.pkg } +func (obj *TypeName) Outer() *Scope { return obj.outer } +func (obj *TypeName) Name() string { return obj.name } +func (obj *TypeName) Type() Type { return obj.typ } +func (obj *TypeName) Pos() token.Pos { return obj.pos } func (obj *TypeName) setOuter(s *Scope) { obj.outer = s } // A Variable represents a declared variable (including function parameters and results). @@ -140,43 +105,17 @@ type Var struct { typ Type visited bool // for initialization cycle detection - decl interface{} } func NewVar(pkg *Package, name string, typ Type) *Var { - return &Var{token.NoPos, pkg, nil, name, typ, false, nil} + return &Var{token.NoPos, pkg, nil, name, typ, false} } -func (obj *Var) Pkg() *Package { return obj.pkg } -func (obj *Var) Outer() *Scope { return obj.outer } -func (obj *Var) Name() string { return obj.name } -func (obj *Var) Type() Type { return obj.typ } -func (obj *Var) Pos() token.Pos { - if obj.pos.IsValid() { - return obj.pos - } - switch d := obj.decl.(type) { - case *ast.Field: - for _, n := range d.Names { - if n.Name == obj.name { - return n.Pos() - } - } - case *ast.ValueSpec: - for _, n := range d.Names { - if n.Name == obj.name { - return n.Pos() - } - } - case *ast.AssignStmt: - for _, x := range d.Lhs { - if ident, isIdent := x.(*ast.Ident); isIdent && ident.Name == obj.name { - return ident.Pos() - } - } - } - return token.NoPos -} +func (obj *Var) Pkg() *Package { return obj.pkg } +func (obj *Var) Outer() *Scope { return obj.outer } +func (obj *Var) Name() string { return obj.name } +func (obj *Var) Type() Type { return obj.typ } +func (obj *Var) Pos() token.Pos { return obj.pos } func (obj *Var) setOuter(s *Scope) { obj.outer = s } // A Func represents a declared function. @@ -187,20 +126,26 @@ type Func struct { name string typ Type // *Signature or *Builtin - decl *ast.FuncDecl + decl *ast.FuncDecl // TODO(gri) can we get rid of this field? } -func (obj *Func) Pkg() *Package { return obj.pkg } -func (obj *Func) Outer() *Scope { return obj.outer } -func (obj *Func) Name() string { return obj.name } -func (obj *Func) Type() Type { return obj.typ } -func (obj *Func) Pos() token.Pos { - if obj.pos.IsValid() { - return obj.pos - } - if obj.decl != nil && obj.decl.Name != nil { - return obj.decl.Name.Pos() - } - return token.NoPos -} +func (obj *Func) Pkg() *Package { return obj.pkg } +func (obj *Func) Outer() *Scope { return obj.outer } +func (obj *Func) Name() string { return obj.name } +func (obj *Func) Type() Type { return obj.typ } +func (obj *Func) Pos() token.Pos { return obj.pos } func (obj *Func) setOuter(s *Scope) { obj.outer = s } + +// A Label represents a declared label. +type Label struct { + pos token.Pos + outer *Scope + name string +} + +func (obj *Label) Pkg() *Package { return nil } +func (obj *Label) Outer() *Scope { return obj.outer } +func (obj *Label) Name() string { return obj.name } +func (obj *Label) Type() Type { return nil } +func (obj *Label) Pos() token.Pos { return obj.pos } +func (obj *Label) setOuter(s *Scope) { obj.outer = s } diff --git a/go/types/resolver.go b/go/types/resolver.go index 4c8d7e07621..7fb8acea9b6 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -5,7 +5,6 @@ package types import ( - "fmt" "go/ast" "go/token" "strconv" @@ -13,18 +12,19 @@ import ( "code.google.com/p/go.tools/go/exact" ) -func (check *checker) declare(scope *Scope, obj Object) { +func (check *checker) declare(scope *Scope, id *ast.Ident, obj Object) { if obj.Name() == "_" { + // blank identifiers are not declared obj.setOuter(scope) - return // blank identifiers are not visible - } - if alt := scope.Insert(obj); alt != nil { - prevDecl := "" + } else if alt := scope.Insert(obj); alt != nil { + check.errorf(obj.Pos(), "%s redeclared in this block", obj.Name()) if pos := alt.Pos(); pos.IsValid() { - prevDecl = fmt.Sprintf("\n\tprevious declaration at %s", check.fset.Position(pos)) + check.errorf(pos, "previous declaration of %s", obj.Name()) } - check.errorf(obj.Pos(), "%s redeclared in this block%s", obj.Name(), prevDecl) - // TODO(gri) Instead, change this into two separate error messages (easier to handle by tools) + obj = nil // for callIdent below + } + if id != nil { + check.callIdent(id, obj) } } @@ -84,7 +84,7 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { for _, file := range files { // the package identifier denotes the current package, but it is in no scope - check.register(file.Name, pkg) + check.callIdent(file.Name, pkg) fileScope = &Scope{Outer: pkg.scope} scopes = append(scopes, fileScope) @@ -100,19 +100,14 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { switch s := spec.(type) { case *ast.ImportSpec: if importer == nil { - //importErrors = true continue } path, _ := strconv.Unquote(s.Path.Value) imp, err := importer(pkg.imports, path) if err != nil { check.errorf(s.Path.Pos(), "could not import %s (%s)", path, err) - //importErrors = true continue } - // TODO(gri) If a local package name != "." is provided, - // we could proceed even if the import failed. Consider - // adjusting the logic here a bit. // local name overrides imported package name name := imp.name @@ -120,6 +115,13 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { name = s.Name.Name } + imp2 := &Package{name: name, scope: imp.scope} + if s.Name != nil { + check.callIdent(s.Name, imp2) + } else { + check.callImplicitObj(s, imp2) + } + // add import to file scope if name == "." { // merge imported scope with file scope @@ -130,20 +132,13 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { if ast.IsExported(obj.Name()) { // Note: This will change each imported object's scope! // May be an issue for types aliases. - check.declare(fileScope, obj) + check.declare(fileScope, nil, obj) + check.callImplicitObj(s, 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 != "_" { + } else { // 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: s} - check.declare(fileScope, obj) + check.declare(fileScope, nil, imp2) } case *ast.ValueSpec: @@ -156,9 +151,8 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { // declare all constants for i, name := range s.Names { - obj := &Const{pos: name.Pos(), pkg: pkg, name: name.Name, val: exact.MakeInt64(int64(iota)), spec: s} - check.declare(pkg.scope, obj) - check.register(name, obj) + obj := &Const{pos: name.Pos(), pkg: pkg, name: name.Name, val: exact.MakeInt64(int64(iota))} + check.declare(pkg.scope, name, obj) var init ast.Expr if i < len(last.Values) { @@ -171,15 +165,11 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { if lhs, rhs := len(s.Names), len(s.Values); rhs > 0 { switch { case lhs < rhs: - // TODO(gri) once resolve is the default, use first message - // x := s.Values[lhs] - // check.errorf(x.Pos(), "too many initialization expressions") - check.errorf(s.Names[0].Pos(), "assignment count mismatch") + x := s.Values[lhs] + check.errorf(x.Pos(), "too many initialization expressions") case lhs > rhs && rhs != 1: - // TODO(gri) once resolve is the default, use first message - // n := s.Names[rhs] - // check.errorf(n.Pos(), "missing initialization expression for %s", n) - check.errorf(s.Names[0].Pos(), "assignment count mismatch") + n := s.Names[rhs] + check.errorf(n.Pos(), "missing initialization expression for %s", n) } } @@ -187,10 +177,9 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { // declare all variables lhs := make([]*Var, len(s.Names)) for i, name := range s.Names { - obj := &Var{pos: name.Pos(), pkg: pkg, name: name.Name, decl: s} + obj := &Var{pos: name.Pos(), pkg: pkg, name: name.Name} lhs[i] = obj - check.declare(pkg.scope, obj) - check.register(name, obj) + check.declare(pkg.scope, name, obj) var init ast.Expr switch len(s.Values) { @@ -208,19 +197,15 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { add(obj, s.Type, init) } - // arity of lhs and rhs must match + // report if there are too many initialization expressions if lhs, rhs := len(s.Names), len(s.Values); rhs > 0 { switch { case lhs < rhs: - // TODO(gri) once resolve is the default, use first message - // x := s.Values[lhs] - // check.errorf(x.Pos(), "too many initialization expressions") - check.errorf(s.Names[0].Pos(), "assignment count mismatch") + x := s.Values[lhs] + check.errorf(x.Pos(), "too many initialization expressions") case lhs > rhs && rhs != 1: - // TODO(gri) once resolve is the default, use first message - // n := s.Names[rhs] - // check.errorf(n.Pos(), "missing initialization expression for %s", n) - check.errorf(s.Names[0].Pos(), "assignment count mismatch") + n := s.Names[rhs] + check.errorf(n.Pos(), "missing initialization expression for %s", n) } } @@ -229,10 +214,9 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { } case *ast.TypeSpec: - obj := &TypeName{pos: s.Name.Pos(), pkg: pkg, name: s.Name.Name, spec: s} - check.declare(pkg.scope, obj) + obj := &TypeName{pos: s.Name.Pos(), pkg: pkg, name: s.Name.Name} + check.declare(pkg.scope, s.Name, obj) add(obj, s.Type, nil) - check.register(s.Name, obj) default: check.invalidAST(s.Pos(), "unknown ast.Spec node %T", s) @@ -249,10 +233,10 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { if obj.name == "init" { // init functions are not visible - don't declare them in package scope obj.outer = pkg.scope + check.callIdent(d.Name, obj) } else { - check.declare(pkg.scope, obj) + check.declare(pkg.scope, d.Name, obj) } - check.register(d.Name, obj) add(obj, nil, nil) default: @@ -317,8 +301,7 @@ func (check *checker) resolveFiles(files []*ast.File, importer Importer) { check.methods[tname] = scope } fun := &Func{pos: m.Name.Pos(), pkg: check.pkg, name: m.Name.Name, decl: m} - check.declare(scope, fun) - check.register(m.Name, fun) + check.declare(scope, m.Name, fun) // HACK(gri) change method outer scope to file scope containing the declaration fun.outer = meth.file // remember the file scope } @@ -379,9 +362,7 @@ func (check *checker) declareConst(obj *Const, typ, init ast.Expr) { var x operand if init == nil { - // TODO(gri) enable error message once resolve is default - // check.errorf(obj.Pos(), "missing initialization expression for %s", obj.name) - goto Error + goto Error // error reported before } check.expr(&x, init, nil, int(iota)) @@ -415,11 +396,9 @@ func (check *checker) declareVar(obj *Var, typ, init ast.Expr) { if init == nil { if typ == nil { - // TODO(gri) enable error message once resolve is default - // check.errorf(obj.Pos(), "missing type or initialization expression for %s", obj.name) obj.typ = Typ[Invalid] } - return + return // error reported before } // unpack projection expression, if any @@ -566,7 +545,8 @@ func (check *checker) declStmt(decl ast.Decl) { // declare all constants lhs := make([]*Const, len(s.Names)) for i, name := range s.Names { - obj := &Const{pos: name.Pos(), pkg: pkg, name: name.Name, val: exact.MakeInt64(int64(iota)), spec: s} + obj := &Const{pos: name.Pos(), pkg: pkg, name: name.Name, val: exact.MakeInt64(int64(iota))} + check.callIdent(name, obj) lhs[i] = obj var init ast.Expr @@ -575,7 +555,6 @@ func (check *checker) declStmt(decl ast.Decl) { } check.declareConst(obj, s.Type, init) - check.register(name, obj) } // arity of lhs and rhs must match @@ -589,16 +568,16 @@ func (check *checker) declStmt(decl ast.Decl) { } for _, obj := range lhs { - check.declare(check.topScope, obj) + check.declare(check.topScope, nil, obj) } case token.VAR: // declare all variables lhs := make([]*Var, len(s.Names)) for i, name := range s.Names { - obj := &Var{pos: name.Pos(), pkg: pkg, name: name.Name, decl: s} + obj := &Var{pos: name.Pos(), pkg: pkg, name: name.Name} + check.callIdent(name, obj) lhs[i] = obj - check.register(name, obj) } // iterate in 2 phases because declareVar requires fully initialized lhs! @@ -621,23 +600,19 @@ func (check *checker) declStmt(decl ast.Decl) { } // arity of lhs and rhs must match - // TODO(gri) Disabled for now to match existing test errors. - // These error messages are better than what we have now. - /* - if lhs, rhs := len(s.Names), len(s.Values); rhs > 0 { - switch { - case lhs < rhs: - x := s.Values[lhs] - check.errorf(x.Pos(), "too many initialization expressions") - case lhs > rhs && rhs != 1: - n := s.Names[rhs] - check.errorf(n.Pos(), "missing initialization expression for %s", n) - } + if lhs, rhs := len(s.Names), len(s.Values); rhs > 0 { + switch { + case lhs < rhs: + x := s.Values[lhs] + check.errorf(x.Pos(), "too many initialization expressions") + case lhs > rhs && rhs != 1: + n := s.Names[rhs] + check.errorf(n.Pos(), "missing initialization expression for %s", n) } - */ + } for _, obj := range lhs { - check.declare(check.topScope, obj) + check.declare(check.topScope, nil, obj) } default: @@ -645,10 +620,9 @@ func (check *checker) declStmt(decl ast.Decl) { } case *ast.TypeSpec: - obj := &TypeName{pos: s.Name.Pos(), pkg: pkg, name: s.Name.Name, spec: s} - check.declare(check.topScope, obj) + obj := &TypeName{pos: s.Name.Pos(), pkg: pkg, name: s.Name.Name} + check.declare(check.topScope, s.Name, obj) check.declareType(obj, s.Type, false) - check.register(s.Name, obj) default: check.invalidAST(s.Pos(), "const, type, or var declaration expected") diff --git a/go/types/resolver_test.go b/go/types/resolver_test.go index 0feda178939..53e6fdb87c1 100644 --- a/go/types/resolver_test.go +++ b/go/types/resolver_test.go @@ -5,6 +5,7 @@ package types import ( + "fmt" "go/ast" "go/parser" "go/token" @@ -54,8 +55,8 @@ func TestResolveQualifiedIdents(t *testing.T) { // parse package files fset := token.NewFileSet() var files []*ast.File - for _, src := range sources { - f, err := parser.ParseFile(fset, "", src, parser.DeclarationErrors) + for i, src := range sources { + f, err := parser.ParseFile(fset, fmt.Sprintf("sources[%d]", i), src, parser.DeclarationErrors) if err != nil { t.Fatal(err) } @@ -65,7 +66,12 @@ func TestResolveQualifiedIdents(t *testing.T) { // resolve and type-check package AST idents := make(map[*ast.Ident]Object) var ctxt Context - ctxt.Ident = func(id *ast.Ident, obj Object) { idents[id] = obj } + ctxt.Ident = func(id *ast.Ident, obj Object) { + if old, found := idents[id]; found && old != obj { + t.Errorf("%s: identifier %s reported multiple times with different objects", fset.Position(id.Pos()), id.Name) + } + idents[id] = obj + } pkg, err := ctxt.Check("testResolveQualifiedIdents", fset, files...) if err != nil { t.Fatal(err) @@ -78,9 +84,6 @@ func TestResolveQualifiedIdents(t *testing.T) { } } - // check that there are no top-level unresolved identifiers - // TODO(gri) add appropriate test code here - // check that qualified identifiers are resolved for _, f := range files { ast.Inspect(f, func(n ast.Node) bool { @@ -103,33 +106,14 @@ func TestResolveQualifiedIdents(t *testing.T) { }) } - // 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 { - 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 { - assert(idents[k] == nil) - idents[k] = &Var{pkg: pkg, 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 { + if x, ok := n.(*ast.Ident); ok { + if _, found := idents[x]; found { delete(idents, x) + } else { + t.Errorf("%s: unresolved identifier %s", fset.Position(x.Pos()), x.Name) } return false } @@ -137,12 +121,10 @@ func TestResolveQualifiedIdents(t *testing.T) { }) } - // 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) - } - */ + // any left-over identifiers didn't exist in the source + for x := range idents { + t.Errorf("%s: identifier %s not present in source", fset.Position(x.Pos()), x.Name) + } + + // TODO(gri) add tests to check ImplicitObj callbacks } diff --git a/go/types/stmt.go b/go/types/stmt.go index 3817d51552c..3b5c0906f4a 100644 --- a/go/types/stmt.go +++ b/go/types/stmt.go @@ -63,6 +63,7 @@ func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool, iota if !decl { // anything can be assigned to the blank identifier if ident != nil && ident.Name == "_" { + check.callIdent(ident, nil) // the rhs has its final type check.updateExprType(rhs, x.typ, true) return @@ -100,17 +101,13 @@ func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool, iota } else { obj = &Var{pos: ident.Pos(), pkg: check.pkg, name: ident.Name} } - check.register(ident, obj) - defer check.declare(check.topScope, obj) + defer check.declare(check.topScope, ident, obj) + // TODO(gri) remove this switch, combine with code above switch obj := obj.(type) { default: unreachable() - case nil: - // TODO(gri) is this really unreachable? - unreachable() - case *Const: typ = obj.typ // may already be Typ[Invalid] if typ == nil { @@ -244,7 +241,7 @@ Error: } else { obj = &Var{pos: ident.Pos(), pkg: check.pkg, name: ident.Name} } - defer check.declare(check.topScope, obj) + defer check.declare(check.topScope, ident, obj) switch obj := obj.(type) { case *Const: @@ -312,7 +309,13 @@ func (check *checker) stmt(s ast.Stmt) { check.declStmt(s.Decl) case *ast.LabeledStmt: - // TODO(gri) Declare label in the respectice label scope; define Label object. + scope := check.funcsig.labels + if scope == nil { + scope = new(Scope) // no label scope chain + check.funcsig.labels = scope + } + label := s.Label + check.declare(scope, label, &Label{pos: label.Pos(), name: label.Name}) check.stmt(s.Stmt) case *ast.ExprStmt: @@ -390,20 +393,18 @@ func (check *checker) stmt(s ast.Stmt) { // short variable declaration lhs := make([]Object, len(s.Lhs)) for i, x := range s.Lhs { - var obj *Var + var obj Object if ident, ok := x.(*ast.Ident); ok { - obj = &Var{pos: ident.Pos(), pkg: check.pkg, name: ident.Name, decl: s} - // If the variable is already declared (redeclaration in :=), - // register the identifier with the existing variable. + // use the correct obj if the ident is redeclared + obj = &Var{pos: ident.Pos(), pkg: check.pkg, name: ident.Name} if alt := check.topScope.Lookup(ident.Name); alt != nil { - check.register(ident, alt) - } else { - check.register(ident, obj) + obj = alt } + check.callIdent(ident, obj) } else { check.errorf(x.Pos(), "cannot declare %s", x) // create a dummy variable - obj = &Var{pos: x.Pos(), pkg: check.pkg, name: "_", decl: s} + obj = &Var{pos: x.Pos(), pkg: check.pkg, name: "_"} } lhs[i] = obj } @@ -493,7 +494,6 @@ func (check *checker) stmt(s ast.Stmt) { } name := ast.NewIdent(res.name) name.NamePos = s.Pos() - check.register(name, &Var{name: res.name, typ: res.typ}) // Pkg == nil lhs[i] = name } if len(s.Results) > 0 || !named { @@ -532,7 +532,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.register(ident, Universe.Lookup("true")) + check.callIdent(ident, Universe.Lookup("true")) tag = ident } check.expr(&x, tag, nil, -1) @@ -604,7 +604,7 @@ func (check *checker) stmt(s ast.Stmt) { // remaining syntactic errors are considered AST errors here. // TODO(gri) better factoring of error handling (invalid ASTs) // - var lhs *Var // lhs variable or nil + var lhs *ast.Ident // lhs identifier or nil var rhs ast.Expr switch guard := s.Assign.(type) { case *ast.ExprStmt: @@ -615,16 +615,12 @@ func (check *checker) stmt(s ast.Stmt) { return } - ident, _ := guard.Lhs[0].(*ast.Ident) - if ident == nil { + lhs, _ = guard.Lhs[0].(*ast.Ident) + if lhs == nil { check.invalidAST(s.Pos(), "incorrect form of type switch guard") return } - // 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: @@ -649,8 +645,10 @@ func (check *checker) stmt(s ast.Stmt) { return } + var obj Object if lhs != nil { - check.declare(check.topScope, lhs) + obj = &Var{pos: lhs.Pos(), pkg: check.pkg, name: lhs.Name, typ: x.typ} + check.declare(check.topScope, lhs, obj) } check.multipleDefaults(s.Body.List) @@ -676,26 +674,19 @@ func (check *checker) stmt(s ast.Stmt) { } } } - // If lhs exists, set its type for each clause. - if lhs != nil { - // In clauses with a case listing exactly one type, the variable has that type; - // otherwise, the variable has the type of the expression in the TypeSwitchGuard. - if len(clause.List) != 1 || typ == nil { - typ = x.typ - } - lhs.typ = typ - } check.openScope() + // If lhs exists, declare a corresponding object in the case-local scope if necessary. + if lhs != nil { + // A single-type case clause implicitly declares a new variable shadowing lhs. + if len(clause.List) == 1 && typ != nil { + obj := &Var{pos: lhs.Pos(), pkg: check.pkg, name: lhs.Name, typ: typ} + check.declare(check.topScope, nil, obj) + check.callImplicitObj(clause, obj) + } + } check.stmtList(clause.Body) check.closeScope() } - - // There is only one object (lhs) associated with a lhs identifier, but that object - // assumes different types for different clauses. Set it back to the type of the - // TypeSwitchGuard expression so that that variable always has a valid type. - if lhs != nil { - lhs.typ = x.typ - } check.closeScope() case *ast.SelectStmt: diff --git a/go/types/testdata/const0.src b/go/types/testdata/const0.src index f87b85d9ac7..1eb46e234d0 100644 --- a/go/types/testdata/const0.src +++ b/go/types/testdata/const0.src @@ -180,8 +180,8 @@ const ( const ( a1, a2, a3 = 7, 3.1415926, "foo" b1, b2, b3 = b3, b1, 42 - c1 /* ERROR "assignment count mismatch" */, c2, c3 = 1, 2 - d1 /* ERROR "assignment count mismatch" */, d2, d3 = 1, 2, 3, 4 + c1, c2, c3 /* ERROR "missing initialization" */ = 1, 2 + d1, d2, d3 = 1, 2, 3, 4 /* ERROR "too many initialization" */ _p0 = assert(a1 == 7) _p1 = assert(a2 == 3.1415926) _p2 = assert(a3 == "foo") @@ -190,6 +190,21 @@ const ( _p5 = assert(b3 == 42) ) +func _() { + const ( + a1, a2, a3 = 7, 3.1415926, "foo" + b1, b2, b3 = b3, b1, 42 + c1, c2, c3 /* ERROR "missing initialization" */ = 1, 2 + d1, d2, d3 = 1, 2, 3, 4 /* ERROR "too many initialization" */ + _p0 = assert(a1 == 7) + _p1 = assert(a2 == 3.1415926) + _p2 = assert(a3 == "foo") + _p3 = assert(b1 == 42) + _p4 = assert(b2 == 42) + _p5 = assert(b3 == 42) + ) +} + // iota const ( iota0 = iota diff --git a/go/types/testdata/decls0.src b/go/types/testdata/decls0.src index 68843f9ada6..0c742435bef 100644 --- a/go/types/testdata/decls0.src +++ b/go/types/testdata/decls0.src @@ -69,13 +69,7 @@ type ( Pi pi /* ERROR "not a type" */ a /* ERROR "illegal cycle" */ a - // TODO(gri) For now we get double redeclaration errors if the redeclaration - // happens at the package level in the same file. This is because one check - // is done in the parser and the other one in the type checker. We cannot - // easily avoid this w/o losing the check or functionality. This will be - // fixed once all resolution is done in the type checker. - // TODO(gri) Disabled for now since this needs to work with the old and new resolver. - // a /* ERROR "redeclared" */ /* ERROR "redeclared" */ int + a /* ERROR "redeclared" */ int // where the cycle error appears depends on the // order in which declarations are processed diff --git a/go/types/testdata/decls1.src b/go/types/testdata/decls1.src index d538140848e..a45c2ad7e7c 100644 --- a/go/types/testdata/decls1.src +++ b/go/types/testdata/decls1.src @@ -100,10 +100,18 @@ var ( // Multiple assignment expressions var ( m1a, m1b = 1, 2 - m2a /* ERROR "assignment count mismatch" */ , m2b, m2c = 1, 2 - m3a /* ERROR "assignment count mismatch" */ , m3b = 1, 2, 3 + m2a, m2b, m2c /* ERROR "missing initialization" */ = 1, 2 + m3a, m3b = 1, 2, 3 /* ERROR "too many initialization" */ ) +func _() { + var ( + m1a, m1b = 1, 2 + m2a, m2b, m2c /* ERROR "missing initialization" */ = 1, 2 + m3a, m3b = 1, 2, 3 /* ERROR "too many initialization" */ + ) +} + // Declaration of parameters and results func f0() {} func f1(a /* ERROR "not a type" */) {} diff --git a/go/types/testdata/stmt0.src b/go/types/testdata/stmt0.src index af0d8061f23..f506338ea56 100644 --- a/go/types/testdata/stmt0.src +++ b/go/types/testdata/stmt0.src @@ -189,12 +189,14 @@ func typeswitches() { } } +// Test that each case clause uses the correct type of the variable +// declared by the type switch (issue 5504). func typeswitch0() { switch y := interface{}(nil).(type) { case int: - // TODO(gri) y has the wrong type here (type-checking - // of captured variable is delayed) - // func() int { return y + 0 }() + func() int { return y + 0 }() + case float32: + func() float32 { return y }() } } @@ -286,3 +288,18 @@ func rangeloops() { xx = x } } + +func labels0() { + L0: + L1: + L1 /* ERROR "redeclared" */: + if true { + L2: + L0 /* ERROR "redeclared" */: + } + _ = func() { + L0: + L1: + L2: + } +} \ No newline at end of file diff --git a/go/types/types.go b/go/types/types.go index 3482bfea46d..6d02c4c1b40 100644 --- a/go/types/types.go +++ b/go/types/types.go @@ -8,6 +8,7 @@ import "go/ast" // TODO(gri) Separate struct fields below into transient (used during type checking only) // and permanent fields. +// TODO(gri) Revisit factory functions - make sure they have all relevant parameters. // A Type represents a type of Go. // All types implement the Type interface. @@ -130,12 +131,15 @@ func NewSlice(elem Type) *Slice { return &Slice{elem} } func (s *Slice) Elem() Type { return s.elt } // A Field represents a field of a struct. -// TODO(gri): Should make this just a Var? type Field struct { Pkg *Package Name string Type Type IsAnonymous bool + + // TODO(gri) Temporary hack so we can report the correct *Var via Context.Ident. + // Make Field just this *Var and we can simplify a lot of struct code. + obj *Var } // A Struct represents a struct type. @@ -235,6 +239,7 @@ func (t *Tuple) ForEach(f func(*Var)) { // A Signature represents a (non-builtin) function type. type Signature struct { scope *Scope // function scope + labels *Scope // label scope, or nil (lazily allocated) recv *Var // nil if not a method params *Tuple // (incoming) parameters from left to right; or nil results *Tuple // (outgoing) results from left to right; or nil @@ -245,7 +250,7 @@ type Signature struct { // and results, either of which may be nil. If isVariadic is set, the function // is variadic. func NewSignature(recv *Var, params, results *Tuple, isVariadic bool) *Signature { - return &Signature{nil, recv, params, results, isVariadic} + return &Signature{nil, nil, recv, params, results, isVariadic} } // Recv returns the receiver of signature s, or nil.