From f2db24a319eae4c252712e9616927ece673bdbb6 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 11 Jul 2014 10:50:09 +0100 Subject: [PATCH] go.tools/go/loader: use new types.TypeAndValue mode predicates. PackageInfo: - deleted IsType - inlined + deleted: ValueOf, TypeCaseVar, ImportSpecPkg - on failure, TypeOf accessor now returns nil (was: panic) go/ssa: avoid extra map lookups by using Uses or Defs directly when safe to do so, and keeping the TypeAndValue around in expr0(). LGTM=gri R=gri, pcc CC=golang-codereviews https://golang.org/cl/107650043 --- go/loader/pkginfo.go | 82 ++++++-------------------------------------- go/ssa/builder.go | 66 +++++++++++++++++++---------------- go/ssa/create.go | 8 ++--- go/ssa/func.go | 8 ++--- go/ssa/ssa.go | 16 ++++----- oracle/callees.go | 4 +-- oracle/describe.go | 25 +++++++++----- oracle/freevars.go | 8 ++--- refactor/eg/eg.go | 2 +- refactor/eg/match.go | 2 +- 10 files changed, 84 insertions(+), 137 deletions(-) diff --git a/go/loader/pkginfo.go b/go/loader/pkginfo.go index 467ae5aed7..564d7188a8 100644 --- a/go/loader/pkginfo.go +++ b/go/loader/pkginfo.go @@ -8,7 +8,6 @@ import ( "fmt" "go/ast" - "code.google.com/p/go.tools/go/exact" "code.google.com/p/go.tools/go/types" ) @@ -32,90 +31,29 @@ func (info *PackageInfo) String() string { return fmt.Sprintf("PackageInfo(%s)", info.Pkg.Path()) } -// TypeOf returns the type of expression e. -// Precondition: e belongs to the package's ASTs. -// +// TODO(gri): move the methods below to types.Info. + +// TypeOf returns the type of expression e, or nil if not found. func (info *PackageInfo) TypeOf(e ast.Expr) types.Type { if t, ok := info.Types[e]; ok { return t.Type } - // Defining ast.Idents (id := expr) get only Ident callbacks - // but not Expr callbacks. + // Idents appear only in Defs/Uses, not Types. if id, ok := e.(*ast.Ident); ok { return info.ObjectOf(id).Type() } - panic("no type for expression") + return nil } -// ValueOf returns the value of expression e if it is a constant, nil -// otherwise. -// Precondition: e belongs to the package's ASTs. +// ObjectOf returns the type-checker object denoted by the specified +// id, or nil if not found. // -func (info *PackageInfo) ValueOf(e ast.Expr) exact.Value { - return info.Types[e].Value -} - -// ObjectOf returns the typechecker object denoted by the specified id. -// -// If id is an anonymous struct field, the field (*types.Var) is -// returned, not the type (*types.TypeName). -// -// Precondition: id belongs to the package's ASTs. +// If id is an anonymous struct field, ObjectOf returns the field +// (*types.Var) it uses, not the type (*types.TypeName) it defines. // func (info *PackageInfo) ObjectOf(id *ast.Ident) types.Object { - obj, ok := info.Defs[id] - if ok { + if obj, ok := info.Defs[id]; ok { return obj } return info.Uses[id] } - -// IsType returns true iff expression e denotes a type. -// Precondition: e belongs to the package's ASTs. -// -// TODO(gri): move this into go/types. -// -func (info *PackageInfo) IsType(e ast.Expr) bool { - switch e := e.(type) { - case *ast.SelectorExpr: // pkg.Type - if _, ok := info.Selections[e]; !ok { - // qualified identifier - _, isType := info.Uses[e.Sel].(*types.TypeName) - return isType - } - case *ast.StarExpr: // *T - return info.IsType(e.X) - case *ast.Ident: - _, isType := info.ObjectOf(e).(*types.TypeName) - return isType - case *ast.ArrayType, *ast.StructType, *ast.FuncType, *ast.InterfaceType, *ast.MapType, *ast.ChanType: - return true - case *ast.ParenExpr: - return info.IsType(e.X) - } - return false -} - -// TypeCaseVar returns the implicit variable created by a single-type -// case clause in a type switch, or nil if not found. -// -func (info *PackageInfo) TypeCaseVar(cc *ast.CaseClause) *types.Var { - if v := info.Implicits[cc]; v != nil { - return v.(*types.Var) - } - return nil -} - -// ImportSpecPkg returns the PkgName for a given ImportSpec, possibly -// an implicit one for a dot-import or an import-without-rename. -// It returns nil if not found. -// -func (info *PackageInfo) ImportSpecPkg(spec *ast.ImportSpec) *types.PkgName { - if spec.Name != nil { - return info.ObjectOf(spec.Name).(*types.PkgName) - } - if p := info.Implicits[spec]; p != nil { - return p.(*types.PkgName) - } - return nil -} diff --git a/go/ssa/builder.go b/go/ssa/builder.go index 9f16261385..8607202ea5 100644 --- a/go/ssa/builder.go +++ b/go/ssa/builder.go @@ -446,19 +446,23 @@ func (b *builder) exprInPlace(fn *Function, loc lvalue, e ast.Expr, isZero bool) // to fn and returning the Value defined by the expression. // func (b *builder) expr(fn *Function, e ast.Expr) Value { - // Is expression a constant? - if v := fn.Pkg.info.ValueOf(e); v != nil { - return NewConst(v, fn.Pkg.typeOf(e)) - } e = unparen(e) - v := b.expr0(fn, e) + + tv := fn.Pkg.info.Types[e] + + // Is expression a constant? + if tv.Value != nil { + return NewConst(tv.Value, tv.Type) + } + + v := b.expr0(fn, e, tv) if fn.debugInfo() { emitDebugRef(fn, e, v, false) } return v } -func (b *builder) expr0(fn *Function, e ast.Expr) Value { +func (b *builder) expr0(fn *Function, e ast.Expr, tv types.TypeAndValue) Value { switch e := e.(type) { case *ast.BasicLit: panic("non-constant BasicLit") // unreachable @@ -479,7 +483,7 @@ func (b *builder) expr0(fn *Function, e ast.Expr) Value { return fn2 } v := &MakeClosure{Fn: fn2} - v.setType(fn.Pkg.typeOf(e)) + v.setType(tv.Type) for _, fv := range fn2.FreeVars { v.Bindings = append(v.Bindings, fv.outer) fv.outer = nil @@ -487,14 +491,13 @@ func (b *builder) expr0(fn *Function, e ast.Expr) Value { return fn.emit(v) case *ast.TypeAssertExpr: // single-result form only - return emitTypeAssert(fn, b.expr(fn, e.X), fn.Pkg.typeOf(e), e.Lparen) + return emitTypeAssert(fn, b.expr(fn, e.X), tv.Type, e.Lparen) case *ast.CallExpr: - typ := fn.Pkg.typeOf(e) - if fn.Pkg.info.IsType(e.Fun) { + if fn.Pkg.info.Types[e.Fun].IsType() { // Explicit type conversion, e.g. string(x) or big.Int(x) x := b.expr(fn, e.Args[0]) - y := emitConv(fn, x, typ) + y := emitConv(fn, x, tv.Type) if y != x { switch y := y.(type) { case *Convert: @@ -509,8 +512,8 @@ func (b *builder) expr0(fn *Function, e ast.Expr) Value { } // Call to "intrinsic" built-ins, e.g. new, make, panic. if id, ok := unparen(e.Fun).(*ast.Ident); ok { - if obj, ok := fn.Pkg.objectOf(id).(*types.Builtin); ok { - if v := b.builtin(fn, obj, e.Args, typ, e.Lparen); v != nil { + if obj, ok := fn.Pkg.info.Uses[id].(*types.Builtin); ok { + if v := b.builtin(fn, obj, e.Args, tv.Type, e.Lparen); v != nil { return v } } @@ -518,7 +521,7 @@ func (b *builder) expr0(fn *Function, e ast.Expr) Value { // Regular function call. var v Call b.setCall(fn, e, &v.Call) - v.setType(typ) + v.setType(tv.Type) return fn.emit(&v) case *ast.UnaryExpr: @@ -541,7 +544,7 @@ func (b *builder) expr0(fn *Function, e ast.Expr) Value { X: b.expr(fn, e.X), } v.setPos(e.OpPos) - v.setType(fn.Pkg.typeOf(e)) + v.setType(tv.Type) return fn.emit(v) default: panic(e.Op) @@ -554,12 +557,12 @@ func (b *builder) expr0(fn *Function, e ast.Expr) Value { case token.SHL, token.SHR: fallthrough case token.ADD, token.SUB, token.MUL, token.QUO, token.REM, token.AND, token.OR, token.XOR, token.AND_NOT: - return emitArith(fn, e.Op, b.expr(fn, e.X), b.expr(fn, e.Y), fn.Pkg.typeOf(e), e.OpPos) + return emitArith(fn, e.Op, b.expr(fn, e.X), b.expr(fn, e.Y), tv.Type, e.OpPos) case token.EQL, token.NEQ, token.GTR, token.LSS, token.LEQ, token.GEQ: cmp := emitCompare(fn, e.Op, b.expr(fn, e.X), b.expr(fn, e.Y), e.OpPos) // The type of x==y may be UntypedBool. - return emitConv(fn, cmp, DefaultType(fn.Pkg.typeOf(e))) + return emitConv(fn, cmp, DefaultType(tv.Type)) default: panic("illegal op in BinaryExpr: " + e.Op.String()) } @@ -592,17 +595,17 @@ func (b *builder) expr0(fn *Function, e ast.Expr) Value { Max: max, } v.setPos(e.Lbrack) - v.setType(fn.Pkg.typeOf(e)) + v.setType(tv.Type) return fn.emit(v) case *ast.Ident: - obj := fn.Pkg.objectOf(e) + obj := fn.Pkg.info.Uses[e] // Universal built-in or nil? switch obj := obj.(type) { case *types.Builtin: - return &Builtin{name: obj.Name(), sig: fn.Pkg.typeOf(e).(*types.Signature)} + return &Builtin{name: obj.Name(), sig: tv.Type.(*types.Signature)} case *types.Nil: - return nilConst(fn.Pkg.typeOf(e)) + return nilConst(tv.Type) } // Package-level func or var? if v := fn.Prog.packageLevelValue(obj); v != nil { @@ -624,7 +627,7 @@ func (b *builder) expr0(fn *Function, e ast.Expr) Value { case types.MethodExpr: // (*T).f or T.f, the method f from the method-set of type T. // The result is a "thunk". - return emitConv(fn, makeThunk(fn.Prog, sel), fn.Pkg.typeOf(e)) + return emitConv(fn, makeThunk(fn.Prog, sel), tv.Type) case types.MethodVal: // e.f where e is an expression and f is a method. @@ -645,7 +648,7 @@ func (b *builder) expr0(fn *Function, e ast.Expr) Value { Bindings: []Value{v}, } c.setPos(e.Sel.Pos()) - c.setType(fn.Pkg.typeOf(e)) + c.setType(tv.Type) return fn.emit(c) case types.FieldVal: @@ -952,10 +955,7 @@ func (b *builder) assignStmt(fn *Function, lhss, rhss []ast.Expr, isDef bool) { var lval lvalue = blank{} if !isBlankIdent(lhs) { if isDef { - // Local may be "redeclared" in the same - // scope, so don't blindly create anew. - obj := fn.Pkg.objectOf(lhs.(*ast.Ident)) - if _, ok := fn.objects[obj]; !ok { + if obj := fn.Pkg.info.Defs[lhs.(*ast.Ident)]; obj != nil { fn.addNamedLocal(obj) } } @@ -1310,7 +1310,7 @@ func (b *builder) typeSwitchStmt(fn *Function, s *ast.TypeSwitchStmt, label *lbl } func (b *builder) typeCaseBody(fn *Function, cc *ast.CaseClause, x Value, done *BasicBlock) { - if obj := fn.Pkg.info.TypeCaseVar(cc); obj != nil { + if obj := fn.Pkg.info.Implicits[cc]; obj != nil { // In a switch y := x.(type), each case clause // implicitly declares a distinct object y. // In a single-type case, y has that type. @@ -2115,7 +2115,7 @@ func (b *builder) buildFuncDecl(pkg *Package, decl *ast.FuncDecl) { v.setType(types.NewTuple()) pkg.init.emit(&v) } else { - fn = pkg.values[pkg.objectOf(id)].(*Function) + fn = pkg.values[pkg.info.Defs[id]].(*Function) } b.buildFunction(fn) } @@ -2250,6 +2250,7 @@ func (p *Package) Build() { } } +// Like ObjectOf, but panics instead of returning nil. // Only valid during p's create and build phases. func (p *Package) objectOf(id *ast.Ident) types.Object { if o := p.info.ObjectOf(id); o != nil { @@ -2259,9 +2260,14 @@ func (p *Package) objectOf(id *ast.Ident) types.Object { id.Name, p.Prog.Fset.Position(id.Pos()))) } +// Like TypeOf, but panics instead of returning nil. // Only valid during p's create and build phases. func (p *Package) typeOf(e ast.Expr) types.Type { - return p.info.TypeOf(e) + if T := p.info.TypeOf(e); T != nil { + return T + } + panic(fmt.Sprintf("no type for %T @ %s", + e, p.Prog.Fset.Position(e.Pos()))) } // needMethodsOf ensures that runtime type information (including the diff --git a/go/ssa/create.go b/go/ssa/create.go index e3191f04a5..d48b2d3c86 100644 --- a/go/ssa/create.go +++ b/go/ssa/create.go @@ -131,7 +131,7 @@ func membersFromDecl(pkg *Package, decl ast.Decl) { for _, spec := range decl.Specs { for _, id := range spec.(*ast.ValueSpec).Names { if !isBlankIdent(id) { - memberFromObject(pkg, pkg.objectOf(id), nil) + memberFromObject(pkg, pkg.info.Defs[id], nil) } } } @@ -140,7 +140,7 @@ func membersFromDecl(pkg *Package, decl ast.Decl) { for _, spec := range decl.Specs { for _, id := range spec.(*ast.ValueSpec).Names { if !isBlankIdent(id) { - memberFromObject(pkg, pkg.objectOf(id), spec) + memberFromObject(pkg, pkg.info.Defs[id], spec) } } } @@ -149,7 +149,7 @@ func membersFromDecl(pkg *Package, decl ast.Decl) { for _, spec := range decl.Specs { id := spec.(*ast.TypeSpec).Name if !isBlankIdent(id) { - memberFromObject(pkg, pkg.objectOf(id), nil) + memberFromObject(pkg, pkg.info.Defs[id], nil) } } } @@ -160,7 +160,7 @@ func membersFromDecl(pkg *Package, decl ast.Decl) { return // no object } if !isBlankIdent(id) { - memberFromObject(pkg, pkg.objectOf(id), decl) + memberFromObject(pkg, pkg.info.Defs[id], decl) } } } diff --git a/go/ssa/func.go b/go/ssa/func.go index 5e364da139..0d19a7df55 100644 --- a/go/ssa/func.go +++ b/go/ssa/func.go @@ -224,7 +224,7 @@ func (f *Function) createSyntacticParams(recv *ast.FieldList, functype *ast.Func if recv != nil { for _, field := range recv.List { for _, n := range field.Names { - f.addSpilledParam(f.Pkg.objectOf(n)) + f.addSpilledParam(f.Pkg.info.Defs[n]) } // Anonymous receiver? No need to spill. if field.Names == nil { @@ -238,7 +238,7 @@ func (f *Function) createSyntacticParams(recv *ast.FieldList, functype *ast.Func n := len(f.Params) // 1 if has recv, 0 otherwise for _, field := range functype.Params.List { for _, n := range field.Names { - f.addSpilledParam(f.Pkg.objectOf(n)) + f.addSpilledParam(f.Pkg.info.Defs[n]) } // Anonymous parameter? No need to spill. if field.Names == nil { @@ -307,7 +307,7 @@ func (f *Function) finishBody() { f.syntax = extentNode{n.Pos(), n.End()} } - // Remove any f.Locals that are now heap-allocated. + // Remove from f.Locals any Allocs that escape to the heap. j := 0 for _, l := range f.Locals { if !l.Heap { @@ -393,7 +393,7 @@ func (f *Function) addNamedLocal(obj types.Object) *Alloc { } func (f *Function) addLocalForIdent(id *ast.Ident) *Alloc { - return f.addNamedLocal(f.Pkg.objectOf(id)) + return f.addNamedLocal(f.Pkg.info.Defs[id]) } // addLocal creates an anonymous local variable of type typ, adds it diff --git a/go/ssa/ssa.go b/go/ssa/ssa.go index 58f3563efe..99417b316a 100644 --- a/go/ssa/ssa.go +++ b/go/ssa/ssa.go @@ -298,14 +298,14 @@ type Function struct { Signature *types.Signature pos token.Pos - Synthetic string // provenance of synthetic function; "" for true source functions - syntax ast.Node // *ast.Func{Decl,Lit}; replaced with simple ast.Node after build, unless debug mode - parent *Function // enclosing function if anon; nil if global - Pkg *Package // enclosing package; nil for shared funcs (wrappers and error.Error) - Prog *Program // enclosing program - Params []*Parameter // function parameters; for methods, includes receiver - FreeVars []*FreeVar // free variables whose values must be supplied by closure - Locals []*Alloc + Synthetic string // provenance of synthetic function; "" for true source functions + syntax ast.Node // *ast.Func{Decl,Lit}; replaced with simple ast.Node after build, unless debug mode + parent *Function // enclosing function if anon; nil if global + Pkg *Package // enclosing package; nil for shared funcs (wrappers and error.Error) + Prog *Program // enclosing program + Params []*Parameter // function parameters; for methods, includes receiver + FreeVars []*FreeVar // free variables whose values must be supplied by closure + Locals []*Alloc // local variables of this function Blocks []*BasicBlock // basic blocks of the function; nil => external Recover *BasicBlock // optional; control transfers here after recovered panic AnonFuncs []*Function // anonymous functions directly beneath this one diff --git a/oracle/callees.go b/oracle/callees.go index a900b7b3ef..8584403079 100644 --- a/oracle/callees.go +++ b/oracle/callees.go @@ -38,13 +38,13 @@ func callees(o *Oracle, qpos *QueryPos) (queryResult, error) { // not what the user intended. // Reject type conversions. - if qpos.info.IsType(e.Fun) { + if qpos.info.Types[e.Fun].IsType() { return nil, fmt.Errorf("this is a type conversion, not a function call") } // Reject calls to built-ins. if id, ok := unparen(e.Fun).(*ast.Ident); ok { - if b, ok := qpos.info.ObjectOf(id).(*types.Builtin); ok { + if b, ok := qpos.info.Uses[id].(*types.Builtin); ok { return nil, fmt.Errorf("this is a call to the built-in '%s' operator", b.Name()) } } diff --git a/oracle/describe.go b/oracle/describe.go index 149172cd7e..26a81b9bdc 100644 --- a/oracle/describe.go +++ b/oracle/describe.go @@ -9,6 +9,7 @@ import ( "fmt" "go/ast" "go/token" + "log" "os" "strings" @@ -180,7 +181,8 @@ func findInterestingNode(pkginfo *loader.PackageInfo, path []ast.Node) ([]ast.No return path, actionExpr case *ast.SelectorExpr: - if pkginfo.ObjectOf(n.Sel) == nil { + // TODO(adonovan): use Selections info directly. + if pkginfo.Uses[n.Sel] == nil { // TODO(adonovan): is this reachable? return path, actionUnknown } @@ -267,15 +269,15 @@ func findInterestingNode(pkginfo *loader.PackageInfo, path []ast.Node) ([]ast.No return path[1:], actionPackage default: - // e.g. blank identifier (go/types bug?) - // or y in "switch y := x.(type)" (go/types bug?) + // e.g. blank identifier + // or y in "switch y := x.(type)" // or code in a _test.go file that's not part of the package. - fmt.Printf("unknown reference %s in %T\n", n, path[1]) + log.Printf("unknown reference %s in %T\n", n, path[1]) return path, actionUnknown } case *ast.StarExpr: - if pkginfo.IsType(n) { + if pkginfo.Types[n].IsType() { return path, actionType } return path, actionExpr @@ -311,7 +313,7 @@ func describeValue(o *Oracle, qpos *QueryPos, path []ast.Node) (*describeValueRe } typ := qpos.info.TypeOf(expr) - constVal := qpos.info.ValueOf(expr) + constVal := qpos.info.Types[expr].Value return &describeValueResult{ qpos: qpos, @@ -497,7 +499,12 @@ func describePackage(o *Oracle, qpos *QueryPos, path []ast.Node) (*describePacka var pkg *types.Package switch n := path[0].(type) { case *ast.ImportSpec: - pkgname := qpos.info.ImportSpecPkg(n) + var pkgname *types.PkgName + if n.Name != nil { + pkgname = qpos.info.Defs[n.Name].(*types.PkgName) + } else if p := qpos.info.Implicits[n]; p != nil { + pkgname = p.(*types.PkgName) + } description = fmt.Sprintf("import of package %q", pkgname.Pkg().Path()) pkg = pkgname.Pkg() @@ -507,7 +514,7 @@ func describePackage(o *Oracle, qpos *QueryPos, path []ast.Node) (*describePacka pkg = qpos.info.Pkg description = fmt.Sprintf("definition of package %q", pkg.Path()) } else { - // e.g. import id + // e.g. import id "..." // or id.F() pkg = qpos.info.ObjectOf(n).Pkg() description = fmt.Sprintf("reference to package %q", pkg.Path()) @@ -662,7 +669,7 @@ func describeStmt(o *Oracle, qpos *QueryPos, path []ast.Node) (*describeStmtResu var description string switch n := path[0].(type) { case *ast.Ident: - if qpos.info.ObjectOf(n).Pos() == n.Pos() { + if qpos.info.Defs[n] != nil { description = "labelled statement" } else { description = "reference to labelled statement" diff --git a/oracle/freevars.go b/oracle/freevars.go index a142d15afd..5582318f47 100644 --- a/oracle/freevars.go +++ b/oracle/freevars.go @@ -51,17 +51,13 @@ func freevars(o *Oracle, qpos *QueryPos) (queryResult, error) { } id = func(n *ast.Ident) types.Object { - obj := qpos.info.ObjectOf(n) + obj := qpos.info.Uses[n] if obj == nil { - return nil // TODO(adonovan): fix: this fails for *types.Label. - panic("no types.Object for ast.Ident") + return nil // not a reference } if _, ok := obj.(*types.PkgName); ok { return nil // imported package } - if n.Pos() == obj.Pos() { - return nil // this ident is the definition, not a reference - } if !(file.Pos() <= obj.Pos() && obj.Pos() <= file.End()) { return nil // not defined in this file } diff --git a/refactor/eg/eg.go b/refactor/eg/eg.go index 4bfcaa1f70..c754917f39 100644 --- a/refactor/eg/eg.go +++ b/refactor/eg/eg.go @@ -113,7 +113,7 @@ Dot imports are forbidden in the template. // TODO(adonovan): expand upon the above documentation as an HTML page. // TODO(adonovan): eliminate dependency on loader.PackageInfo. -// Move its ObjectOf/IsType/TypeOf methods into go/types. +// Move its TypeOf method into go/types. // A Transformer represents a single example-based transformation. type Transformer struct { diff --git a/refactor/eg/match.go b/refactor/eg/match.go index 0f328a9041..79c529e017 100644 --- a/refactor/eg/match.go +++ b/refactor/eg/match.go @@ -105,7 +105,7 @@ func (tr *Transformer) matchExpr(x, y ast.Expr) bool { case *ast.CallExpr: y := y.(*ast.CallExpr) match := tr.matchExpr // function call - if tr.info.IsType(x.Fun) { + if tr.info.Types[x.Fun].IsType() { match = tr.matchType // type conversion } return x.Ellipsis.IsValid() == y.Ellipsis.IsValid() &&