From 28aef64757f4d432485ab970b094e1af8b301e84 Mon Sep 17 00:00:00 2001 From: Andrew Gerrand Date: Thu, 17 May 2018 10:14:04 +1000 Subject: [PATCH] go/internal/gcimporter: return error from BExportData This change adds an error return value to BExportData and replaces the various calls to log.Fatal within that library with panics that propagate the internal error up the call stack to BExportData which recovers and returns the error. Fixes golang/go#25431 Change-Id: Id628c63a04145bb469bd6fbc7fbab1b0fa291b2c Reviewed-on: https://go-review.googlesource.com/113457 Reviewed-by: David Symonds Reviewed-by: Rob Pike Reviewed-by: Robert Griesemer Run-TryBot: Robert Griesemer TryBot-Result: Gobot Gobot --- go/gcexportdata/gcexportdata.go | 6 ++- go/internal/gcimporter/bexport.go | 49 ++++++++++++++++-------- go/internal/gcimporter/bexport19_test.go | 5 ++- go/internal/gcimporter/bexport_test.go | 10 ++++- 4 files changed, 51 insertions(+), 19 deletions(-) diff --git a/go/gcexportdata/gcexportdata.go b/go/gcexportdata/gcexportdata.go index b44bdbfc..997d3b29 100644 --- a/go/gcexportdata/gcexportdata.go +++ b/go/gcexportdata/gcexportdata.go @@ -92,6 +92,10 @@ func Read(in io.Reader, fset *token.FileSet, imports map[string]*types.Package, // Write writes encoded type information for the specified package to out. // The FileSet provides file position information for named objects. func Write(out io.Writer, fset *token.FileSet, pkg *types.Package) error { - _, err := out.Write(gcimporter.BExportData(fset, pkg)) + b, err := gcimporter.BExportData(fset, pkg) + if err != nil { + return err + } + _, err = out.Write(b) return err } diff --git a/go/internal/gcimporter/bexport.go b/go/internal/gcimporter/bexport.go index cbf8bc00..b1061725 100644 --- a/go/internal/gcimporter/bexport.go +++ b/go/internal/gcimporter/bexport.go @@ -16,7 +16,6 @@ import ( "go/constant" "go/token" "go/types" - "log" "math" "math/big" "sort" @@ -76,9 +75,29 @@ type exporter struct { indent int // for trace } +// internalError represents an error generated inside this package. +type internalError string + +func (e internalError) Error() string { return "gcimporter: " + string(e) } + +func internalErrorf(format string, args ...interface{}) error { + return internalError(fmt.Sprintf(format, args...)) +} + // BExportData returns binary export data for pkg. // If no file set is provided, position info will be missing. -func BExportData(fset *token.FileSet, pkg *types.Package) []byte { +func BExportData(fset *token.FileSet, pkg *types.Package) (b []byte, err error) { + defer func() { + if e := recover(); e != nil { + if ierr, ok := e.(internalError); ok { + err = ierr + return + } + // Not an internal error; panic again. + panic(e) + } + }() + p := exporter{ fset: fset, strIndex: map[string]int{"": 0}, // empty string is mapped to 0 @@ -107,7 +126,7 @@ func BExportData(fset *token.FileSet, pkg *types.Package) []byte { p.typIndex[typ] = index } if len(p.typIndex) != len(predeclared) { - log.Fatalf("gcimporter: duplicate entries in type map?") + return nil, internalError("duplicate entries in type map?") } // write package data @@ -145,12 +164,12 @@ func BExportData(fset *token.FileSet, pkg *types.Package) []byte { // --- end of export data --- - return p.out.Bytes() + return p.out.Bytes(), nil } func (p *exporter) pkg(pkg *types.Package, emptypath bool) { if pkg == nil { - log.Fatalf("gcimporter: unexpected nil pkg") + panic(internalError("unexpected nil pkg")) } // if we saw the package before, write its index (>= 0) @@ -209,7 +228,7 @@ func (p *exporter) obj(obj types.Object) { p.paramList(sig.Results(), false) default: - log.Fatalf("gcimporter: unexpected object %v (%T)", obj, obj) + panic(internalErrorf("unexpected object %v (%T)", obj, obj)) } } @@ -273,7 +292,7 @@ func (p *exporter) qualifiedName(obj types.Object) { func (p *exporter) typ(t types.Type) { if t == nil { - log.Fatalf("gcimporter: nil type") + panic(internalError("nil type")) } // Possible optimization: Anonymous pointer types *T where @@ -356,7 +375,7 @@ func (p *exporter) typ(t types.Type) { p.typ(t.Elem()) default: - log.Fatalf("gcimporter: unexpected type %T: %s", t, t) + panic(internalErrorf("unexpected type %T: %s", t, t)) } } @@ -422,7 +441,7 @@ func (p *exporter) fieldList(t *types.Struct) { func (p *exporter) field(f *types.Var) { if !f.IsField() { - log.Fatalf("gcimporter: field expected") + panic(internalError("field expected")) } p.pos(f) @@ -452,7 +471,7 @@ func (p *exporter) iface(t *types.Interface) { func (p *exporter) method(m *types.Func) { sig := m.Type().(*types.Signature) if sig.Recv() == nil { - log.Fatalf("gcimporter: method expected") + panic(internalError("method expected")) } p.pos(m) @@ -575,13 +594,13 @@ func (p *exporter) value(x constant.Value) { p.tag(unknownTag) default: - log.Fatalf("gcimporter: unexpected value %v (%T)", x, x) + panic(internalErrorf("unexpected value %v (%T)", x, x)) } } func (p *exporter) float(x constant.Value) { if x.Kind() != constant.Float { - log.Fatalf("gcimporter: unexpected constant %v, want float", x) + panic(internalErrorf("unexpected constant %v, want float", x)) } // extract sign (there is no -0) sign := constant.Sign(x) @@ -616,7 +635,7 @@ func (p *exporter) float(x constant.Value) { m.SetMantExp(&m, int(m.MinPrec())) mant, acc := m.Int(nil) if acc != big.Exact { - log.Fatalf("gcimporter: internal error") + panic(internalError("internal error")) } p.int(sign) @@ -653,7 +672,7 @@ func (p *exporter) bool(b bool) bool { func (p *exporter) index(marker byte, index int) { if index < 0 { - log.Fatalf("gcimporter: invalid index < 0") + panic(internalError("invalid index < 0")) } if debugFormat { p.marker('t') @@ -666,7 +685,7 @@ func (p *exporter) index(marker byte, index int) { func (p *exporter) tag(tag int) { if tag >= 0 { - log.Fatalf("gcimporter: invalid tag >= 0") + panic(internalError("invalid tag >= 0")) } if debugFormat { p.marker('t') diff --git a/go/internal/gcimporter/bexport19_test.go b/go/internal/gcimporter/bexport19_test.go index 0107f808..5c3cf2dd 100644 --- a/go/internal/gcimporter/bexport19_test.go +++ b/go/internal/gcimporter/bexport19_test.go @@ -80,7 +80,10 @@ func TestTypeAliases(t *testing.T) { checkPkg(t, pkg1, "export") // export - exportdata := gcimporter.BExportData(fset1, pkg1) + exportdata, err := gcimporter.BExportData(fset1, pkg1) + if err != nil { + t.Fatal(err) + } // import imports := make(map[string]*types.Package) diff --git a/go/internal/gcimporter/bexport_test.go b/go/internal/gcimporter/bexport_test.go index 4279ad36..dac9ca29 100644 --- a/go/internal/gcimporter/bexport_test.go +++ b/go/internal/gcimporter/bexport_test.go @@ -63,7 +63,10 @@ type UnknownType undefined if info.Files == nil { continue // empty directory } - exportdata := gcimporter.BExportData(conf.Fset, pkg) + exportdata, err := gcimporter.BExportData(conf.Fset, pkg) + if err != nil { + t.Fatal(err) + } imports := make(map[string]*types.Package) fset2 := token.NewFileSet() @@ -306,7 +309,10 @@ func TestVeryLongFile(t *testing.T) { } // export - exportdata := gcimporter.BExportData(fset1, pkg) + exportdata, err := gcimporter.BExportData(fset1, pkg) + if err != nil { + t.Fatal(err) + } // import imports := make(map[string]*types.Package)