gob: finish up GobEncoder/Decoder by providing indirection

to the receiver. Remove lots of TODOS.

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

src/pkg/gob/decode.go

@@ -936,37 +936,29 @@ func (dec *Decoder) decIgnoreOpFor(wireId typeId) decOp {
 // GobDecoder.
 func (dec *Decoder) gobDecodeOpFor(ut *userTypeInfo) (*decOp, int) {
 	rt := ut.user
-	if ut.decIndir > 0 {
-		errorf("gob: TODO: can't handle >0 indirections to reach GobDecoder")
-	}
 	if ut.decIndir == -1 {
 		rt = reflect.PtrTo(rt)
+	} else if ut.decIndir > 0 {
+		for i := int8(0); i < ut.decIndir; i++ {
+			rt = rt.(*reflect.PtrType).Elem()
 		}
-	index := -1
-	for i := 0; i < rt.NumMethod(); i++ {
-		if rt.Method(i).Name == gobDecodeMethodName {
-			index = i
-			break
-		}
-	}
-	if index < 0 {
-		panic("can't find GobDecode method")
 	}
 	var op decOp
 	op = func(i *decInstr, state *decoderState, p unsafe.Pointer) {
 		// Allocate the underlying data, but hold on to the address we have,
-		// since it's known to be the receiver's address.
+		// since we need it to get to the receiver's address.
 		allocate(ut.base, uintptr(p), ut.indir)
 		var v reflect.Value
-		switch {
+		if ut.decIndir == -1 {
-		case ut.decIndir == 0:
+			// Need to climb up one level to turn value into pointer.
-			v = reflect.NewValue(unsafe.Unreflect(rt, p))
-		case ut.decIndir == -1:
 			v = reflect.NewValue(unsafe.Unreflect(rt, unsafe.Pointer(&p)))
-		default:
+		} else {
-			errorf("gob: TODO: can't handle >0 indirections to reach GobDecoder")
+			if ut.decIndir > 0 {
+				p = decIndirect(p, int(ut.decIndir))
 			}
-		state.dec.decodeGobDecoder(state, v, index)
+			v = reflect.NewValue(unsafe.Unreflect(rt, p))
+		}
+		state.dec.decodeGobDecoder(state, v, methodIndex(rt, gobDecodeMethodName))
 	}
 	return &op, int(ut.decIndir)
 
@@ -1190,9 +1182,6 @@ func (dec *Decoder) decodeValue(wireId typeId, val reflect.Value) (err os.Error)
 	indir := ut.indir
 	if ut.isGobDecoder {
 		indir = int(ut.decIndir)
-		if indir != 0 {
-			errorf("TODO: can't handle indirection in GobDecoder value")
-		}
 	}
 	enginePtr, err := dec.getDecEnginePtr(wireId, ut)
 	if err != nil {

src/pkg/gob/encode.go

@@ -92,11 +92,14 @@ func (state *encoderState) update(instr *encInstr) {
 	}
 }
 
-// Each encoder is responsible for handling any indirections associated
+// Each encoder for a composite is responsible for handling any
-// with the data structure.  If any pointer so reached is nil, no bytes are written.
+// indirections associated with the elements of the data structure.
-// If the data item is zero, no bytes are written.
+// If any pointer so reached is nil, no bytes are written.  If the
-// Otherwise, the output (for a scalar) is the field number, as an encoded integer,
+// data item is zero, no bytes are written.  Single values - ints,
-// followed by the field data in its appropriate format.
+// strings etc. - are indirected before calling their encoders.
+// Otherwise, the output (for a scalar) is the field number, as an
+// encoded integer, followed by the field data in its appropriate
+// format.
 
 // encIndirect dereferences p indir times and returns the result.
 func encIndirect(p unsafe.Pointer, indir int) unsafe.Pointer {
@@ -569,41 +572,40 @@ func (enc *Encoder) encOpFor(rt reflect.Type, inProgress map[reflect.Type]*encOp
 	return &op, indir
 }
 
+// methodIndex returns which method of rt implements the method.
+func methodIndex(rt reflect.Type, method string) int {
+	for i := 0; i < rt.NumMethod(); i++ {
+		if rt.Method(i).Name == method {
+			return i
+		}
+	}
+	panic("can't find method " + method)
+}
+
 // gobEncodeOpFor returns the op for a type that is known to implement
 // GobEncoder.
 func (enc *Encoder) gobEncodeOpFor(ut *userTypeInfo) (*encOp, int) {
 	rt := ut.user
-	if ut.encIndir > 0 {
-		errorf("gob: TODO: can't handle >0 indirections to reach GobEncoder")
-	}
 	if ut.encIndir == -1 {
 		rt = reflect.PtrTo(rt)
+	} else if ut.encIndir > 0 {
+		for i := int8(0); i < ut.encIndir; i++ {
+			rt = rt.(*reflect.PtrType).Elem()
 		}
-	index := -1
-	for i := 0; i < rt.NumMethod(); i++ {
-		if rt.Method(i).Name == gobEncodeMethodName {
-			index = i
-			break
-		}
-	}
-	if index < 0 {
-		panic("can't find GobEncode method")
 	}
 	var op encOp
 	op = func(i *encInstr, state *encoderState, p unsafe.Pointer) {
 		var v reflect.Value
-		switch {
+		if ut.encIndir == -1 {
-		case ut.encIndir == 0:
+			// Need to climb up one level to turn value into pointer.
-			v = reflect.NewValue(unsafe.Unreflect(rt, p))
-		case ut.encIndir == -1:
 			v = reflect.NewValue(unsafe.Unreflect(rt, unsafe.Pointer(&p)))
-		default:
+		} else {
-			errorf("gob: TODO: can't handle >0 indirections to reach GobEncoder")
+			v = reflect.NewValue(unsafe.Unreflect(rt, p))
 		}
 		state.update(i)
-		state.enc.encodeGobEncoder(state.b, v, index)
+		state.enc.encodeGobEncoder(state.b, v, methodIndex(rt, gobEncodeMethodName))
 	}
-	return &op, int(ut.encIndir)
+	return &op, int(ut.encIndir) // encIndir: op will get called with p == address of receiver.
 }
 
 // compileEnc returns the engine to compile the type.
@@ -666,9 +668,6 @@ func (enc *Encoder) encode(b *bytes.Buffer, value reflect.Value, ut *userTypeInf
 	indir := ut.indir
 	if ut.isGobEncoder {
 		indir = int(ut.encIndir)
-		if indir != 0 {
-			errorf("TODO: can't handle indirection in GobEncoder value")
-		}
 	}
 	for i := 0; i < indir; i++ {
 		value = reflect.Indirect(value)

src/pkg/gob/encoder.go

@@ -172,9 +172,6 @@ func (enc *Encoder) sendTypeDescriptor(w io.Writer, state *encoderState, ut *use
 	rt := ut.base
 	if ut.isGobEncoder {
 		rt = ut.user
-		if ut.encIndir != 0 {
-			panic("TODO: can't handle non-zero encIndir")
-		}
 	}
 	if _, alreadySent := enc.sent[rt]; !alreadySent {
 		// No, so send it.

src/pkg/gob/gobencdec_test.go

@@ -111,7 +111,7 @@ type GobTest2 struct {
 
 type GobTest3 struct {
 	X int // guarantee we have  something in common with GobTest*
-	G *Gobber // TODO: should be able to satisfy interface without a pointer
+	G *Gobber
 }
 
 type GobTest4 struct {
@@ -133,6 +133,11 @@ type GobTestValueEncDec struct {
 	G StringStruct // not a pointer.
 }
 
+type GobTestIndirectEncDec struct {
+	X int             // guarantee we have  something in common with GobTest*
+	G ***StringStruct // indirections to the receiver.
+}
+
 func TestGobEncoderField(t *testing.T) {
 	b := new(bytes.Buffer)
 	// First a field that's a structure.
@@ -188,6 +193,29 @@ func TestGobEncoderValueField(t *testing.T) {
 	}
 }
 
+// GobEncode/Decode should work even if the value is
+// more indirect than the receiver.
+func TestGobEncoderIndirectField(t *testing.T) {
+	b := new(bytes.Buffer)
+	// First a field that's a structure.
+	enc := NewEncoder(b)
+	s := &StringStruct{"HIJKL"}
+	sp := &s
+	err := enc.Encode(GobTestIndirectEncDec{17, &sp})
+	if err != nil {
+		t.Fatal("encode error:", err)
+	}
+	dec := NewDecoder(b)
+	x := new(GobTestIndirectEncDec)
+	err = dec.Decode(x)
+	if err != nil {
+		t.Fatal("decode error:", err)
+	}
+	if (***x.G).s != "HIJKL" {
+		t.Errorf("expected `HIJKL` got %s", (***x.G).s)
+	}
+}
+
 // As long as the fields have the same name and implement the
 // interface, we can cross-connect them.  Not sure it's useful
 // and may even be bad but it works and it's hard to prevent
@@ -301,8 +329,7 @@ func TestGobEncoderStructSingleton(t *testing.T) {
 func TestGobEncoderNonStructSingleton(t *testing.T) {
 	b := new(bytes.Buffer)
 	enc := NewEncoder(b)
-	g := Gobber(1234) // TODO: shouldn't need to take the address here.
+	err := enc.Encode(Gobber(1234))
-	err := enc.Encode(&g)
 	if err != nil {
 		t.Fatal("encode error:", err)
 	}

src/pkg/gob/type.go

@@ -77,11 +77,6 @@ func validUserType(rt reflect.Type) (ut *userTypeInfo, err os.Error) {
 	ut.isGobEncoder, ut.encIndir = implementsInterface(ut.user, gobEncoderCheck)
 	ut.isGobDecoder, ut.decIndir = implementsInterface(ut.user, gobDecoderCheck)
 	userTypeCache[rt] = ut
-	if ut.encIndir > 0 || ut.decIndir > 0 {
-		// There are checks in lots of other places, but putting this here means we won't even
-		// attempt to encode/decode this type.
-		return nil, os.ErrorString("TODO: gob can't handle indirections to GobEncoder/Decoder")
-	}
 	return
 }
 
@@ -123,7 +118,7 @@ func implementsInterface(typ reflect.Type, check func(typ reflect.Type) bool) (s
 	// The type might be a pointer and we need to keep
 	// dereferencing to the base type until we find an implementation.
 	for {
-		if implements(typ, check) {
+		if implements(rt, check) {
 			return true, indir
 		}
 		if p, ok := rt.(*reflect.PtrType); ok {
@@ -697,11 +692,6 @@ func mustGetTypeInfo(rt reflect.Type) *typeInfo {
 // to guarantee the encoding used by a GobEncoder is stable as the
 // software evolves.  For instance, it might make sense for GobEncode
 // to include a version number in the encoding.
-//
-// Note: At the moment, the type implementing GobEncoder must
-// be more indirect than the type passed to Decode.  For example, if
-// if *T implements GobDecoder, the data item must be of type *T or T,
-// not **T or ***T.
 type GobEncoder interface {
 	// GobEncode returns a byte slice representing the encoding of the
 	// receiver for transmission to a GobDecoder, usually of the same
@@ -711,11 +701,6 @@ type GobEncoder interface {
 
 // GobDecoder is the interface describing data that provides its own
 // routine for decoding transmitted values sent by a GobEncoder.
-//
-// Note: At the moment, the type implementing GobDecoder must
-// be more indirect than the type passed to Decode.  For example, if
-// if *T implements GobDecoder, the data item must be of type *T or T,
-// not **T or ***T.
 type GobDecoder interface {
 	// GobDecode overwrites the receiver, which must be a pointer,
 	// with the value represented by the byte slice, which was written