clean up the code, flow errors out to decoder.

R=rsc
DELTA=99  (32 added, 22 deleted, 45 changed)
OCL=31759
CL=31759

src/pkg/gob/decode.go

@@ -17,6 +17,11 @@ import (
 	"unsafe";
 )
 
+var (
+	ErrRange = os.ErrorString("gob: internal error: field numbers out of bounds");
+	ErrNotStruct = os.ErrorString("gob: TODO: can only handle structs")
+)
+
 // The global execution state of an instance of the decoder.
 type decodeState struct {
 	b	*bytes.Buffer;
@@ -342,7 +347,8 @@ func decodeStruct(engine *decEngine, rtyp *reflect.StructType, b *bytes.Buffer,
 		}
 		fieldnum := state.fieldnum + delta;
 		if fieldnum >= len(engine.instr) {
-			panicln("TODO(r): field number out of range", fieldnum, len(engine.instr));
+			state.err = ErrRange;
+			break;
 		}
 		instr := &engine.instr[fieldnum];
 		p := unsafe.Pointer(basep+instr.offset);
@@ -452,11 +458,11 @@ var decIgnoreOpMap = map[TypeId] decOp {
 	tString: ignoreUint8Array,
 }
 
-func getDecEnginePtr(wireId TypeId, rt reflect.Type) **decEngine
+func getDecEnginePtr(wireId TypeId, rt reflect.Type) (enginePtr **decEngine, err os.Error)
 
 // Return the decoding op for the base type under rt and
 // the indirection count to reach it.
-func decOpFor(wireId TypeId, rt reflect.Type) (decOp, int) {
+func decOpFor(wireId TypeId, rt reflect.Type) (decOp, int, os.Error) {
 	typ, indir := indirect(rt);
 	op, ok := decOpMap[reflect.Typeof(typ)];
 	if !ok {
@@ -468,21 +474,30 @@ func decOpFor(wireId TypeId, rt reflect.Type) (decOp, int) {
 				break;
 			}
 			elemId := wireId.gobType().(*sliceType).Elem;
-			elemOp, elemIndir := decOpFor(elemId, t.Elem());
+			elemOp, elemIndir, err := decOpFor(elemId, t.Elem());
+			if err != nil {
+				return nil, 0, err
+			}
 			op = func(i *decInstr, state *decodeState, p unsafe.Pointer) {
 				state.err = decodeSlice(t, state, uintptr(p), elemOp, t.Elem().Size(), i.indir, elemIndir);
 			};
 
 		case *reflect.ArrayType:
 			elemId := wireId.gobType().(*arrayType).Elem;
-			elemOp, elemIndir := decOpFor(elemId, t.Elem());
+			elemOp, elemIndir, err := decOpFor(elemId, t.Elem());
+			if err != nil {
+				return nil, 0, err
+			}
 			op = func(i *decInstr, state *decodeState, p unsafe.Pointer) {
 				state.err = decodeArray(t, state, uintptr(p), elemOp, t.Elem().Size(), t.Len(), i.indir, elemIndir);
 			};
 
 		case *reflect.StructType:
 			// Generate a closure that calls out to the engine for the nested type.
-			enginePtr := getDecEnginePtr(wireId, typ);
+			enginePtr, err := getDecEnginePtr(wireId, typ);
+			if err != nil {
+				return nil, 0, err
+			}
 			op = func(i *decInstr, state *decodeState, p unsafe.Pointer) {
 				// indirect through info to delay evaluation for recursive structs
 				state.err = decodeStruct(*enginePtr, t, state.b, uintptr(p), i.indir)
@@ -490,27 +505,33 @@ func decOpFor(wireId TypeId, rt reflect.Type) (decOp, int) {
 		}
 	}
 	if op == nil {
-		panicln("decode can't handle type", rt.String());
+		return nil, 0, os.ErrorString("gob: decode can't handle type " + rt.String());
 	}
-	return op, indir
+	return op, indir, nil
 }
 
 // Return the decoding op for a field that has no destination.
-func decIgnoreOpFor(wireId TypeId) decOp {
+func decIgnoreOpFor(wireId TypeId) (decOp, os.Error) {
 	op, ok := decIgnoreOpMap[wireId];
 	if !ok {
 		// Special cases
 		switch t := wireId.gobType().(type) {
 		case *sliceType:
 			elemId := wireId.gobType().(*sliceType).Elem;
-			elemOp := decIgnoreOpFor(elemId);
+			elemOp, err := decIgnoreOpFor(elemId);
+			if err != nil {
+				return nil, err
+			}
 			op = func(i *decInstr, state *decodeState, p unsafe.Pointer) {
 				state.err = ignoreSlice(state, elemOp);
 			};
 
 		case *arrayType:
 			elemId := wireId.gobType().(*arrayType).Elem;
-			elemOp := decIgnoreOpFor(elemId);
+			elemOp, err := decIgnoreOpFor(elemId);
+			if err != nil {
+				return nil, err
+			}
 			op = func(i *decInstr, state *decodeState, p unsafe.Pointer) {
 				state.err = ignoreArray(state, elemOp, t.Len);
 			};
@@ -520,9 +541,9 @@ func decIgnoreOpFor(wireId TypeId) decOp {
 		}
 	}
 	if op == nil {
-		panicln("decode can't handle type", wireId.gobType().String());
+		return nil, os.ErrorString("ignore can't handle type " + wireId.String());
 	}
-	return op;
+	return op, nil;
 }
 
 // Are these two gob Types compatible?
@@ -588,48 +609,46 @@ func compatibleType(fr reflect.Type, fw TypeId) bool {
 	return true;
 }
 
-func compileDec(wireId TypeId, rt reflect.Type) *decEngine {
+func compileDec(wireId TypeId, rt reflect.Type) (engine *decEngine, err os.Error) {
 	srt, ok1 := rt.(*reflect.StructType);
 	wireStruct, ok2 := wireId.gobType().(*structType);
 	if !ok1 || !ok2 {
-		panicln("gob: TODO: can't handle non-structs");
+		return nil, ErrNotStruct
 	}
-	engine := new(decEngine);
+	engine = new(decEngine);
 	engine.instr = make([]decInstr, len(wireStruct.field));
 	// Loop over the fields of the wire type.
 	for fieldnum := 0; fieldnum < len(wireStruct.field); fieldnum++ {
 		wireField := wireStruct.field[fieldnum];
 		// Find the field of the local type with the same name.
-		// TODO: put this as a method in reflect
+		localField, present := srt.FieldByName(wireField.name);
-		var localField reflect.StructField;
-		for lfn := 0; lfn < srt.NumField(); lfn++ {
-			if srt.Field(lfn).Name == wireField.name {
-				localField = srt.Field(lfn);
-				break;
-			}
-		}
 		// TODO(r): anonymous names
-		if localField.Anonymous || localField.Name == "" {
+		if !present || localField.Anonymous {
 			println("no matching field", wireField.name, "in type", wireId.String());
-			op := decIgnoreOpFor(wireField.typeId);
+			op, err := decIgnoreOpFor(wireField.typeId);
+			if err != nil {
+				return nil, err
+			}
 			engine.instr[fieldnum] = decInstr{op, fieldnum, 0, 0};
 			continue;
 		}
 		if !compatibleType(localField.Type, wireField.typeId) {
-			panicln("TODO: wrong type for field", wireField.name, "in type", wireId.String());
+			return nil, os.ErrorString("gob: TODO: wrong type for field " + wireField.name + " in type " + wireId.String());
+		}
+		op, indir, err := decOpFor(wireField.typeId, localField.Type);
+		if err != nil {
+			return nil, err
 		}
-		op, indir := decOpFor(wireField.typeId, localField.Type);
 		engine.instr[fieldnum] = decInstr{op, fieldnum, indir, uintptr(localField.Offset)};
 		engine.numInstr++;
 	}
-	return engine;
+	return;
 }
 
 
 // typeLock must be held.
-func getDecEnginePtr(wireId TypeId, rt reflect.Type) **decEngine {
+func getDecEnginePtr(wireId TypeId, rt reflect.Type) (enginePtr **decEngine, err os.Error) {
 	info := getTypeInfo(rt);	// TODO: eliminate this; creates a gobType you don't need.
-	var enginePtr **decEngine;
 	var ok bool;
 	if enginePtr, ok = info.decoderPtr[wireId]; !ok {
 		if info.typeId.gobType() == nil {
@@ -639,9 +658,12 @@ func getDecEnginePtr(wireId TypeId, rt reflect.Type) **decEngine {
 		// mark this engine as underway before compiling to handle recursive types.
 		enginePtr = new(*decEngine);
 		info.decoderPtr[wireId] = enginePtr;
-		*enginePtr = compileDec(wireId, rt);
+		*enginePtr, err = compileDec(wireId, rt);
+		if err != nil {
+			info.decoderPtr[wireId] = nil, false;
+		}
 	}
-	return enginePtr
+	return
 }
 
 func decode(b *bytes.Buffer, wireId TypeId, e interface{}) os.Error {
@@ -657,11 +679,15 @@ func decode(b *bytes.Buffer, wireId TypeId, e interface{}) os.Error {
 		return os.ErrorString("gob: decode can't handle " + rt.String())
 	}
 	typeLock.Lock();
-	engine := *getDecEnginePtr(wireId, rt);
+	enginePtr, err := getDecEnginePtr(wireId, rt);
 	typeLock.Unlock();
+	if err != nil {
+		return err
+	}
+	engine := *enginePtr;
 	if engine.numInstr == 0 && st.NumField() > 0 {
 		path, name := rt.Name();
-		return os.ErrorString("no fields matched compiling decoder for " + name)
+		return os.ErrorString("type mismatch: no fields matched compiling decoder for " + name)
 	}
 	return decodeStruct(engine, rt.(*reflect.StructType), b, uintptr(v.Addr()), 0);
 }

src/pkg/gob/decoder.go

@@ -83,7 +83,7 @@ func (dec *Decoder) Decode(e interface{}) os.Error {
 		// Receive a type id.
 		id := TypeId(decodeInt(dec.state));
 		if dec.state.err != nil {
-			return dec.state.err
+			break;
 		}
 
 		// Is it a new type?
@@ -91,28 +91,14 @@ func (dec *Decoder) Decode(e interface{}) os.Error {
 			// If the id is negative, we have a type.
 			dec.recvType(-id);
 			if dec.state.err != nil {
-				return dec.state.err
+				break;
 			}
 			continue;
 		}
 
 		// No, it's a value.
-		typeLock.Lock();
+		dec.state.err = decode(dec.state.b, id, e);
-		info := getTypeInfo(rt);
+		break;
-		typeLock.Unlock();
-
-		// Check type compatibility.
-		// TODO(r): need to make the decoder work correctly if the wire type is compatible
-		// but not equal to the local type (e.g, extra fields).
-		if info.wire.name() != dec.seen[id].name() {
-			dec.state.err = os.ErrorString("gob decode: incorrect type for wire value: want " + info.wire.name() + "; received " + dec.seen[id].name());
-			return dec.state.err
-		}
-
-		// Receive a value.
-		decode(dec.state.b, id, e);
-
-		return dec.state.err
 	}
-	return nil	// silence compiler
+	return dec.state.err
 }

src/pkg/gob/encode.go

@@ -372,7 +372,7 @@ func encOpFor(rt reflect.Type) (encOp, int) {
 		}
 	}
 	if op == nil {
-		panicln("encode can't handle type", rt.String());
+		panicln("can't happen: encode type", rt.String());
 	}
 	return op, indir
 }
@@ -381,7 +381,7 @@ func encOpFor(rt reflect.Type) (encOp, int) {
 func compileEnc(rt reflect.Type) *encEngine {
 	srt, ok := rt.(*reflect.StructType);
 	if !ok {
-		panicln("TODO: can't handle non-structs");
+		panicln("can't happen: non-struct");
 	}
 	engine := new(encEngine);
 	engine.instr = make([]encInstr, srt.NumField()+1);	// +1 for terminator

src/pkg/gob/encoder_test.go

@@ -38,11 +38,9 @@ type ET4 struct {
 	next *ET2;
 }
 
-// Like ET1 but with a different type for a self-referencing field
+// Has different type for a self-referencing field compared to ET1
 type ET5 struct {
-	a int;
+	next *ET2;
-	et2 *ET2;
-	next *ET1;
 }
 
 func TestBasicEncoder(t *testing.T) {
@@ -227,7 +225,7 @@ func badTypeCheck(e interface{}, msg string, t *testing.T) {
 
 // Test that we recognize a bad type the first time.
 func TestWrongTypeDecoder(t *testing.T) {
-	badTypeCheck(new(ET2), "different number of fields", t);
+	badTypeCheck(new(ET2), "no fields in common", t);
 	badTypeCheck(new(ET3), "different name of field", t);
 	badTypeCheck(new(ET4), "different type of field", t);
 	badTypeCheck(new(ET5), "different type of self-reference field", t);