Skip to content

Commit

Permalink
[merge] Fix merge of non-nullable slices (gogo#569)
Browse files Browse the repository at this point in the history
  • Loading branch information
euroelessar authored and jmarais committed Jul 30, 2019
1 parent f6077ae commit 28a6bbf
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 6 deletions.
19 changes: 19 additions & 0 deletions proto/table_merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,25 @@ func (mi *mergeInfo) computeMergeInfo() {
}
case reflect.Struct:
switch {
case isSlice && !isPointer: // E.g. []pb.T
mergeInfo := getMergeInfo(tf)
zero := reflect.Zero(tf)
mfi.merge = func(dst, src pointer) {
// TODO: Make this faster?
dstsp := dst.asPointerTo(f.Type)
dsts := dstsp.Elem()
srcs := src.asPointerTo(f.Type).Elem()
for i := 0; i < srcs.Len(); i++ {
dsts = reflect.Append(dsts, zero)
srcElement := srcs.Index(i).Addr()
dstElement := dsts.Index(dsts.Len() - 1).Addr()
mergeInfo.merge(valToPointer(dstElement), valToPointer(srcElement))
}
if dsts.IsNil() {
dsts = reflect.MakeSlice(f.Type, 0, 0)
}
dstsp.Elem().Set(dsts)
}
case !isPointer:
mergeInfo := getMergeInfo(tf)
mfi.merge = func(dst, src pointer) {
Expand Down
58 changes: 53 additions & 5 deletions test/merge/merge.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion test/merge/merge.proto
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@ import "github.com/gogo/protobuf/gogoproto/gogo.proto";

message A {
B B = 1 [(gogoproto.nullable) = false];
repeated C c = 2 [(gogoproto.nullable) = false];
}

message B {
int64 c = 1;
}
}

message C {
int64 d = 1;
}
32 changes: 32 additions & 0 deletions test/merge/merge_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package merge

import (
"fmt"
"reflect"
"testing"

"github.com/gogo/protobuf/proto"
Expand All @@ -11,6 +13,17 @@ func TestClone1(t *testing.T) {
proto.Clone(f1)
}

func TestClone2(t *testing.T) {
f1 := &A{C: []C{{D: 1}, {D: 2}}}
f2 := proto.Clone(f1).(*A)
if !reflect.DeepEqual(f1.C, f2.C) {
t.Fatalf("got %v want %v", f2, f1)
}
if fmt.Sprintf("%p", f1.C) == fmt.Sprintf("%p", f2.C) {
t.Fatalf("slice is not deep copied")
}
}

func TestMerge1(t *testing.T) {
f1 := &A{}
f2 := &A{}
Expand All @@ -34,3 +47,22 @@ func TestMerge3(t *testing.T) {
t.Fatalf("got %d want %d", f1.B.C, 1)
}
}

func TestMerge4(t *testing.T) {
f1 := &A{}
f2 := &A{C: []C{}}
proto.Merge(f1, f2)
if f1.C == nil {
t.Fatalf("got %v want %v", f1, []C{})
}
}

func TestMerge5(t *testing.T) {
f1 := &A{C: []C{{D: 1}, {D: 2}}}
f2 := &A{C: []C{{D: 3}, {D: 4}}}
f3 := &A{C: []C{{D: 1}, {D: 2}, {D: 3}, {D: 4}}}
proto.Merge(f1, f2)
if !reflect.DeepEqual(f1, f3) {
t.Fatalf("got %v want %v", f1, f3)
}
}

0 comments on commit 28a6bbf

Please sign in to comment.