Skip to content

Commit

Permalink
Add DetectCycles to detect cycles in compared structs
Browse files Browse the repository at this point in the history
The current comparison fails on stack overflow when given a cyclic struct
(such as a cyclic linked list for example).

This fix adds a new option: DecetctCycles, which finds cycles by adding information
to the path stack, when iterating over pointers.

When a cycle is checked (every time a pointer kind is compared) the path stack
is iterated to find cycles. The comparison of cycles is by they length.

Fixes: #74
  • Loading branch information
posener committed Mar 17, 2018
1 parent 0c93777 commit 6073507
Show file tree
Hide file tree
Showing 3 changed files with 210 additions and 2 deletions.
99 changes: 99 additions & 0 deletions cmp/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import (

var now = time.Now()

var reAddress = regexp.MustCompile(`\(0x[0-9a-f]+\)`)

func intPtr(n int) *int { return &n }

type test struct {
Expand All @@ -44,6 +46,7 @@ func TestDiff(t *testing.T) {
tests = append(tests, transformerTests()...)
tests = append(tests, embeddedTests()...)
tests = append(tests, methodTests()...)
tests = append(tests, detectCyclesTest()...)
tests = append(tests, project1Tests()...)
tests = append(tests, project2Tests()...)
tests = append(tests, project3Tests()...)
Expand All @@ -69,6 +72,10 @@ func TestDiff(t *testing.T) {
if gotPanic != "" {
t.Fatalf("unexpected panic message: %s", gotPanic)
}

// change all addresses in the diff to be 0x00, so they could be expected
gotDiff = reAddress.ReplaceAllString(gotDiff, "(0x00)")

if got, want := strings.TrimSpace(gotDiff), strings.TrimSpace(tt.wantDiff); got != want {
t.Fatalf("difference message:\ngot:\n%s\n\nwant:\n%s", got, want)
}
Expand Down Expand Up @@ -1966,6 +1973,98 @@ func project4Tests() []test {
}}
}

func detectCyclesTest() []test {
const label = "DetectCycles/"

type node struct {
Value string
Next *node
}

var a = node{Value: "a"}
a.Next = &a

var anotherA = node{Value: "a"}
anotherA.Next = &anotherA

var b = node{Value: "b"}
b.Next = &b

// a cyclic link list in length 2
var len21, len22 node
len21.Next = &len22
len22.Next = &len21

// a cyclic link list in length 3
var len31, len32, len33 node
len31.Next = &len32
len32.Next = &len33
len33.Next = &len31

var insideA1, insideA2, insideA3 node
insideA1.Next = &insideA2
insideA2.Next = &insideA3
insideA3.Next = &insideA1
insideA2.Value = "a"

var insideB1, insideB2, insideB3 node
insideB1.Next = &insideB2
insideB2.Next = &insideB3
insideB3.Next = &insideB1
insideB2.Value = "b"

return []test{{
label: label + "simple cycle/different",
x: a,
y: b,
opts: []cmp.Option{cmp.DetectCycles()},
wantDiff: `
{cmp_test.node}.Value:
-: "a"
+: "b"
{cmp_test.node}.Next{cmp_test.node}.Value:
-: "a"
+: "b"
`,
}, {
label: label + "simple cycle/equal",
x: a,
y: anotherA,
opts: []cmp.Option{cmp.DetectCycles()},
}, {
label: label + "simple cycle/equal identity",
x: a,
y: a,
opts: []cmp.Option{cmp.DetectCycles()},
}, {
label: label + "different size cycles",
x: len21,
y: len31,
opts: []cmp.Option{cmp.DetectCycles()},
wantDiff: `
{cmp_test.node}.Next{cmp_test.node}.Next{cmp_test.node}.Next:
-: &cmp_test.node{Next: &cmp_test.node{Next: (*cmp_test.node)(0x00)}}
+: &cmp_test.node{Next: &cmp_test.node{Next: &cmp_test.node{Next: (*cmp_test.node)(0x00)}}}
{cmp_test.node}.Next{cmp_test.node}.Next:
-: &cmp_test.node{Next: &cmp_test.node{Next: (*cmp_test.node)(0x00)}}
+: &cmp_test.node{Next: &cmp_test.node{Next: &cmp_test.node{Next: (*cmp_test.node)(0x00)}}}
{cmp_test.node}.Next:
-: &cmp_test.node{Next: &cmp_test.node{Next: (*cmp_test.node)(0x00)}}
+: &cmp_test.node{Next: &cmp_test.node{Next: &cmp_test.node{Next: (*cmp_test.node)(0x00)}}}
`,
}, {
label: label + "value inside an equal cycle is different",
x: insideA1,
y: insideB1,
opts: []cmp.Option{cmp.DetectCycles()},
wantDiff: `
{cmp_test.node}.Next{cmp_test.node}.Value:
-: "a"
+: "b"
`,
}}
}

// TODO: Delete this hack when we drop Go1.6 support.
func tRunParallel(t *testing.T, name string, f func(t *testing.T)) {
type runner interface {
Expand Down
105 changes: 103 additions & 2 deletions cmp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ func (opts Options) filter(s *state, vx, vy reflect.Value, t reflect.Type) (out
return ignore{} // Only ignore can short-circuit evaluation
case invalid:
out = invalid{} // Takes precedence over comparer or transformer
case *comparer, *transformer, Options:
case *comparer, *transformer, *cycles, Options:
switch out.(type) {
case nil:
out = opt
case invalid:
// Keep invalid
case *comparer, *transformer, Options:
case *comparer, *transformer, *cycles, Options:
out = Options{out, opt} // Conflicting comparers or transformers
}
}
Expand Down Expand Up @@ -451,3 +451,104 @@ func getFuncName(p uintptr) string {
}
return name
}

// DetectCycles is an option that prevents infinite searching in cyclic data structure.
// It iterates the whole path stack every time a pointer is tested to check for
// cycles thus it reduces the runtime performance.
// It finds cycles by adding information to the path stack any time a pointer is tested.
func DetectCycles() Option {
return &cycles{}
}

type cycles struct {
core
// xCycleLen and yCycleLen are the length of cycles found by the last filter operation
// if they are zero, it means that no cycles were found.
xCycleLen, yCycleLen int
}

func (c *cycles) filter(s *state, vx, vy reflect.Value, t reflect.Type) applicableOption {
// The only relevant kind to detect cycles is a pointer
if t.Kind() != reflect.Ptr {
return nil
}

// If any of the pointers is nil, let the default compare function handle it
if vx.IsNil() || vy.IsNil() {
return nil
}

// Extract the pointer addresses
xAddr := vx.Elem().UnsafeAddr()
yAddr := vy.Elem().UnsafeAddr()

// push a cyclic step to the path stack
s.curPath.push(&cycleStep{
pathStep: pathStep{t.Elem()},
xAddr: xAddr,
yAddr: yAddr,
})
defer s.curPath.pop()

// Check for a cycle
c.xCycleLen, c.yCycleLen = cyclesLen(s.curPath)
if c.xCycleLen == 0 && c.yCycleLen == 0 {
// If there was no cycle continue to compare into the pointer elements.
s.compareAny(vx.Elem(), vy.Elem())
}
return c
}

func (c *cycles) apply(s *state, vx, vy reflect.Value) {
// the cyclic returns equality if the cycle lengths are equal
s.report(c.xCycleLen == c.yCycleLen, vx, vy)
}

func (c cycles) String() string {
return "DetectCycles()"
}

// cyclesLen calculates the cyclic pointer chain length by going
// over the path stack.
// It calculates both a cyclic chain of x values and for y values.
// If no chain was found, a length 0 is returned.
func cyclesLen(p Path) (xLen int, yLen int) {
if len(p) == 0 {
return
}

// Check the current path step for the address values.
// If it is not a cycleStep, there is no point to check the
// chains lengths
curStep, ok := p[len(p)-1].(*cycleStep)
if !ok {
return
}

// Find the next occurrence in the chain path of either xAddr or yAddr
// of the curStep.
length := 0
for i := len(p) - 2; i > 0; i-- {
if cs, ok := p[i].(*cycleStep); ok {

// sinse this step is a cyclic step, increase the chain length
length += 1

// Check for the same address. We want to return the smallest cycle,
// so we update only if the current length value is 0
if xLen == 0 && cs.xAddr == curStep.xAddr {
xLen = length
}
if yLen == 0 && cs.yAddr == curStep.yAddr {
yLen = length
}

// If we found lengths for both x and y, we can return, there is no
// need to go over the whole stack.
if xLen != 0 && yLen != 0 {
return
}
}
}
return
}
8 changes: 8 additions & 0 deletions cmp/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,13 @@ type (
pathStep
trans *transformer
}
// cycleStep is a step inserted by the DetectCycle option when comparing pointers
cycleStep struct {
pathStep
// xAddr and yAddr are the addresses pointing by the compared pointers,
// pointed by vx an vy.
xAddr, yAddr uintptr
}
)

func (ps pathStep) Type() reflect.Type { return ps.typ }
Expand Down Expand Up @@ -289,6 +296,7 @@ var (
_ PathStep = structField{}
_ PathStep = indirect{}
_ PathStep = transform{}
_ PathStep = cycleStep{}
)

// isExported reports whether the identifier is exported.
Expand Down

0 comments on commit 6073507

Please sign in to comment.