goprotobuf: Handle XXX_unrecognized correctly.

In particular,
  - add it to marshaled messages
  - examine it in Clone, Equal

R=r
CC=golang-dev
http://codereview.appspot.com/6449091

proto/all_test.go

@@ -940,6 +940,55 @@ func TestSkippingUnrecognizedFields(t *testing.T) {
 	}
 }
 
+// Check that unrecognized fields of a submessage are preserved.
+func TestSubmessageUnrecognizedFields(t *testing.T) {
+	nm := &NewMessage{
+		Nested: &NewMessage_Nested{
+			Name:      String("Nigel"),
+			FoodGroup: String("carbs"),
+		},
+	}
+	b, err := Marshal(nm)
+	if err != nil {
+		t.Fatalf("Marshal of NewMessage: %v", err)
+	}
+
+	// Unmarshal into an OldMessage.
+	om := new(OldMessage)
+	if err := Unmarshal(b, om); err != nil {
+		t.Fatalf("Unmarshal to OldMessage: %v", err)
+	}
+	exp := &OldMessage{
+		Nested: &OldMessage_Nested{
+			Name: String("Nigel"),
+			// normal protocol buffer users should not do this
+			XXX_unrecognized: []byte("\x12\x05carbs"),
+		},
+	}
+	if !Equal(om, exp) {
+		t.Errorf("om = %v, want %v", om, exp)
+	}
+
+	// Clone the OldMessage.
+	om = Clone(om).(*OldMessage)
+	if !Equal(om, exp) {
+		t.Errorf("Clone(om) = %v, want %v", om, exp)
+	}
+
+	// Marshal the OldMessage, then unmarshal it into an empty NewMessage.
+	if b, err = Marshal(om); err != nil {
+		t.Fatalf("Marshal of OldMessage: %v", err)
+	}
+	t.Logf("Marshal(%v) -> %q", om, b)
+	nm2 := new(NewMessage)
+	if err := Unmarshal(b, nm2); err != nil {
+		t.Fatalf("Unmarshal to NewMessage: %v", err)
+	}
+	if !Equal(nm, nm2) {
+		t.Errorf("NewMessage round-trip: %v => %v", nm, nm2)
+	}
+}
+
 // Check that we can grow an array (repeated field) to have many elements.
 // This test doesn't depend only on our encoding; for variety, it makes sure
 // we create, encode, and decode the correct contents explicitly.  It's therefore

proto/clone.go

@@ -43,8 +43,8 @@ import (
 // Clone returns a deep copy of a protocol buffer.
 func Clone(pb Message) Message {
 	in := reflect.ValueOf(pb)
-	if in.Kind() != reflect.Ptr || in.Elem().Kind() != reflect.Struct {
-		return nil
+	if in.IsNil() {
+		return pb
 	}
 
 	out := reflect.New(in.Type().Elem())
@@ -66,12 +66,17 @@ func copyStruct(out, in reflect.Value) {
 		copyExtension(emOut.ExtensionMap(), emIn.ExtensionMap())
 	}
 
-	// TODO: Deal with XXX_unrecognized.
+	uin := in.FieldByName("XXX_unrecognized").Bytes()
+	if len(uin) > 0 {
+		out.FieldByName("XXX_unrecognized").SetBytes(append([]byte(nil), uin...))
+	}
 }
 
 func copyAny(out, in reflect.Value) {
 	if in.Type() == protoMessageType {
-		out.Set(reflect.ValueOf(Clone(in.Interface().(Message))))
+		if !in.IsNil() {
+			out.Set(reflect.ValueOf(Clone(in.Interface().(Message))))
+		}
 		return
 	}
 	switch in.Kind() {

proto/clone_test.go

@@ -78,3 +78,10 @@ func TestClone(t *testing.T) {
 		t.Error("Mutating clone changed the original")
 	}
 }
+
+func TestCloneNil(t *testing.T) {
+	var m *pb.MyMessage
+	if c := proto.Clone(m); !proto.Equal(m, c) {
+		t.Errorf("Clone(%v) = %v", m, c)
+	}
+}

proto/encode.go

@@ -580,5 +580,11 @@ func (o *Buffer) enc_struct(t reflect.Type, base uintptr) error {
 		return &ErrRequiredNotSet{t}
 	}
 
+	// Add unrecognized fields at the end.
+	v := *(*[]byte)(unsafe.Pointer(base + prop.unrecOffset))
+	if prop.unrecOffset > 0 && len(v) > 0 {
+		o.buf = append(o.buf, v...)
+	}
+
 	return nil
 }

proto/equal.go

@@ -126,7 +126,11 @@ func equalStruct(v1, v2 reflect.Value) bool {
 		}
 	}
 
-	// TODO: Deal with XXX_unrecognized.
+	u1 := v1.FieldByName("XXX_unrecognized").Bytes()
+	u2 := v2.FieldByName("XXX_unrecognized").Bytes()
+	if !bytes.Equal(u1, u2) {
+		return false
+	}
 
 	return true
 }

