From b4b6fe2cb82970f144debbe03ecb71e340b15446 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 17 Jan 2019 11:56:24 -0500 Subject: [PATCH] go/{analysis,packages}: add TypesSizes Many Analyzers need to measure the width of an integer and all today use hacks. This change causes analysis.Pass to retain and expose the type sizing function used during type checking. This in turn requires go/packages to retain and expose the type sizing function in Packages.TypesSizes, which addresses a longstanding need among many of its clients. Change-Id: Ia8362019bcde34c10cb4fbc38cfdfddcbef3eb5c Reviewed-on: https://go-review.googlesource.com/c/158317 Reviewed-by: Michael Matloob Run-TryBot: Michael Matloob --- go/analysis/analysis.go | 1 + go/analysis/internal/checker/checker.go | 1 + go/analysis/passes/asmdecl/asmdecl.go | 3 ++- go/analysis/passes/cgocall/cgocall.go | 8 +++---- go/analysis/passes/printf/types.go | 3 --- go/analysis/passes/shift/shift.go | 31 +------------------------ go/analysis/unitchecker/unitchecker.go | 1 + go/packages/packages.go | 5 ++++ 8 files changed, 14 insertions(+), 39 deletions(-) diff --git a/go/analysis/analysis.go b/go/analysis/analysis.go index 21baa02a8d..4d8a6e5e7d 100644 --- a/go/analysis/analysis.go +++ b/go/analysis/analysis.go @@ -87,6 +87,7 @@ type Pass struct { OtherFiles []string // names of non-Go files of this package Pkg *types.Package // type information about the package TypesInfo *types.Info // type information about the syntax trees + TypesSizes types.Sizes // function for computing sizes of types // Report reports a Diagnostic, a finding about a specific location // in the analyzed source code such as a potential mistake. diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go index e70a5133ac..0d36e0bfd7 100644 --- a/go/analysis/internal/checker/checker.go +++ b/go/analysis/internal/checker/checker.go @@ -492,6 +492,7 @@ func (act *action) execOnce() { OtherFiles: act.pkg.OtherFiles, Pkg: act.pkg.Types, TypesInfo: act.pkg.TypesInfo, + TypesSizes: act.pkg.TypesSizes, ResultOf: inputs, Report: func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) }, ImportObjectFact: act.importObjectFact, diff --git a/go/analysis/passes/asmdecl/asmdecl.go b/go/analysis/passes/asmdecl/asmdecl.go index dce1ef7bd5..6403d7783a 100644 --- a/go/analysis/passes/asmdecl/asmdecl.go +++ b/go/analysis/passes/asmdecl/asmdecl.go @@ -114,7 +114,8 @@ func init() { // library we cannot assume types.SizesFor is consistent with arches. // For now, assume 64-bit norms and print a warning. // But this warning should really be deferred until we attempt to use - // arch, which is very unlikely. + // arch, which is very unlikely. Better would be + // to defer size computation until we have Pass.TypesSizes. arch.sizes = types.SizesFor("gc", "amd64") log.Printf("unknown architecture %s", arch.name) } diff --git a/go/analysis/passes/cgocall/cgocall.go b/go/analysis/passes/cgocall/cgocall.go index 993f1ce3c4..1e4fac8595 100644 --- a/go/analysis/passes/cgocall/cgocall.go +++ b/go/analysis/passes/cgocall/cgocall.go @@ -9,7 +9,6 @@ package cgocall import ( "fmt" "go/ast" - "go/build" "go/format" "go/parser" "go/token" @@ -45,7 +44,7 @@ func run(pass *analysis.Pass) (interface{}, error) { return nil, nil // doesn't use cgo } - cgofiles, info, err := typeCheckCgoSourceFiles(pass.Fset, pass.Pkg, pass.Files, pass.TypesInfo) + cgofiles, info, err := typeCheckCgoSourceFiles(pass.Fset, pass.Pkg, pass.Files, pass.TypesInfo, pass.TypesSizes) if err != nil { return nil, err } @@ -171,7 +170,7 @@ func checkCgo(fset *token.FileSet, f *ast.File, info *types.Info, reportf func(t // limited ourselves here to preserving function bodies and initializer // expressions since that is all that the cgocall analyzer needs. // -func typeCheckCgoSourceFiles(fset *token.FileSet, pkg *types.Package, files []*ast.File, info *types.Info) ([]*ast.File, *types.Info, error) { +func typeCheckCgoSourceFiles(fset *token.FileSet, pkg *types.Package, files []*ast.File, info *types.Info, sizes types.Sizes) ([]*ast.File, *types.Info, error) { const thispkg = "·this·" // Which files are cgo files? @@ -269,8 +268,7 @@ func typeCheckCgoSourceFiles(fset *token.FileSet, pkg *types.Package, files []*a Importer: importerFunc(func(path string) (*types.Package, error) { return importMap[path], nil }), - // TODO(adonovan): Sizes should probably be provided by analysis.Pass. - Sizes: types.SizesFor("gc", build.Default.GOARCH), + Sizes: sizes, Error: func(error) {}, // ignore errors (e.g. unused import) } diff --git a/go/analysis/passes/printf/types.go b/go/analysis/passes/printf/types.go index e8810464cd..2eb6dc7475 100644 --- a/go/analysis/passes/printf/types.go +++ b/go/analysis/passes/printf/types.go @@ -2,7 +2,6 @@ package printf import ( "go/ast" - "go/build" "go/types" "golang.org/x/tools/go/analysis" @@ -235,5 +234,3 @@ func matchStructArgType(pass *analysis.Pass, t printfArgType, typ *types.Struct, } return true } - -var archSizes = types.SizesFor("gc", build.Default.GOARCH) diff --git a/go/analysis/passes/shift/shift.go b/go/analysis/passes/shift/shift.go index 56b150b2b1..39f54573c9 100644 --- a/go/analysis/passes/shift/shift.go +++ b/go/analysis/passes/shift/shift.go @@ -12,10 +12,8 @@ package shift import ( "go/ast" - "go/build" "go/constant" "go/token" - "go/types" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" @@ -93,36 +91,9 @@ func checkLongShift(pass *analysis.Pass, node ast.Node, x, y ast.Expr) { if t == nil { return } - b, ok := t.Underlying().(*types.Basic) - if !ok { - return - } - var size int64 - switch b.Kind() { - case types.Uint8, types.Int8: - size = 8 - case types.Uint16, types.Int16: - size = 16 - case types.Uint32, types.Int32: - size = 32 - case types.Uint64, types.Int64: - size = 64 - case types.Int, types.Uint: - size = uintBitSize - case types.Uintptr: - size = uintptrBitSize - default: - return - } + size := 8 * pass.TypesSizes.Sizeof(t) if amt >= size { ident := analysisutil.Format(pass.Fset, x) pass.Reportf(node.Pos(), "%s (%d bits) too small for shift of %d", ident, size, amt) } } - -var ( - uintBitSize = 8 * archSizes.Sizeof(types.Typ[types.Uint]) - uintptrBitSize = 8 * archSizes.Sizeof(types.Typ[types.Uintptr]) -) - -var archSizes = types.SizesFor("gc", build.Default.GOARCH) diff --git a/go/analysis/unitchecker/unitchecker.go b/go/analysis/unitchecker/unitchecker.go index 76dabc28b9..ba2e66fed2 100644 --- a/go/analysis/unitchecker/unitchecker.go +++ b/go/analysis/unitchecker/unitchecker.go @@ -329,6 +329,7 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re OtherFiles: cfg.NonGoFiles, Pkg: pkg, TypesInfo: info, + TypesSizes: tc.Sizes, ResultOf: inputs, Report: func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) }, ImportObjectFact: facts.ImportObjectFact, diff --git a/go/packages/packages.go b/go/packages/packages.go index e69ddfcb4c..5da437fc18 100644 --- a/go/packages/packages.go +++ b/go/packages/packages.go @@ -250,6 +250,9 @@ type Package struct { // TypesInfo provides type information about the package's syntax trees. // It is set only when Syntax is set. TypesInfo *types.Info + + // TypesSizes provides the effective size function for types in TypesInfo. + TypesSizes types.Sizes } // An Error describes a problem with a package's metadata, syntax, or types. @@ -582,6 +585,7 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) { lpkg.Fset = ld.Fset lpkg.Syntax = []*ast.File{} lpkg.TypesInfo = new(types.Info) + lpkg.TypesSizes = ld.sizes return } @@ -669,6 +673,7 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) { Scopes: make(map[ast.Node]*types.Scope), Selections: make(map[*ast.SelectorExpr]*types.Selection), } + lpkg.TypesSizes = ld.sizes importer := importerFunc(func(path string) (*types.Package, error) { if path == "unsafe" {