gobs: error cleanup part 1.

Remove err from the encoderState and decoderState types, so we're
not always copying to and from various copies of the error, and then
use panic/recover to eliminate lots of error checking.

another pass might take a crack at the same thing for the compilation phase.

R=rsc
CC=golang-dev
https://golang.org/cl/2660042

src/pkg/gob/Makefile

@@ -11,6 +11,7 @@ GOFILES=\
 	doc.go\
 	encode.go\
 	encoder.go\
+	error.go\
 	type.go\
 
 include ../../Make.pkg

src/pkg/gob/codec_test.go

@@ -37,16 +37,23 @@ var encodeT = []EncodeT{
 	{1 << 63, []byte{0xF8, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
 }
 
+// testError is meant to be used as a deferred function to turn a panic(gobError) into a
+// plain test.Error call.
+func testError(t *testing.T) {
+	if e := recover(); e != nil {
+		t.Error(e.(gobError).Error) // Will re-panic if not one of our errors, such as a runtime error.
+	}
+	return
+}
+
 // Test basic encode/decode routines for unsigned integers
 func TestUintCodec(t *testing.T) {
+	defer testError(t)
 	b := new(bytes.Buffer)
 	encState := newEncoderState(b)
 	for _, tt := range encodeT {
 		b.Reset()
 		encodeUint(encState, tt.x)
-		if encState.err != nil {
-			t.Error("encodeUint:", tt.x, encState.err)
-		}
 		if !bytes.Equal(tt.b, b.Bytes()) {
 			t.Errorf("encodeUint: %#x encode: expected % x got % x", tt.x, tt.b, b.Bytes())
 		}
@@ -55,13 +62,7 @@ func TestUintCodec(t *testing.T) {
 	for u := uint64(0); ; u = (u + 1) * 7 {
 		b.Reset()
 		encodeUint(encState, u)
-		if encState.err != nil {
-			t.Error("encodeUint:", u, encState.err)
-		}
 		v := decodeUint(decState)
-		if decState.err != nil {
-			t.Error("DecodeUint:", u, decState.err)
-		}
 		if u != v {
 			t.Errorf("Encode/Decode: sent %#x received %#x", u, v)
 		}
@@ -72,18 +73,13 @@ func TestUintCodec(t *testing.T) {
 }
 
 func verifyInt(i int64, t *testing.T) {
+	defer testError(t)
 	var b = new(bytes.Buffer)
 	encState := newEncoderState(b)
 	encodeInt(encState, i)
-	if encState.err != nil {
-		t.Error("encodeInt:", i, encState.err)
-	}
 	decState := newDecodeState(&b)
 	decState.buf = make([]byte, 8)
 	j := decodeInt(decState)
-	if decState.err != nil {
-		t.Error("DecodeInt:", i, decState.err)
-	}
 	if i != j {
 		t.Errorf("Encode/Decode: sent %#x received %#x", uint64(i), uint64(j))
 	}
@@ -320,10 +316,8 @@ func TestScalarEncInstructions(t *testing.T) {
 }
 
 func execDec(typ string, instr *decInstr, state *decodeState, t *testing.T, p unsafe.Pointer) {
+	defer testError(t)
 	v := int(decodeUint(state))
-	if state.err != nil {
-		t.Fatalf("decoding %s field: %v", typ, state.err)
-	}
 	if v+state.fieldnum != 6 {
 		t.Fatalf("decoding field number %d, got %d", 6, v+state.fieldnum)
 	}

src/pkg/gob/decoder.go

@@ -25,6 +25,7 @@ type Decoder struct {
 	buf          []byte
 	countBuf     [9]byte // counts may be uint64s (unlikely!), require 9 bytes
 	byteBuffer   *bytes.Buffer
+	err          os.Error
 }
 
 // NewDecoder returns a new decoder that reads from the io.Reader.
@@ -43,13 +44,16 @@ func NewDecoder(r io.Reader) *Decoder {
 func (dec *Decoder) recvType(id typeId) {
 	// Have we already seen this type?  That's an error
 	if dec.wireType[id] != nil {
-		dec.state.err = os.ErrorString("gob: duplicate type received")
+		dec.err = os.ErrorString("gob: duplicate type received")
 		return
 	}
 
 	// Type:
 	wire := new(wireType)
-	dec.state.err = dec.decode(tWireType, reflect.NewValue(wire))
+	dec.err = dec.decode(tWireType, reflect.NewValue(wire))
+	if dec.err != nil {
+		return
+	}
 	// Remember we've seen this type.
 	dec.wireType[id] = wire
 
@@ -66,8 +70,8 @@ func (dec *Decoder) Decode(e interface{}) os.Error {
 	// If e represents a value as opposed to a pointer, the answer won't
 	// get back to the caller.  Make sure it's a pointer.
 	if value.Type().Kind() != reflect.Ptr {
-		dec.state.err = os.ErrorString("gob: attempt to decode into a non-pointer")
-		return dec.state.err
+		dec.err = os.ErrorString("gob: attempt to decode into a non-pointer")
+		return dec.err
 	}
 	return dec.DecodeValue(value)
 }
@@ -77,8 +81,8 @@ func (dec *Decoder) Decode(e interface{}) os.Error {
 func (dec *Decoder) recv() {
 	// Read a count.
 	var nbytes uint64
-	nbytes, dec.state.err = decodeUintReader(dec.r, dec.countBuf[0:])
-	if dec.state.err != nil {
+	nbytes, dec.err = decodeUintReader(dec.r, dec.countBuf[0:])
+	if dec.err != nil {
 		return
 	}
 	// Allocate the buffer.
@@ -88,10 +92,10 @@ func (dec *Decoder) recv() {
 	dec.byteBuffer = bytes.NewBuffer(dec.buf[0:nbytes])
 
 	// Read the data
-	_, dec.state.err = io.ReadFull(dec.r, dec.buf[0:nbytes])
-	if dec.state.err != nil {
-		if dec.state.err == os.EOF {
-			dec.state.err = io.ErrUnexpectedEOF
+	_, dec.err = io.ReadFull(dec.r, dec.buf[0:nbytes])
+	if dec.err != nil {
+		if dec.err == os.EOF {
+			dec.err = io.ErrUnexpectedEOF
 		}
 		return
 	}
@@ -104,7 +108,7 @@ func (dec *Decoder) decodeValueFromBuffer(value reflect.Value, ignore bool) {
 	for dec.state.b.Len() > 0 {
 		// Receive a type id.
 		id := typeId(decodeInt(dec.state))
-		if dec.state.err != nil {
+		if dec.err != nil {
 			break
 		}
 
@@ -112,7 +116,7 @@ func (dec *Decoder) decodeValueFromBuffer(value reflect.Value, ignore bool) {
 		if id < 0 { // 0 is the error state, handled above
 			// If the id is negative, we have a type.
 			dec.recvType(-id)
-			if dec.state.err != nil {
+			if dec.err != nil {
 				break
 			}
 			continue
@@ -126,10 +130,10 @@ func (dec *Decoder) decodeValueFromBuffer(value reflect.Value, ignore bool) {
 		// Make sure the type has been defined already or is a builtin type (for
 		// top-level singleton values).
 		if dec.wireType[id] == nil && builtinIdToType[id] == nil {
-			dec.state.err = errBadType
+			dec.err = errBadType
 			break
 		}
-		dec.state.err = dec.decode(id, value)
+		dec.err = dec.decode(id, value)
 		break
 	}
 }
@@ -143,11 +147,11 @@ func (dec *Decoder) DecodeValue(value reflect.Value) os.Error {
 	dec.mutex.Lock()
 	defer dec.mutex.Unlock()
 
-	dec.state.err = nil
+	dec.err = nil
 	dec.recv()
-	if dec.state.err != nil {
-		return dec.state.err
+	if dec.err != nil {
+		return dec.err
 	}
 	dec.decodeValueFromBuffer(value, false)
-	return dec.state.err
+	return dec.err
 }

src/pkg/gob/encode.go

@@ -21,7 +21,6 @@ const uint64Size = unsafe.Sizeof(uint64(0))
 // 0 terminates the structure.
 type encoderState struct {
 	b        *bytes.Buffer
-	err      os.Error             // error encountered during encoding.
 	sendZero bool                 // encoding an array element or map key/value pair; send zero values
 	fieldnum int                  // the last field number written.
 	buf      [1 + uint64Size]byte // buffer used by the encoder; here to avoid allocation.
@@ -39,11 +38,11 @@ func newEncoderState(b *bytes.Buffer) *encoderState {
 // encodeUint writes an encoded unsigned integer to state.b.  Sets state.err.
 // If state.err is already non-nil, it does nothing.
 func encodeUint(state *encoderState, x uint64) {
-	if state.err != nil {
-		return
-	}
 	if x <= 0x7F {
-		state.err = state.b.WriteByte(uint8(x))
+		err := state.b.WriteByte(uint8(x))
+		if err != nil {
+			error(err)
+		}
 		return
 	}
 	var n, m int
@@ -54,7 +53,10 @@ func encodeUint(state *encoderState, x uint64) {
 		m--
 	}
 	state.buf[m] = uint8(-(n - 1))
-	n, state.err = state.b.Write(state.buf[m : uint64Size+1])
+	n, err := state.b.Write(state.buf[m : uint64Size+1])
+	if err != nil {
+		error(err)
+	}
 }
 
 // encodeInt writes an encoded signed integer to state.w.
@@ -317,7 +319,8 @@ type encEngine struct {
 
 const singletonField = 0
 
-func encodeSingle(engine *encEngine, b *bytes.Buffer, basep uintptr) os.Error {
+func encodeSingle(engine *encEngine, b *bytes.Buffer, basep uintptr) (err os.Error) {
+	defer catchError(&err)
 	state := newEncoderState(b)
 	state.fieldnum = singletonField
 	// There is no surrounding struct to frame the transmission, so we must
@@ -331,10 +334,11 @@ func encodeSingle(engine *encEngine, b *bytes.Buffer, basep uintptr) os.Error {
 		}
 	}
 	instr.op(instr, state, p)
-	return state.err
+	return
 }
 
-func encodeStruct(engine *encEngine, b *bytes.Buffer, basep uintptr) os.Error {
+func encodeStruct(engine *encEngine, b *bytes.Buffer, basep uintptr) (err os.Error) {
+	defer catchError(&err)
 	state := newEncoderState(b)
 	state.fieldnum = -1
 	for i := 0; i < len(engine.instr); i++ {
@@ -346,32 +350,27 @@ func encodeStruct(engine *encEngine, b *bytes.Buffer, basep uintptr) os.Error {
 			}
 		}
 		instr.op(instr, state, p)
-		if state.err != nil {
-			break
-		}
 	}
-	return state.err
+	return nil
 }
 
-func encodeArray(b *bytes.Buffer, p uintptr, op encOp, elemWid uintptr, elemIndir int, length int) os.Error {
+func encodeArray(b *bytes.Buffer, p uintptr, op encOp, elemWid uintptr, elemIndir int, length int) {
 	state := newEncoderState(b)
 	state.fieldnum = -1
 	state.sendZero = true
 	encodeUint(state, uint64(length))
-	for i := 0; i < length && state.err == nil; i++ {
+	for i := 0; i < length; i++ {
 		elemp := p
 		up := unsafe.Pointer(elemp)
 		if elemIndir > 0 {
 			if up = encIndirect(up, elemIndir); up == nil {
-				state.err = os.ErrorString("gob: encodeArray: nil element")
-				break
+				errorf("gob: encodeArray: nil element")
 			}
 			elemp = uintptr(up)
 		}
 		op(nil, state, unsafe.Pointer(elemp))
 		p += uintptr(elemWid)
 	}
-	return state.err
 }
 
 func encodeReflectValue(state *encoderState, v reflect.Value, op encOp, indir int) {
@@ -379,61 +378,54 @@ func encodeReflectValue(state *encoderState, v reflect.Value, op encOp, indir in
 		v = reflect.Indirect(v)
 	}
 	if v == nil {
-		state.err = os.ErrorString("gob: encodeReflectValue: nil element")
-		return
+		errorf("gob: encodeReflectValue: nil element")
 	}
 	op(nil, state, unsafe.Pointer(v.Addr()))
 }
 
-func encodeMap(b *bytes.Buffer, mv *reflect.MapValue, keyOp, elemOp encOp, keyIndir, elemIndir int) os.Error {
+func encodeMap(b *bytes.Buffer, mv *reflect.MapValue, keyOp, elemOp encOp, keyIndir, elemIndir int) {
 	state := newEncoderState(b)
 	state.fieldnum = -1
 	state.sendZero = true
 	keys := mv.Keys()
 	encodeUint(state, uint64(len(keys)))
 	for _, key := range keys {
-		if state.err != nil {
-			break
-		}
 		encodeReflectValue(state, key, keyOp, keyIndir)
 		encodeReflectValue(state, mv.Elem(key), elemOp, elemIndir)
 	}
-	return state.err
 }
 
 // To send an interface, we send a string identifying the concrete type, followed
 // by the type identifier (which might require defining that type right now), followed
 // by the concrete value.  A nil value gets sent as the empty string for the name,
 // followed by no value.
-func (enc *Encoder) encodeInterface(b *bytes.Buffer, iv *reflect.InterfaceValue) os.Error {
+func (enc *Encoder) encodeInterface(b *bytes.Buffer, iv *reflect.InterfaceValue) {
 	state := newEncoderState(b)
 	state.fieldnum = -1
 	state.sendZero = true
 	if iv.IsNil() {
 		encodeUint(state, 0)
-		return state.err
+		return
 	}
 
 	typ := iv.Elem().Type()
 	name, ok := concreteTypeToName[typ]
 	if !ok {
-		state.err = os.ErrorString("gob: type not registered for interface: " + typ.String())
-		return state.err
+		errorf("gob: type not registered for interface: %s", typ)
 	}
 	// Send the name.
 	encodeUint(state, uint64(len(name)))
-	_, state.err = io.WriteString(state.b, name)
-	if state.err != nil {
-		return state.err
+	_, err := io.WriteString(state.b, name)
+	if err != nil {
+		error(err)
 	}
 	// Send (and maybe first define) the type id.
 	enc.sendTypeDescriptor(typ)
-	if state.err != nil {
-		return state.err
-	}
 	// Send the value.
-	state.err = enc.encode(state.b, iv.Elem())
-	return state.err
+	err = enc.encode(state.b, iv.Elem())
+	if err != nil {
+		error(err)
+	}
 }
 
 var encOpMap = []encOp{
@@ -486,7 +478,7 @@ func (enc *Encoder) encOpFor(rt reflect.Type) (encOp, int, os.Error) {
 					return
 				}
 				state.update(i)
-				state.err = encodeArray(state.b, slice.Data, elemOp, t.Elem().Size(), indir, int(slice.Len))
+				encodeArray(state.b, slice.Data, elemOp, t.Elem().Size(), indir, int(slice.Len))
 			}
 		case *reflect.ArrayType:
 			// True arrays have size in the type.
@@ -496,7 +488,7 @@ func (enc *Encoder) encOpFor(rt reflect.Type) (encOp, int, os.Error) {
 			}
 			op = func(i *encInstr, state *encoderState, p unsafe.Pointer) {
 				state.update(i)
-				state.err = encodeArray(state.b, uintptr(p), elemOp, t.Elem().Size(), indir, t.Len())
+				encodeArray(state.b, uintptr(p), elemOp, t.Elem().Size(), indir, t.Len())
 			}
 		case *reflect.MapType:
 			keyOp, keyIndir, err := enc.encOpFor(t.Key())
@@ -517,7 +509,7 @@ func (enc *Encoder) encOpFor(rt reflect.Type) (encOp, int, os.Error) {
 					return
 				}
 				state.update(i)
-				state.err = encodeMap(state.b, mv, keyOp, elemOp, keyIndir, elemIndir)
+				encodeMap(state.b, mv, keyOp, elemOp, keyIndir, elemIndir)
 			}
 		case *reflect.StructType:
 			// Generate a closure that calls out to the engine for the nested type.
@@ -529,7 +521,7 @@ func (enc *Encoder) encOpFor(rt reflect.Type) (encOp, int, os.Error) {
 			op = func(i *encInstr, state *encoderState, p unsafe.Pointer) {
 				state.update(i)
 				// indirect through info to delay evaluation for recursive structs
-				state.err = encodeStruct(info.encoder, state.b, uintptr(p))
+				encodeStruct(info.encoder, state.b, uintptr(p))
 			}
 		case *reflect.InterfaceType:
 			op = func(i *encInstr, state *encoderState, p unsafe.Pointer) {
@@ -541,7 +533,7 @@ func (enc *Encoder) encOpFor(rt reflect.Type) (encOp, int, os.Error) {
 					return
 				}
 				state.update(i)
-				state.err = enc.encodeInterface(state.b, iv)
+				enc.encodeInterface(state.b, iv)
 			}
 		}
 	}

src/pkg/gob/encoder.go

@@ -21,6 +21,7 @@ type Encoder struct {
 	state      *encoderState           // so we can encode integers, strings directly
 	countState *encoderState           // stage for writing counts
 	buf        []byte                  // for collecting the output.
+	err        os.Error
 }
 
 // NewEncoder returns a new encoder that will transmit on the io.Writer.
@@ -38,8 +39,8 @@ func (enc *Encoder) badType(rt reflect.Type) {
 }
 
 func (enc *Encoder) setError(err os.Error) {
-	if enc.state.err == nil { // remember the first.
-		enc.state.err = err
+	if enc.err == nil { // remember the first.
+		enc.err = err
 	}
 	enc.state.b.Reset()
 }
@@ -115,7 +116,7 @@ func (enc *Encoder) sendType(origt reflect.Type) (sent bool) {
 	// Type:
 	enc.encode(enc.state.b, reflect.NewValue(info.wire))
 	enc.send()
-	if enc.state.err != nil {
+	if enc.err != nil {
 		return
 	}
 
@@ -150,7 +151,7 @@ func (enc *Encoder) sendTypeDescriptor(rt reflect.Type) {
 	if _, alreadySent := enc.sent[rt]; !alreadySent {
 		// No, so send it.
 		sent := enc.sendType(rt)
-		if enc.state.err != nil {
+		if enc.err != nil {
 			return
 		}
 		// If the type info has still not been transmitted, it means we have
@@ -180,18 +181,18 @@ func (enc *Encoder) EncodeValue(value reflect.Value) os.Error {
 	enc.mutex.Lock()
 	defer enc.mutex.Unlock()
 
-	enc.state.err = nil
+	enc.err = nil
 	rt, _ := indirect(value.Type())
 
 	// Sanity check only: encoder should never come in with data present.
 	if enc.state.b.Len() > 0 || enc.countState.b.Len() > 0 {
-		enc.state.err = os.ErrorString("encoder: buffer not empty")
-		return enc.state.err
+		enc.err = os.ErrorString("encoder: buffer not empty")
+		return enc.err
 	}
 
 	enc.sendTypeDescriptor(rt)
-	if enc.state.err != nil {
-		return enc.state.err
+	if enc.err != nil {
+		return enc.err
 	}
 
 	// Encode the object.
@@ -202,5 +203,5 @@ func (enc *Encoder) EncodeValue(value reflect.Value) os.Error {
 		enc.send()
 	}
 
-	return enc.state.err
+	return enc.err
 }

src/pkg/gob/encoder_test.go

@@ -43,15 +43,15 @@ func TestEncoderDecoder(t *testing.T) {
 	et1 := new(ET1)
 	et1.a = 7
 	et1.et2 = new(ET2)
-	enc.Encode(et1)
-	if enc.state.err != nil {
-		t.Error("encoder fail:", enc.state.err)
+	err := enc.Encode(et1)
+	if err != nil {
+		t.Error("encoder fail:", err)
 	}
 	dec := NewDecoder(b)
 	newEt1 := new(ET1)
-	dec.Decode(newEt1)
-	if dec.state.err != nil {
-		t.Fatal("error decoding ET1:", dec.state.err)
+	err = dec.Decode(newEt1)
+	if err != nil {
+		t.Fatal("error decoding ET1:", err)
 	}
 
 	if !reflect.DeepEqual(et1, newEt1) {
@@ -63,9 +63,9 @@ func TestEncoderDecoder(t *testing.T) {
 
 	enc.Encode(et1)
 	newEt1 = new(ET1)
-	dec.Decode(newEt1)
-	if dec.state.err != nil {
-		t.Fatal("round 2: error decoding ET1:", dec.state.err)
+	err = dec.Decode(newEt1)
+	if err != nil {
+		t.Fatal("round 2: error decoding ET1:", err)
 	}
 	if !reflect.DeepEqual(et1, newEt1) {
 		t.Fatalf("round 2: invalid data for et1: expected %+v; got %+v", *et1, *newEt1)
@@ -75,13 +75,13 @@ func TestEncoderDecoder(t *testing.T) {
 	}
 
 	// Now test with a running encoder/decoder pair that we recognize a type mismatch.
-	enc.Encode(et1)
-	if enc.state.err != nil {
-		t.Error("round 3: encoder fail:", enc.state.err)
+	err = enc.Encode(et1)
+	if err != nil {
+		t.Error("round 3: encoder fail:", err)
 	}
 	newEt2 := new(ET2)
-	dec.Decode(newEt2)
-	if dec.state.err == nil {
+	err = dec.Decode(newEt2)
+	if err == nil {
 		t.Fatal("round 3: expected `bad type' error decoding ET2")
 	}
 }
@@ -94,17 +94,17 @@ func badTypeCheck(e interface{}, shouldFail bool, msg string, t *testing.T) {
 	et1 := new(ET1)
 	et1.a = 7
 	et1.et2 = new(ET2)
-	enc.Encode(et1)
-	if enc.state.err != nil {
-		t.Error("encoder fail:", enc.state.err)
+	err := enc.Encode(et1)
+	if err != nil {
+		t.Error("encoder fail:", err)
 	}
 	dec := NewDecoder(b)
-	dec.Decode(e)
-	if shouldFail && (dec.state.err == nil) {
+	err = dec.Decode(e)
+	if shouldFail && err == nil {
 		t.Error("expected error for", msg)
 	}
-	if !shouldFail && (dec.state.err != nil) {
-		t.Error("unexpected error for", msg, dec.state.err)
+	if !shouldFail && err != nil {
+		t.Error("unexpected error for", msg, err)
 	}
 }
 
@@ -118,9 +118,9 @@ func TestWrongTypeDecoder(t *testing.T) {
 func corruptDataCheck(s string, err os.Error, t *testing.T) {
 	b := bytes.NewBufferString(s)
 	dec := NewDecoder(b)
-	dec.Decode(new(ET2))
-	if dec.state.err != err {
-		t.Error("expected error", err, "got", dec.state.err)
+	err1 := dec.Decode(new(ET2))
+	if err1 != err {
+		t.Error("expected error", err, "got", err1)
 	}
 }
 
@@ -151,14 +151,14 @@ func TestUnsupported(t *testing.T) {
 func encAndDec(in, out interface{}) os.Error {
 	b := new(bytes.Buffer)
 	enc := NewEncoder(b)
-	enc.Encode(in)
-	if enc.state.err != nil {
-		return enc.state.err
+	err := enc.Encode(in)
+	if err != nil {
+		return err
 	}
 	dec := NewDecoder(b)
-	dec.Decode(out)
-	if dec.state.err != nil {
-		return dec.state.err
+	err = dec.Decode(out)
+	if err != nil {
+		return err
 	}
 	return nil
 }

src/pkg/gob/error.go

+// Copyright 2009 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package gob
+
+import (
+	"fmt"
+	"os"
+)
+
+// Errors in decoding and encoding are handled using panic and recover.
+// Panics caused by user error (that is, everything except run-time panics
+// such as "index out of bounds" errors) do not leave the file that caused
+// them, but are instead turned into plain os.Error returns.  Encoding and
+// decoding functions and methods that do not return an os.Error either use
+// panic to report an error or are guaranteed error-free.
+
+// A gobError wraps an os.Error and is used to distinguish errors (panics) generated in this package.
+type gobError struct {
+	os.Error
+}
+
+// errorf is like error but takes Printf-style arguments to construct an os.Error.
+func errorf(format string, args ...interface{}) {
+	error(fmt.Errorf(format, args...))
+}
+
+// error wraps the argument error and uses it as the argument to panic.
+func error(err os.Error) {
+	panic(gobError{Error: err})
+}
+
+// catchError is meant to be used as a deferred function to turn a panic(gobError) into a
+// plain os.Error.  It overwrites the error return of the function that deferred its call.
+func catchError(err *os.Error) {
+	if e := recover(); e != nil {
+		*err = e.(gobError).Error // Will re-panic if not one of our errors, such as a runtime error.
+	}
+	return
+}