proto/message_set.go

@@ -36,7 +36,6 @@ package proto
  */
 
 import (
-	"bytes"
 	"errors"
 	"reflect"
 )
@@ -70,7 +69,7 @@ type _MessageSet_Item struct {
 
 type MessageSet struct {
 	Item             []*_MessageSet_Item `protobuf:"group,1,rep"`
-	XXX_unrecognized *bytes.Buffer
+	XXX_unrecognized []byte
 	// TODO: caching?
 }
 

proto/properties.go

@@ -82,6 +82,8 @@ type StructProperties struct {
 	tags      map[int]int    // map from proto tag to struct field number
 	origNames map[string]int // map from original name to struct field number
 	order     []int          // list of struct field numbers in tag order
+
+	unrecOffset uintptr // offset of the XXX_unrecognized []byte field
 }
 
 // Implement the sorting interface so we can sort the fields in tag order, as recommended by the spec.
@@ -458,6 +460,9 @@ func GetProperties(t reflect.Type) *StructProperties {
 			p.enc = (*Buffer).enc_map
 			p.dec = nil // not needed
 		}
+		if f.Name == "XXX_unrecognized" { // special case
+			prop.unrecOffset = f.Offset
+		}
 		prop.Prop[i] = p
 		prop.order[i] = i
 		if debug {

proto/testdata/test.pb.go

@@ -837,6 +837,78 @@ func (this *MaxTag) GetLastField() string {
 	return ""
 }
 
+type OldMessage struct {
+	Nested           *OldMessage_Nested `protobuf:"bytes,1,opt,name=nested" json:"nested,omitempty"`
+	XXX_unrecognized []byte             `json:"-"`
+}
+
+func (this *OldMessage) Reset()         { *this = OldMessage{} }
+func (this *OldMessage) String() string { return proto.CompactTextString(this) }
+func (*OldMessage) ProtoMessage()       {}
+
+func (this *OldMessage) GetNested() *OldMessage_Nested {
+	if this != nil {
+		return this.Nested
+	}
+	return nil
+}
+
+type OldMessage_Nested struct {
+	Name             *string `protobuf:"bytes,1,opt,name=name" json:"name,omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
+}
+
+func (this *OldMessage_Nested) Reset()         { *this = OldMessage_Nested{} }
+func (this *OldMessage_Nested) String() string { return proto.CompactTextString(this) }
+func (*OldMessage_Nested) ProtoMessage()       {}
+
+func (this *OldMessage_Nested) GetName() string {
+	if this != nil && this.Name != nil {
+		return *this.Name
+	}
+	return ""
+}
+
+type NewMessage struct {
+	Nested           *NewMessage_Nested `protobuf:"bytes,1,opt,name=nested" json:"nested,omitempty"`
+	XXX_unrecognized []byte             `json:"-"`
+}
+
+func (this *NewMessage) Reset()         { *this = NewMessage{} }
+func (this *NewMessage) String() string { return proto.CompactTextString(this) }
+func (*NewMessage) ProtoMessage()       {}
+
+func (this *NewMessage) GetNested() *NewMessage_Nested {
+	if this != nil {
+		return this.Nested
+	}
+	return nil
+}
+
+type NewMessage_Nested struct {
+	Name             *string `protobuf:"bytes,1,opt,name=name" json:"name,omitempty"`
+	FoodGroup        *string `protobuf:"bytes,2,opt,name=food_group" json:"food_group,omitempty"`
+	XXX_unrecognized []byte  `json:"-"`
+}
+
+func (this *NewMessage_Nested) Reset()         { *this = NewMessage_Nested{} }
+func (this *NewMessage_Nested) String() string { return proto.CompactTextString(this) }
+func (*NewMessage_Nested) ProtoMessage()       {}
+
+func (this *NewMessage_Nested) GetName() string {
+	if this != nil && this.Name != nil {
+		return *this.Name
+	}
+	return ""
+}
+
+func (this *NewMessage_Nested) GetFoodGroup() string {
+	if this != nil && this.FoodGroup != nil {
+		return *this.FoodGroup
+	}
+	return ""
+}
+
 type InnerMessage struct {
 	Host             *string `protobuf:"bytes,1,req,name=host" json:"host,omitempty"`
 	Port             *int32  `protobuf:"varint,2,opt,name=port,def=4000" json:"port,omitempty"`

proto/testdata/test.proto

@@ -198,6 +198,23 @@ message MaxTag {
   optional string last_field = 536870911;
 }
 
+message OldMessage {
+  message Nested {
+    optional string name = 1;
+  }
+  optional Nested nested = 1;
+}
+
+// NewMessage is wire compatible with OldMessage;
+// imagine it as a future version.
+message NewMessage {
+  message Nested {
+    optional string name = 1;
+    optional string food_group = 2;
+  }
+  optional Nested nested = 1;
+}
+
 // Smaller tests for ASCII formatting.
 
 message InnerMessage {