goprotobuf: Split encoding of int32 and uint32 fields.

int32 needs special handling; negative values need to be sign-extended,
so need to be converted from uint32 back to int32 before converting to
uint64 for the varint encoding step (yielding 10 bytes).
uint32 is simpler and stays as just encoding the bit pattern,
and thus never takes more than 5 bytes.
This permits upgrading int32 fields to int64, and matches C++.

LGTM=nigeltao
R=nigeltao
CC=golang-codereviews
https://codereview.appspot.com/114190043

proto/all_test.go

@@ -1047,6 +1047,35 @@ func TestSubmessageUnrecognizedFields(t *testing.T) {
 	}
 }
 
+// Check that an int32 field can be upgraded to an int64 field.
+func TestNegativeInt32(t *testing.T) {
+	om := &OldMessage{
+		Num: Int32(-1),
+	}
+	b, err := Marshal(om)
+	if err != nil {
+		t.Fatalf("Marshal of OldMessage: %v", err)
+	}
+
+	// Check the size. It should be 11 bytes;
+	// 1 for the field/wire type, and 10 for the negative number.
+	if len(b) != 11 {
+		t.Errorf("%v marshaled as %q, wanted 11 bytes", om, b)
+	}
+
+	// Unmarshal into a NewMessage.
+	nm := new(NewMessage)
+	if err := Unmarshal(b, nm); err != nil {
+		t.Fatalf("Unmarshal to NewMessage: %v", err)
+	}
+	want := &NewMessage{
+		Num: Int64(-1),
+	}
+	if !Equal(nm, want) {
+		t.Errorf("nm = %v, want %v", nm, want)
+	}
+}
+
 // 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
