Skip to content

Commit b79f76f

Browse files
committed
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 <[email protected]> Trust: Matthew Dempsky <[email protected]> Run-TryBot: Matthew Dempsky <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Rebecca Stambler <[email protected]>
1 parent 7fde01f commit b79f76f

File tree

3 files changed

+56
-11
lines changed

3 files changed

+56
-11
lines changed

go/internal/gcimporter/bexport_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"go/parser"
1313
"go/token"
1414
"go/types"
15+
"path/filepath"
1516
"reflect"
1617
"runtime"
1718
"strings"
@@ -117,7 +118,8 @@ type UnknownType undefined
117118

118119
func fileLine(fset *token.FileSet, obj types.Object) string {
119120
posn := fset.Position(obj.Pos())
120-
return fmt.Sprintf("%s:%d", posn.Filename, posn.Line)
121+
filename := filepath.Clean(strings.ReplaceAll(posn.Filename, "$GOROOT", build.Default.GOROOT))
122+
return fmt.Sprintf("%s:%d", filename, posn.Line)
121123
}
122124

123125
// equalObj reports how x and y differ. They are assumed to belong to

go/internal/gcimporter/iexport.go

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -472,10 +472,10 @@ func (w *exportWriter) param(obj types.Object) {
472472
func (w *exportWriter) value(typ types.Type, v constant.Value) {
473473
w.typ(typ, nil)
474474

475-
switch v.Kind() {
476-
case constant.Bool:
475+
switch b := typ.Underlying().(*types.Basic); b.Info() & types.IsConstType {
476+
case types.IsBoolean:
477477
w.bool(constant.BoolVal(v))
478-
case constant.Int:
478+
case types.IsInteger:
479479
var i big.Int
480480
if i64, exact := constant.Int64Val(v); exact {
481481
i.SetInt64(i64)
@@ -485,25 +485,27 @@ func (w *exportWriter) value(typ types.Type, v constant.Value) {
485485
i.SetString(v.ExactString(), 10)
486486
}
487487
w.mpint(&i, typ)
488-
case constant.Float:
488+
case types.IsFloat:
489489
f := constantToFloat(v)
490490
w.mpfloat(f, typ)
491-
case constant.Complex:
491+
case types.IsComplex:
492492
w.mpfloat(constantToFloat(constant.Real(v)), typ)
493493
w.mpfloat(constantToFloat(constant.Imag(v)), typ)
494-
case constant.String:
494+
case types.IsString:
495495
w.string(constant.StringVal(v))
496-
case constant.Unknown:
497-
// package contains type errors
498496
default:
499-
panic(internalErrorf("unexpected value %v (%T)", v, v))
497+
if b.Kind() == types.Invalid {
498+
// package contains type errors
499+
break
500+
}
501+
panic(internalErrorf("unexpected type %v (%v)", typ, typ.Underlying()))
500502
}
501503
}
502504

503505
// constantToFloat converts a constant.Value with kind constant.Float to a
504506
// big.Float.
505507
func constantToFloat(x constant.Value) *big.Float {
506-
assert(x.Kind() == constant.Float)
508+
x = constant.ToFloat(x)
507509
// Use the same floating-point precision (512) as cmd/compile
508510
// (see Mpprec in cmd/compile/internal/gc/mpfloat.go).
509511
const mpprec = 512

go/internal/gcimporter/iexport_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package gcimporter_test
1010

1111
import (
12+
"bufio"
1213
"bytes"
1314
"fmt"
1415
"go/ast"
@@ -17,7 +18,9 @@ import (
1718
"go/parser"
1819
"go/token"
1920
"go/types"
21+
"io/ioutil"
2022
"math/big"
23+
"os"
2124
"reflect"
2225
"runtime"
2326
"sort"
@@ -29,6 +32,27 @@ import (
2932
"golang.org/x/tools/go/loader"
3033
)
3134

35+
func readExportFile(filename string) ([]byte, error) {
36+
f, err := os.Open(filename)
37+
if err != nil {
38+
return nil, err
39+
}
40+
defer f.Close()
41+
42+
buf := bufio.NewReader(f)
43+
if _, err := gcimporter.FindExportData(buf); err != nil {
44+
return nil, err
45+
}
46+
47+
if ch, err := buf.ReadByte(); err != nil {
48+
return nil, err
49+
} else if ch != 'i' {
50+
return nil, fmt.Errorf("unexpected byte: %v", ch)
51+
}
52+
53+
return ioutil.ReadAll(buf)
54+
}
55+
3256
func iexport(fset *token.FileSet, pkg *types.Package) ([]byte, error) {
3357
var buf bytes.Buffer
3458
if err := gcimporter.IExportData(&buf, fset, pkg); err != nil {
@@ -54,6 +78,9 @@ func TestIExportData_stdlib(t *testing.T) {
5478
conf := loader.Config{
5579
Build: &ctxt,
5680
AllowErrors: true,
81+
TypeChecker: types.Config{
82+
Sizes: types.SizesFor(ctxt.Compiler, ctxt.GOARCH),
83+
},
5784
}
5885
for _, path := range buildutil.AllPackages(conf.Build) {
5986
conf.Import(path)
@@ -96,6 +123,16 @@ type UnknownType undefined
96123
} else {
97124
testPkgData(t, conf.Fset, pkg, exportdata)
98125
}
126+
127+
if pkg.Name() == "main" || pkg.Name() == "haserrors" {
128+
// skip; no export data
129+
} else if bp, err := ctxt.Import(pkg.Path(), "", build.FindOnly); err != nil {
130+
t.Log("warning:", err)
131+
} else if exportdata, err := readExportFile(bp.PkgObj); err != nil {
132+
t.Log("warning:", err)
133+
} else {
134+
testPkgData(t, conf.Fset, pkg, exportdata)
135+
}
99136
}
100137
}
101138

@@ -111,6 +148,10 @@ func testPkgData(t *testing.T, fset *token.FileSet, pkg *types.Package, exportda
111148
}
112149

113150
func testPkg(t *testing.T, fset *token.FileSet, pkg *types.Package, fset2 *token.FileSet, pkg2 *types.Package) {
151+
if _, err := iexport(fset2, pkg2); err != nil {
152+
t.Errorf("reexport %q: %v", pkg.Path(), err)
153+
}
154+
114155
// Compare the packages' corresponding members.
115156
for _, name := range pkg.Scope().Names() {
116157
if !ast.IsExported(name) {

0 commit comments

Comments
 (0)