goprotobuf: Fix Clone to deep copy []byte fields.

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

proto/clone.go

@@ -125,6 +125,12 @@ func mergeAny(out, in reflect.Value) {
 		if in.IsNil() {
 			return
 		}
+		if in.Type().Elem().Kind() == reflect.Uint8 {
+			// []byte is a scalar bytes field, not a repeated field.
+			// Make a deep copy.
+			out.SetBytes(append([]byte(nil), in.Bytes()...))
+			return
+		}
 		n := in.Len()
 		if out.IsNil() {
 			out.Set(reflect.MakeSlice(in.Type(), 0, n))
@@ -133,9 +139,6 @@ func mergeAny(out, in reflect.Value) {
 		case reflect.Bool, reflect.Float32, reflect.Float64, reflect.Int32, reflect.Int64,
 			reflect.String, reflect.Uint32, reflect.Uint64:
 			out.Set(reflect.AppendSlice(out, in))
-		case reflect.Uint8:
-			// []byte is a scalar bytes field.
-			out.Set(in)
 		default:
 			for i := 0; i < n; i++ {
 				x := reflect.Indirect(reflect.New(in.Type().Elem()))

proto/clone_test.go

@@ -79,6 +79,22 @@ func TestClone(t *testing.T) {
 	if proto.Equal(m, cloneTestMessage) {
 		t.Error("Mutating clone changed the original")
 	}
+	// Byte fields and repeated fields should be copied.
+	if &m.Pet[0] == &cloneTestMessage.Pet[0] {
+		t.Error("Pet: repeated field not copied")
+	}
+	if &m.Others[0] == &cloneTestMessage.Others[0] {
+		t.Error("Others: repeated field not copied")
+	}
+	if &m.Others[0].Value[0] == &cloneTestMessage.Others[0].Value[0] {
+		t.Error("Others[0].Value: bytes field not copied")
+	}
+	if &m.RepBytes[0] == &cloneTestMessage.RepBytes[0] {
+		t.Error("RepBytes: repeated field not copied")
+	}
+	if &m.RepBytes[0][0] == &cloneTestMessage.RepBytes[0][0] {
+		t.Error("RepBytes[0]: bytes field not copied")
+	}
 }
 
 func TestCloneNil(t *testing.T) {

proto/text_parser.go

@@ -430,6 +430,7 @@ func (p *textParser) readStruct(sv reflect.Value, terminator string) error {
 	st := sv.Type()
 	reqCount := GetProperties(st).reqCount
 	var reqFieldErr error
+	fieldSet := make(map[string]bool)
 	// A struct is a sequence of "name: value", terminated by one of
 	// '>' or '}', or the end of the input.  A name may also be
 	// "[extension]".
@@ -511,17 +512,17 @@ func (p *textParser) readStruct(sv reflect.Value, terminator string) error {
 			}
 		} else {
 			// This is a normal, non-extension field.
-			fi, props, ok := structFieldByName(st, tok.value)
+			name := tok.value
+			fi, props, ok := structFieldByName(st, name)
 			if !ok {
-				return p.errorf("unknown field name %q in %v", tok.value, st)
+				return p.errorf("unknown field name %q in %v", name, st)
 			}
 
 			dst := sv.Field(fi)
-			isDstNil := isNil(dst)
 
 			// Check that it's not already set if it's not a repeated field.
-			if !props.Repeated && !isDstNil {
-				return p.errorf("non-repeated field %q was repeated", tok.value)
+			if !props.Repeated && fieldSet[name] {
+				return p.errorf("non-repeated field %q was repeated", name)
 			}
 
 			if err := p.checkForColon(props, st.Field(fi).Type); err != nil {
@@ -529,6 +530,7 @@ func (p *textParser) readStruct(sv reflect.Value, terminator string) error {
 			}
 
 			// Parse into the field.
+			fieldSet[name] = true
 			if err := p.readAny(dst, props); err != nil {
 				if _, ok := err.(*RequiredNotSetError); !ok {
 					return err