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

7 files changed, 91 insertions, 7 deletions

diff --git a/proto/all_test.go b/proto/all_test.go
index a54e3a3..1ac9587 100644
--- a/proto/all_test.go
+++ b/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 {
diff --git a/proto/encode.go b/proto/encode.go
index 2d3e03f..b160372 100644
--- a/proto/encode.go
+++ b/proto/encode.go
@@ -312,7 +312,7 @@ 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
@@ -323,6 +323,30 @@ func size_int32(p *Properties, base structPointer) (n int) {
 	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
+	}
 	x := word32_Get(v)
 	n += len(p.tagcode)
 	n += p.valSize(uint64(x))
diff --git a/proto/properties.go b/proto/properties.go
index 7177cfc..da352f4 100644
--- a/proto/properties.go
+++ b/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
diff --git a/proto/size_test.go b/proto/size_test.go
index 1d08547..5aab80b 100644
--- a/proto/size_test.go
+++ b/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)}},
diff --git a/proto/testdata/test.pb.go b/proto/testdata/test.pb.go
index 823e6f5..9ff2d1b 100644
--- a/proto/testdata/test.pb.go
+++ b/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"`
diff --git a/proto/testdata/test.proto b/proto/testdata/test.proto
index 4f4b3d1..f1fbf2b 100644
--- a/proto/testdata/test.proto
+++ b/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.
diff --git a/protoc-gen-go/generator/generator.go b/protoc-gen-go/generator/generator.go
index cf2367b..0ed6175 100644
--- a/protoc-gen-go/generator/generator.go
+++ b/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)