From b79f76fe3e9733fa292a2a558160b3658f45f187 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Mon, 15 Feb 2021 23:42:21 -0800 Subject: [PATCH] go/internal/gcimporter: fix reexporting compiler data The current versions of the indexed export format encode constant values based on their type. However, IExportData was encoding constants values based on their own kind instead. While cmd/compile internally keeps these matched, go/types does not guarantee the same (see also golang/go#43891). This CL fixes this issue, and also extends testing to check reading in the compiler's export data and that imported packages can be exported again. Change-Id: I08f1845f371edd87773df0ef85bcd4e28f5db2f5 Reviewed-on: https://go-review.googlesource.com/c/tools/+/292351 gopls-CI: kokoro Trust: Matthew Dempsky Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Rebecca Stambler --- go/internal/gcimporter/bexport_test.go | 4 ++- go/internal/gcimporter/iexport.go | 22 +++++++------- go/internal/gcimporter/iexport_test.go | 41 ++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 11 deletions(-) diff --git a/go/internal/gcimporter/bexport_test.go b/go/internal/gcimporter/bexport_test.go index de7b921af0..73585a68ee 100644 --- a/go/internal/gcimporter/bexport_test.go +++ b/go/internal/gcimporter/bexport_test.go @@ -12,6 +12,7 @@ import ( "go/parser" "go/token" "go/types" + "path/filepath" "reflect" "runtime" "strings" @@ -117,7 +118,8 @@ type UnknownType undefined func fileLine(fset *token.FileSet, obj types.Object) string { posn := fset.Position(obj.Pos()) - return fmt.Sprintf("%s:%d", posn.Filename, posn.Line) + filename := filepath.Clean(strings.ReplaceAll(posn.Filename, "$GOROOT", build.Default.GOROOT)) + return fmt.Sprintf("%s:%d", filename, posn.Line) } // equalObj reports how x and y differ. They are assumed to belong to diff --git a/go/internal/gcimporter/iexport.go b/go/internal/gcimporter/iexport.go index 144948c822..c4fae426de 100644 --- a/go/internal/gcimporter/iexport.go +++ b/go/internal/gcimporter/iexport.go @@ -472,10 +472,10 @@ func (w *exportWriter) param(obj types.Object) { func (w *exportWriter) value(typ types.Type, v constant.Value) { w.typ(typ, nil) - switch v.Kind() { - case constant.Bool: + switch b := typ.Underlying().(*types.Basic); b.Info() & types.IsConstType { + case types.IsBoolean: w.bool(constant.BoolVal(v)) - case constant.Int: + case types.IsInteger: var i big.Int if i64, exact := constant.Int64Val(v); exact { i.SetInt64(i64) @@ -485,25 +485,27 @@ func (w *exportWriter) value(typ types.Type, v constant.Value) { i.SetString(v.ExactString(), 10) } w.mpint(&i, typ) - case constant.Float: + case types.IsFloat: f := constantToFloat(v) w.mpfloat(f, typ) - case constant.Complex: + case types.IsComplex: w.mpfloat(constantToFloat(constant.Real(v)), typ) w.mpfloat(constantToFloat(constant.Imag(v)), typ) - case constant.String: + case types.IsString: w.string(constant.StringVal(v)) - case constant.Unknown: - // package contains type errors default: - panic(internalErrorf("unexpected value %v (%T)", v, v)) + if b.Kind() == types.Invalid { + // package contains type errors + break + } + panic(internalErrorf("unexpected type %v (%v)", typ, typ.Underlying())) } } // constantToFloat converts a constant.Value with kind constant.Float to a // big.Float. func constantToFloat(x constant.Value) *big.Float { - assert(x.Kind() == constant.Float) + x = constant.ToFloat(x) // Use the same floating-point precision (512) as cmd/compile // (see Mpprec in cmd/compile/internal/gc/mpfloat.go). const mpprec = 512 diff --git a/go/internal/gcimporter/iexport_test.go b/go/internal/gcimporter/iexport_test.go index 6839844082..8284edeac7 100644 --- a/go/internal/gcimporter/iexport_test.go +++ b/go/internal/gcimporter/iexport_test.go @@ -9,6 +9,7 @@ package gcimporter_test import ( + "bufio" "bytes" "fmt" "go/ast" @@ -17,7 +18,9 @@ import ( "go/parser" "go/token" "go/types" + "io/ioutil" "math/big" + "os" "reflect" "runtime" "sort" @@ -29,6 +32,27 @@ import ( "golang.org/x/tools/go/loader" ) +func readExportFile(filename string) ([]byte, error) { + f, err := os.Open(filename) + if err != nil { + return nil, err + } + defer f.Close() + + buf := bufio.NewReader(f) + if _, err := gcimporter.FindExportData(buf); err != nil { + return nil, err + } + + if ch, err := buf.ReadByte(); err != nil { + return nil, err + } else if ch != 'i' { + return nil, fmt.Errorf("unexpected byte: %v", ch) + } + + return ioutil.ReadAll(buf) +} + func iexport(fset *token.FileSet, pkg *types.Package) ([]byte, error) { var buf bytes.Buffer if err := gcimporter.IExportData(&buf, fset, pkg); err != nil { @@ -54,6 +78,9 @@ func TestIExportData_stdlib(t *testing.T) { conf := loader.Config{ Build: &ctxt, AllowErrors: true, + TypeChecker: types.Config{ + Sizes: types.SizesFor(ctxt.Compiler, ctxt.GOARCH), + }, } for _, path := range buildutil.AllPackages(conf.Build) { conf.Import(path) @@ -96,6 +123,16 @@ type UnknownType undefined } else { testPkgData(t, conf.Fset, pkg, exportdata) } + + if pkg.Name() == "main" || pkg.Name() == "haserrors" { + // skip; no export data + } else if bp, err := ctxt.Import(pkg.Path(), "", build.FindOnly); err != nil { + t.Log("warning:", err) + } else if exportdata, err := readExportFile(bp.PkgObj); err != nil { + t.Log("warning:", err) + } else { + testPkgData(t, conf.Fset, pkg, exportdata) + } } } @@ -111,6 +148,10 @@ func testPkgData(t *testing.T, fset *token.FileSet, pkg *types.Package, exportda } func testPkg(t *testing.T, fset *token.FileSet, pkg *types.Package, fset2 *token.FileSet, pkg2 *types.Package) { + if _, err := iexport(fset2, pkg2); err != nil { + t.Errorf("reexport %q: %v", pkg.Path(), err) + } + // Compare the packages' corresponding members. for _, name := range pkg.Scope().Names() { if !ast.IsExported(name) {