@@ -1710,7 +1739,8 @@ func TestEncodingSizes(t *testing.T) {
 		n int
 	}{
 		{&Defaults{F_Int32: Int32(math.MaxInt32)}, 6},
-		{&Defaults{F_Int32: Int32(math.MinInt32)}, 6},
+		{&Defaults{F_Int32: Int32(math.MinInt32)}, 11},
+		{&Defaults{F_Uint32: Uint32(uint32(math.MaxInt32) + 1)}, 6},
 		{&Defaults{F_Uint32: Uint32(math.MaxUint32)}, 6},
 	}
 	for _, test := range tests {

proto/encode.go

@@ -312,13 +312,37 @@ func (o *Buffer) enc_int32(p *Properties, base structPointer) error {
 	if word32_IsNil(v) {
 		return ErrNil
 	}
-	x := word32_Get(v)
+	x := int32(word32_Get(v)) // permit sign extension to use full 64-bit range
 	o.buf = append(o.buf, p.tagcode...)
 	p.valEnc(o, uint64(x))
 	return nil
 }
 
 func size_int32(p *Properties, base structPointer) (n int) {
+	v := structPointer_Word32(base, p.field)
+	if word32_IsNil(v) {
+		return 0
+	}
+	x := int32(word32_Get(v)) // permit sign extension to use full 64-bit range
+	n += len(p.tagcode)
+	n += p.valSize(uint64(x))
+	return
+}
+
+// Encode a uint32.
+// Exactly the same as int32, except for no sign extension.
+func (o *Buffer) enc_uint32(p *Properties, base structPointer) error {
+	v := structPointer_Word32(base, p.field)
+	if word32_IsNil(v) {
+		return ErrNil
+	}
+	x := word32_Get(v)
+	o.buf = append(o.buf, p.tagcode...)
+	p.valEnc(o, uint64(x))
+	return nil
+}
+
+func size_uint32(p *Properties, base structPointer) (n int) {
 	v := structPointer_Word32(base, p.field)
 	if word32_IsNil(v) {
 		return 0

proto/properties.go

@@ -308,18 +308,22 @@ func (p *Properties) setEncAndDec(typ reflect.Type, lockGetProp bool) {
 			p.enc = (*Buffer).enc_bool
 			p.dec = (*Buffer).dec_bool
 			p.size = size_bool
-		case reflect.Int32, reflect.Uint32:
+		case reflect.Int32:
 			p.enc = (*Buffer).enc_int32
 			p.dec = (*Buffer).dec_int32
 			p.size = size_int32
+		case reflect.Uint32:
+			p.enc = (*Buffer).enc_uint32
+			p.dec = (*Buffer).dec_int32 // can reuse
+			p.size = size_uint32
 		case reflect.Int64, reflect.Uint64:
 			p.enc = (*Buffer).enc_int64
 			p.dec = (*Buffer).dec_int64
 			p.size = size_int64
 		case reflect.Float32:
-			p.enc = (*Buffer).enc_int32 // can just treat them as bits
+			p.enc = (*Buffer).enc_uint32 // can just treat them as bits
 			p.dec = (*Buffer).dec_int32
-			p.size = size_int32
+			p.size = size_uint32
 		case reflect.Float64:
 			p.enc = (*Buffer).enc_int64 // can just treat them as bits
 			p.dec = (*Buffer).dec_int64

proto/size_test.go

@@ -65,8 +65,10 @@ var SizeTests = []struct {
 	// Basic types.
 	{"bool", &pb.Defaults{F_Bool: Bool(true)}},
 	{"int32", &pb.Defaults{F_Int32: Int32(12)}},
+	{"negative int32", &pb.Defaults{F_Int32: Int32(-1)}},
 	{"small int64", &pb.Defaults{F_Int64: Int64(1)}},
 	{"big int64", &pb.Defaults{F_Int64: Int64(1 << 20)}},
+	{"negative int64", &pb.Defaults{F_Int64: Int64(-1)}},
 	{"fixed32", &pb.Defaults{F_Fixed32: Uint32(71)}},
 	{"fixed64", &pb.Defaults{F_Fixed64: Uint64(72)}},
 	{"uint32", &pb.Defaults{F_Uint32: Uint32(123)}},

proto/testdata/test.pb.go

@@ -1070,6 +1070,7 @@ func (m *MaxTag) GetLastField() string {
 
 type OldMessage struct {
 	Nested           *OldMessage_Nested `protobuf:"bytes,1,opt,name=nested" json:"nested,omitempty"`
+	Num              *int32             `protobuf:"varint,2,opt,name=num" json:"num,omitempty"`
 	XXX_unrecognized []byte             `json:"-"`
 }
 
@@ -1084,6 +1085,13 @@ func (m *OldMessage) GetNested() *OldMessage_Nested {
 	return nil
 }
 
+func (m *OldMessage) GetNum() int32 {
+	if m != nil && m.Num != nil {
+		return *m.Num
+	}
+	return 0
+}
+
 type OldMessage_Nested struct {
 	Name             *string `protobuf:"bytes,1,opt,name=name" json:"name,omitempty"`
 	XXX_unrecognized []byte  `json:"-"`
@@ -1103,8 +1111,10 @@ func (m *OldMessage_Nested) GetName() string {
 // NewMessage is wire compatible with OldMessage;
 // imagine it as a future version.
 type NewMessage struct {
-	Nested           *NewMessage_Nested `protobuf:"bytes,1,opt,name=nested" json:"nested,omitempty"`
-	XXX_unrecognized []byte             `json:"-"`
+	Nested *NewMessage_Nested `protobuf:"bytes,1,opt,name=nested" json:"nested,omitempty"`
+	// This is an int32 in OldMessage.
+	Num              *int64 `protobuf:"varint,2,opt,name=num" json:"num,omitempty"`
+	XXX_unrecognized []byte `json:"-"`
 }
 
 func (m *NewMessage) Reset()         { *m = NewMessage{} }
@@ -1118,6 +1128,13 @@ func (m *NewMessage) GetNested() *NewMessage_Nested {
 	return nil
 }
 
+func (m *NewMessage) GetNum() int64 {
+	if m != nil && m.Num != nil {
+		return *m.Num
+	}
+	return 0
+}
+
 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"`

proto/testdata/test.proto

@@ -203,6 +203,8 @@ message OldMessage {
     optional string name = 1;
   }
   optional Nested nested = 1;
+
+  optional int32 num = 2;
 }
 
 // NewMessage is wire compatible with OldMessage;
@@ -213,6 +215,9 @@ message NewMessage {
     optional string food_group = 2;
   }
   optional Nested nested = 1;
+
+  // This is an int32 in OldMessage.
+  optional int64 num = 2;
 }
 
 // Smaller tests for ASCII formatting.

protoc-gen-go/generator/generator.go

@@ -134,6 +134,7 @@ type EnumDescriptor struct {
 	*descriptor.EnumDescriptorProto
 	parent   *Descriptor // The containing message, if any.
 	typename []string    // Cached typename vector.
+	index    int         // The index into the container, whether the file or a message.
 	path     string      // The SourceCodeInfo path as comma-separated integers.
 }
 
@@ -769,6 +770,7 @@ func newEnumDescriptor(desc *descriptor.EnumDescriptorProto, parent *Descriptor,
 		common:              common{file},
 		EnumDescriptorProto: desc,
 		parent:              parent,
+		index:               index,
 	}
 	if parent == nil {
 		ed.path = fmt.Sprintf("%d,%d", enumPath, index)