go.tools/go/types: combine Info.{Types,Values} maps.

This results in significant improvement to type-checking time:
it reduces by 4% the entire running time of ssa/stdlib_test
(GOMAXPROCS=8, n=7).

LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/57770043
This commit is contained in:
Alan Donovan 2014-01-28 16:46:24 -05:00
parent e5e20e3af5
commit 64ec206bfd
13 changed files with 60 additions and 65 deletions

View File

@ -8,9 +8,10 @@ package main
import ( import (
"bytes" "bytes"
"code.google.com/p/go.tools/go/types"
"fmt" "fmt"
"go/ast" "go/ast"
"code.google.com/p/go.tools/go/types"
) )
// checkCopyLocks checks whether a function might // checkCopyLocks checks whether a function might
@ -24,7 +25,7 @@ func (f *File) checkCopyLocks(d *ast.FuncDecl) {
if d.Recv != nil && len(d.Recv.List) > 0 { if d.Recv != nil && len(d.Recv.List) > 0 {
expr := d.Recv.List[0].Type expr := d.Recv.List[0].Type
if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr]); path != nil { if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil {
f.Warnf(expr.Pos(), "%s passes Lock by value: %v", d.Name.Name, path) f.Warnf(expr.Pos(), "%s passes Lock by value: %v", d.Name.Name, path)
} }
} }
@ -32,7 +33,7 @@ func (f *File) checkCopyLocks(d *ast.FuncDecl) {
if d.Type.Params != nil { if d.Type.Params != nil {
for _, field := range d.Type.Params.List { for _, field := range d.Type.Params.List {
expr := field.Type expr := field.Type
if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr]); path != nil { if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil {
f.Warnf(expr.Pos(), "%s passes Lock by value: %v", d.Name.Name, path) f.Warnf(expr.Pos(), "%s passes Lock by value: %v", d.Name.Name, path)
} }
} }
@ -41,7 +42,7 @@ func (f *File) checkCopyLocks(d *ast.FuncDecl) {
if d.Type.Results != nil { if d.Type.Results != nil {
for _, field := range d.Type.Results.List { for _, field := range d.Type.Results.List {
expr := field.Type expr := field.Type
if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr]); path != nil { if path := lockPath(f.pkg.typesPkg, f.pkg.types[expr].Type); path != nil {
f.Warnf(expr.Pos(), "%s returns Lock by value: %v", d.Name.Name, path) f.Warnf(expr.Pos(), "%s returns Lock by value: %v", d.Name.Name, path)
} }
} }

View File

@ -21,7 +21,6 @@ import (
"strconv" "strconv"
"strings" "strings"
"code.google.com/p/go.tools/go/exact"
_ "code.google.com/p/go.tools/go/gcimporter" _ "code.google.com/p/go.tools/go/gcimporter"
"code.google.com/p/go.tools/go/types" "code.google.com/p/go.tools/go/types"
) )
@ -203,8 +202,7 @@ func doPackageDir(directory string) {
type Package struct { type Package struct {
path string path string
idents map[*ast.Ident]types.Object idents map[*ast.Ident]types.Object
types map[ast.Expr]types.Type types map[ast.Expr]types.TypeAndValue
values map[ast.Expr]exact.Value
spans map[types.Object]Span spans map[types.Object]Span
files []*File files []*File
typesPkg *types.Package typesPkg *types.Package
@ -459,7 +457,7 @@ func (f *File) prepStringerReceiver(d *ast.FuncDecl) {
func (f *File) isStringer(d *ast.FuncDecl) bool { func (f *File) isStringer(d *ast.FuncDecl) bool {
return d.Recv != nil && d.Name.Name == "String" && return d.Recv != nil && d.Name.Name == "String" &&
len(d.Type.Params.List) == 0 && len(d.Type.Results.List) == 1 && len(d.Type.Params.List) == 0 && len(d.Type.Results.List) == 1 &&
f.pkg.types[d.Type.Results.List[0].Type] == types.Typ[types.String] f.pkg.types[d.Type.Results.List[0].Type].Type == types.Typ[types.String]
} }
// walkGenDecl walks a general declaration. // walkGenDecl walks a general declaration.

View File

@ -59,5 +59,5 @@ func (f *File) checkNilFuncComparison(e *ast.BinaryExpr) {
// isNil reports whether the provided expression is the built-in nil // isNil reports whether the provided expression is the built-in nil
// identifier. // identifier.
func (f *File) isNil(e ast.Expr) bool { func (f *File) isNil(e ast.Expr) bool {
return f.pkg.types[e] == types.Typ[types.UntypedNil] return f.pkg.types[e].Type == types.Typ[types.UntypedNil]
} }

View File

@ -86,7 +86,7 @@ func (f *File) checkPrintf(call *ast.CallExpr, name string, formatIndex int) {
f.Warn(call.Pos(), "too few arguments in call to", name) f.Warn(call.Pos(), "too few arguments in call to", name)
return return
} }
lit := f.pkg.values[call.Args[formatIndex]] lit := f.pkg.types[call.Args[formatIndex]].Value
if lit == nil { if lit == nil {
if *verbose { if *verbose {
f.Warn(call.Pos(), "can't check non-constant format in call to", name) f.Warn(call.Pos(), "can't check non-constant format in call to", name)
@ -380,7 +380,7 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) {
arg := call.Args[argNum] arg := call.Args[argNum]
if !f.matchArgType(v.typ, nil, arg) { if !f.matchArgType(v.typ, nil, arg) {
typeString := "" typeString := ""
if typ := f.pkg.types[arg]; typ != nil { if typ := f.pkg.types[arg].Type; typ != nil {
typeString = typ.String() typeString = typ.String()
} }
f.Badf(call.Pos(), "arg %s for printf verb %%%c of wrong type: %s", f.gofmt(arg), state.verb, typeString) f.Badf(call.Pos(), "arg %s for printf verb %%%c of wrong type: %s", f.gofmt(arg), state.verb, typeString)

View File

@ -10,15 +10,13 @@ import (
"go/ast" "go/ast"
"go/token" "go/token"
"code.google.com/p/go.tools/go/exact"
"code.google.com/p/go.tools/go/types" "code.google.com/p/go.tools/go/types"
) )
func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) error { func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) error {
pkg.idents = make(map[*ast.Ident]types.Object) pkg.idents = make(map[*ast.Ident]types.Object)
pkg.spans = make(map[types.Object]Span) pkg.spans = make(map[types.Object]Span)
pkg.types = make(map[ast.Expr]types.Type) pkg.types = make(map[ast.Expr]types.TypeAndValue)
pkg.values = make(map[ast.Expr]exact.Value)
// By providing a Config with our own error function, it will continue // By providing a Config with our own error function, it will continue
// past the first error. There is no need for that function to do anything. // past the first error. There is no need for that function to do anything.
config := types.Config{ config := types.Config{
@ -26,7 +24,6 @@ func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) error {
} }
info := &types.Info{ info := &types.Info{
Types: pkg.types, Types: pkg.types,
Values: pkg.values,
Objects: pkg.idents, Objects: pkg.idents,
} }
typesPkg, err := config.Check(pkg.path, fs, astFiles, info) typesPkg, err := config.Check(pkg.path, fs, astFiles, info)
@ -42,7 +39,7 @@ func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) error {
// If it is not (probably a struct), it returns a printable form of the type. // If it is not (probably a struct), it returns a printable form of the type.
func (pkg *Package) isStruct(c *ast.CompositeLit) (bool, string) { func (pkg *Package) isStruct(c *ast.CompositeLit) (bool, string) {
// Check that the CompositeLit's type is a slice or array (which needs no field keys), if possible. // Check that the CompositeLit's type is a slice or array (which needs no field keys), if possible.
typ := pkg.types[c] typ := pkg.types[c].Type
// If it's a named type, pull out the underlying type. If it's not, the Underlying // If it's a named type, pull out the underlying type. If it's not, the Underlying
// method returns the type itself. // method returns the type itself.
actual := typ actual := typ
@ -91,7 +88,7 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp
} }
if typ == nil { if typ == nil {
// external call // external call
typ = f.pkg.types[arg] typ = f.pkg.types[arg].Type
if typ == nil { if typ == nil {
return true // probably a type check problem return true // probably a type check problem
} }
@ -250,7 +247,7 @@ func (f *File) matchStructArgType(t printfArgType, typ *types.Struct, arg ast.Ex
// being called has. // being called has.
func (f *File) numArgsInSignature(call *ast.CallExpr) int { func (f *File) numArgsInSignature(call *ast.CallExpr) int {
// Check the type of the function or method declaration // Check the type of the function or method declaration
typ := f.pkg.types[call.Fun] typ := f.pkg.types[call.Fun].Type
if typ == nil { if typ == nil {
return 0 return 0
} }
@ -266,10 +263,10 @@ func (f *File) numArgsInSignature(call *ast.CallExpr) int {
// func Error() string // func Error() string
// where "string" is the universe's string type. We know the method is called "Error". // where "string" is the universe's string type. We know the method is called "Error".
func (f *File) isErrorMethodCall(call *ast.CallExpr) bool { func (f *File) isErrorMethodCall(call *ast.CallExpr) bool {
typ := f.pkg.types[call] typ := f.pkg.types[call].Type
if typ != nil { if typ != nil {
// We know it's called "Error", so just check the function signature. // We know it's called "Error", so just check the function signature.
return types.Identical(f.pkg.types[call.Fun], stringerMethodType) return types.Identical(f.pkg.types[call.Fun].Type, stringerMethodType)
} }
// Without types, we can still check by hand. // Without types, we can still check by hand.
// Is it a selector expression? Otherwise it's a function call, not a method call. // Is it a selector expression? Otherwise it's a function call, not a method call.
@ -282,7 +279,7 @@ func (f *File) isErrorMethodCall(call *ast.CallExpr) bool {
return false return false
} }
// Check the type of the method declaration // Check the type of the method declaration
typ = f.pkg.types[sel] typ = f.pkg.types[sel].Type
if typ == nil { if typ == nil {
return false return false
} }

View File

@ -107,7 +107,6 @@ import (
"strings" "strings"
"code.google.com/p/go.tools/astutil" "code.google.com/p/go.tools/astutil"
"code.google.com/p/go.tools/go/exact"
"code.google.com/p/go.tools/go/gcimporter" "code.google.com/p/go.tools/go/gcimporter"
"code.google.com/p/go.tools/go/types" "code.google.com/p/go.tools/go/types"
) )
@ -620,8 +619,7 @@ func (imp *importer) createPackage(path string, files ...*ast.File) *PackageInfo
info := &PackageInfo{ info := &PackageInfo{
Files: files, Files: files,
Info: types.Info{ Info: types.Info{
Types: make(map[ast.Expr]types.Type), Types: make(map[ast.Expr]types.TypeAndValue),
Values: make(map[ast.Expr]exact.Value),
Objects: make(map[*ast.Ident]types.Object), Objects: make(map[*ast.Ident]types.Object),
Implicits: make(map[ast.Node]types.Object), Implicits: make(map[ast.Node]types.Object),
Scopes: make(map[ast.Node]*types.Scope), Scopes: make(map[ast.Node]*types.Scope),

View File

@ -34,7 +34,7 @@ func (info *PackageInfo) String() string {
// //
func (info *PackageInfo) TypeOf(e ast.Expr) types.Type { func (info *PackageInfo) TypeOf(e ast.Expr) types.Type {
if t, ok := info.Types[e]; ok { if t, ok := info.Types[e]; ok {
return t return t.Type
} }
// Defining ast.Idents (id := expr) get only Ident callbacks // Defining ast.Idents (id := expr) get only Ident callbacks
// but not Expr callbacks. // but not Expr callbacks.
@ -49,7 +49,7 @@ func (info *PackageInfo) TypeOf(e ast.Expr) types.Type {
// Precondition: e belongs to the package's ASTs. // Precondition: e belongs to the package's ASTs.
// //
func (info *PackageInfo) ValueOf(e ast.Expr) exact.Value { func (info *PackageInfo) ValueOf(e ast.Expr) exact.Value {
return info.Values[e] return info.Types[e].Value
} }
// ObjectOf returns the typechecker object denoted by the specified id. // ObjectOf returns the typechecker object denoted by the specified id.

View File

@ -14,11 +14,11 @@
// //
// Constant folding computes the exact constant value (exact.Value) for // Constant folding computes the exact constant value (exact.Value) for
// every expression (ast.Expr) that is a compile-time constant. // every expression (ast.Expr) that is a compile-time constant.
// Use Info.Values for the results of constant folding. // Use Info.Types[expr].Value for the results of constant folding.
// //
// Type inference computes the type (Type) of every expression (ast.Expr) // Type inference computes the type (Type) of every expression (ast.Expr)
// and checks for compliance with the language specification. // and checks for compliance with the language specification.
// Use Info.Types for the results of type evaluation. // Use Info.Types[expr].Type for the results of type inference.
// //
package types package types
@ -115,22 +115,26 @@ type Config struct {
// in a client of go/types will initialize DefaultImport to gcimporter.Import. // in a client of go/types will initialize DefaultImport to gcimporter.Import.
var DefaultImport Importer var DefaultImport Importer
type TypeAndValue struct {
Type Type
Value exact.Value
}
// Info holds result type information for a type-checked package. // Info holds result type information for a type-checked package.
// Only the information for which a map is provided is collected. // Only the information for which a map is provided is collected.
// If the package has type errors, the collected information may // If the package has type errors, the collected information may
// be incomplete. // be incomplete.
type Info struct { type Info struct {
// Types maps expressions to their types. Identifiers on the // Types maps expressions to their types, and for constant
// lhs of declarations are collected in Objects, not Types. // expressions, their values.
// Identifiers on the lhs of declarations are collected in
// Objects, not Types.
// //
// For an expression denoting a predeclared built-in function // For an expression denoting a predeclared built-in function
// the recorded signature is call-site specific. If the call // the recorded signature is call-site specific. If the call
// result is not a constant, the recorded type is an argument- // result is not a constant, the recorded type is an argument-
// specific signature. Otherwise, the recorded type is invalid. // specific signature. Otherwise, the recorded type is invalid.
Types map[ast.Expr]Type Types map[ast.Expr]TypeAndValue
// Values maps constant expressions to their values.
Values map[ast.Expr]exact.Value
// Objects maps identifiers to their corresponding objects (including // Objects maps identifiers to their corresponding objects (including
// package names, dots "." of dot-imports, and blank "_" identifiers). // package names, dots "." of dot-imports, and blank "_" identifiers).

View File

@ -13,7 +13,6 @@ import (
"strings" "strings"
"testing" "testing"
"code.google.com/p/go.tools/go/exact"
_ "code.google.com/p/go.tools/go/gcimporter" _ "code.google.com/p/go.tools/go/gcimporter"
. "code.google.com/p/go.tools/go/types" . "code.google.com/p/go.tools/go/types"
) )
@ -94,14 +93,13 @@ func TestValuesInfo(t *testing.T) {
for _, test := range tests { for _, test := range tests {
info := Info{ info := Info{
Types: make(map[ast.Expr]Type), Types: make(map[ast.Expr]TypeAndValue),
Values: make(map[ast.Expr]exact.Value),
} }
name := mustTypecheck(t, "ValuesInfo", test.src, &info) name := mustTypecheck(t, "ValuesInfo", test.src, &info)
// look for constant expression // look for constant expression
var expr ast.Expr var expr ast.Expr
for e := range info.Values { for e := range info.Types {
if ExprString(e) == test.expr { if ExprString(e) == test.expr {
expr = e expr = e
break break
@ -111,15 +109,16 @@ func TestValuesInfo(t *testing.T) {
t.Errorf("package %s: no expression found for %s", name, test.expr) t.Errorf("package %s: no expression found for %s", name, test.expr)
continue continue
} }
tv := info.Types[expr]
// check that type is correct // check that type is correct
if got := info.Types[expr].String(); got != test.typ { if got := tv.Type.String(); got != test.typ {
t.Errorf("package %s: got type %s; want %s", name, got, test.typ) t.Errorf("package %s: got type %s; want %s", name, got, test.typ)
continue continue
} }
// check that value is correct // check that value is correct
if got := info.Values[expr].String(); got != test.val { if got := tv.Value.String(); got != test.val {
t.Errorf("package %s: got value %s; want %s", name, got, test.val) t.Errorf("package %s: got value %s; want %s", name, got, test.val)
} }
} }
@ -206,14 +205,14 @@ func TestTypesInfo(t *testing.T) {
} }
for _, test := range tests { for _, test := range tests {
info := Info{Types: make(map[ast.Expr]Type)} info := Info{Types: make(map[ast.Expr]TypeAndValue)}
name := mustTypecheck(t, "TypesInfo", test.src, &info) name := mustTypecheck(t, "TypesInfo", test.src, &info)
// look for expression type // look for expression type
var typ Type var typ Type
for e, t := range info.Types { for e, tv := range info.Types {
if ExprString(e) == test.expr { if ExprString(e) == test.expr {
typ = t typ = tv.Type
break break
} }
} }

View File

@ -134,7 +134,7 @@ func testBuiltinSignature(t *testing.T, name, src0, want string) {
var conf Config var conf Config
objects := make(map[*ast.Ident]Object) objects := make(map[*ast.Ident]Object)
types := make(map[ast.Expr]Type) types := make(map[ast.Expr]TypeAndValue)
_, err = conf.Check(f.Name.Name, fset, []*ast.File{f}, &Info{Objects: objects, Types: types}) _, err = conf.Check(f.Name.Name, fset, []*ast.File{f}, &Info{Objects: objects, Types: types})
if err != nil { if err != nil {
t.Errorf("%s: %s", src0, err) t.Errorf("%s: %s", src0, err)
@ -144,7 +144,7 @@ func testBuiltinSignature(t *testing.T, name, src0, want string) {
// find called function // find called function
n := 0 n := 0
var fun ast.Expr var fun ast.Expr
for x, _ := range types { for x := range types {
if call, _ := x.(*ast.CallExpr); call != nil { if call, _ := x.(*ast.CallExpr); call != nil {
fun = call.Fun fun = call.Fun
n++ n++
@ -158,7 +158,7 @@ func testBuiltinSignature(t *testing.T, name, src0, want string) {
// check recorded types for fun and descendents (may be parenthesized) // check recorded types for fun and descendents (may be parenthesized)
for { for {
// the recorded type for the built-in must match the wanted signature // the recorded type for the built-in must match the wanted signature
typ := types[fun] typ := types[fun].Type
if typ == nil { if typ == nil {
t.Errorf("%s: no type recorded for %s", src0, ExprString(fun)) t.Errorf("%s: no type recorded for %s", src0, ExprString(fun))
return return

View File

@ -92,14 +92,11 @@ func (check *checker) delay(f func()) {
func (check *checker) recordTypeAndValue(x ast.Expr, typ Type, val exact.Value) { func (check *checker) recordTypeAndValue(x ast.Expr, typ Type, val exact.Value) {
assert(x != nil && typ != nil) assert(x != nil && typ != nil)
if m := check.Types; m != nil {
m[x] = typ
}
if val != nil { if val != nil {
assert(isConstType(typ)) assert(isConstType(typ))
if m := check.Values; m != nil { }
m[x] = val if m := check.Types; m != nil {
} m[x] = TypeAndValue{typ, val}
} }
} }
@ -129,12 +126,14 @@ func (check *checker) recordCommaOkTypes(x ast.Expr, a [2]Type) {
assert(isTyped(a[0]) && isTyped(a[1]) && isBoolean(a[1])) assert(isTyped(a[0]) && isTyped(a[1]) && isBoolean(a[1]))
if m := check.Types; m != nil { if m := check.Types; m != nil {
for { for {
assert(m[x] != nil) // should have been recorded already tv := m[x]
assert(tv.Type != nil) // should have been recorded already
pos := x.Pos() pos := x.Pos()
m[x] = NewTuple( tv.Type = NewTuple(
NewVar(pos, check.pkg, "", a[0]), NewVar(pos, check.pkg, "", a[0]),
NewVar(pos, check.pkg, "", a[1]), NewVar(pos, check.pkg, "", a[1]),
) )
m[x] = tv
// if x is a parenthesized expression (p.X), update p.X // if x is a parenthesized expression (p.X), update p.X
p, _ := x.(*ast.ParenExpr) p, _ := x.(*ast.ParenExpr)
if p == nil { if p == nil {
@ -248,7 +247,7 @@ func (conf *Config) check(pkgPath string, fset *token.FileSet, files []*ast.File
// TODO(gri) Consider doing this before and // TODO(gri) Consider doing this before and
// after function body checking for smaller // after function body checking for smaller
// map size and more immediate feedback. // map size and more immediate feedback.
if check.Types != nil || check.Values != nil { if check.Types != nil {
for x, info := range check.untyped { for x, info := range check.untyped {
check.recordTypeAndValue(x, info.typ, info.val) check.recordTypeAndValue(x, info.typ, info.val)
} }

View File

@ -53,8 +53,7 @@ constant lhs must be representable as an integer.
When an expression gets its final type, either on the way out from rawExpr, When an expression gets its final type, either on the way out from rawExpr,
on the way down in updateExprType, or at the end of the type checker run, on the way down in updateExprType, or at the end of the type checker run,
the type (and constant value, if any) is recorded via Info.Types and Values, the type (and constant value, if any) is recorded via Info.Types, if present.
if present.
*/ */
type opPredicates map[token.Token]func(Type) bool type opPredicates map[token.Token]func(Type) bool

View File

@ -48,13 +48,13 @@ var (
} }
var conf Config var conf Config
types := make(map[ast.Expr]Type) types := make(map[ast.Expr]TypeAndValue)
_, err = conf.Check(f.Name.Name, fset, []*ast.File{f}, &Info{Types: types}) _, err = conf.Check(f.Name.Name, fset, []*ast.File{f}, &Info{Types: types})
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
for x, typ := range types { for x, tv := range types {
var want Type var want Type
switch x := x.(type) { switch x := x.(type) {
case *ast.BasicLit: case *ast.BasicLit:
@ -75,8 +75,8 @@ var (
want = Typ[UntypedNil] want = Typ[UntypedNil]
} }
} }
if want != nil && !Identical(typ, want) { if want != nil && !Identical(tv.Type, want) {
t.Errorf("got %s; want %s", typ, want) t.Errorf("got %s; want %s", tv.Type, want)
} }
} }
} }
@ -96,7 +96,7 @@ func f() int {
} }
var conf Config var conf Config
types := make(map[ast.Expr]Type) types := make(map[ast.Expr]TypeAndValue)
_, err = conf.Check(f.Name.Name, fset, []*ast.File{f}, &Info{Types: types}) _, err = conf.Check(f.Name.Name, fset, []*ast.File{f}, &Info{Types: types})
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
@ -104,10 +104,10 @@ func f() int {
want := Typ[Int] want := Typ[Int]
n := 0 n := 0
for x, got := range types { for x, tv := range types {
if _, ok := x.(*ast.CallExpr); ok { if _, ok := x.(*ast.CallExpr); ok {
if got != want { if tv.Type != want {
t.Errorf("%s: got %s; want %s", fset.Position(x.Pos()), got, want) t.Errorf("%s: got %s; want %s", fset.Position(x.Pos()), tv.Type, want)
} }
n++ n++
} }