Skip to content

Commit 2f2416d

Browse files
committed
making the cyclic check resistant to reused slices and maps
1 parent a80ff5b commit 2f2416d

File tree

2 files changed

+27
-2
lines changed

2 files changed

+27
-2
lines changed

diff/diff.go

+7-2
Original file line numberDiff line numberDiff line change
@@ -158,13 +158,13 @@ type visited struct {
158158
}
159159

160160
func (v *visited) add(lhs, rhs reflect.Value) error {
161-
if canAddr(lhs) {
161+
if canAddr(lhs) && !isEmptyMapOrSlice(lhs) {
162162
if inPointers(v.lhs, lhs) {
163163
return ErrCyclic
164164
}
165165
v.lhs = append(v.lhs, lhs.Pointer())
166166
}
167-
if canAddr(rhs) {
167+
if canAddr(rhs) && !isEmptyMapOrSlice(lhs) {
168168
if inPointers(v.rhs, rhs) {
169169
return ErrCyclic
170170
}
@@ -174,6 +174,11 @@ func (v *visited) add(lhs, rhs reflect.Value) error {
174174
return nil
175175
}
176176

177+
func isEmptyMapOrSlice(v reflect.Value) bool {
178+
// we don't want to include empty slices and maps in our cyclic check, since these are not problematic
179+
return (v.Kind() == reflect.Slice || v.Kind() == reflect.Map) && v.Len() == 0
180+
}
181+
177182
func inPointers(pointers []uintptr, val reflect.Value) bool {
178183
for _, lhs := range pointers {
179184
if lhs == val.Pointer() {

diff/diff_test.go

+20
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,22 @@ func TestCircular(t *testing.T) {
555555
},
556556
},
557557
}
558+
emptySlice := map[int]interface{}{
559+
0: []interface{}{},
560+
}
561+
emptySlice[1] = emptySlice[0]
562+
emptySliceNotRepeating := map[int]interface{}{
563+
0: []interface{}{},
564+
1: []interface{}{},
565+
}
566+
emptyMap := map[int]interface{}{
567+
0: map[int]interface{}{},
568+
}
569+
emptyMap[1] = emptyMap[0]
570+
emptyMapNotRepeating := map[int]interface{}{
571+
0: map[int]interface{}{},
572+
1: map[int]interface{}{},
573+
}
558574

559575
for _, test := range []struct {
560576
lhs interface{}
@@ -567,6 +583,10 @@ func TestCircular(t *testing.T) {
567583
{lhs: first, rhs: notCyclic, wantError: true},
568584
{lhs: notCyclic, rhs: first, wantError: true},
569585
{lhs: notCyclic, rhs: notCyclic},
586+
{lhs: emptySlice, rhs: emptySliceNotRepeating},
587+
{lhs: emptySliceNotRepeating, rhs: emptySlice},
588+
{lhs: emptyMap, rhs: emptyMapNotRepeating},
589+
{lhs: emptyMapNotRepeating, rhs: emptyMap},
570590
} {
571591
d, err := Diff(test.lhs, test.rhs)
572592

0 commit comments

Comments
 (0)