diff --git a/types/views/views.go b/types/views/views.go index d8acf27ce..ae776c3b2 100644 --- a/types/views/views.go +++ b/types/views/views.go @@ -386,14 +386,32 @@ func SliceEqualAnyOrderFunc[T any, V comparable](a, b Slice[T], cmp func(T) V) b // do the quadratic thing. We can also only check the items between // diffStart and the end. nRemain := a.Len() - diffStart - if nRemain <= 5 { - maxLen := a.Len() // same as b.Len() - for i := diffStart; i < maxLen; i++ { - av := cmp(a.At(i)) + const shortOptLen = 5 + if nRemain <= shortOptLen { + // These track which elements in a and b have been matched, so + // that we don't treat arrays with differing number of + // duplicate elements as equal (e.g. [1, 1, 2] and [1, 2, 2]). + var aMatched, bMatched [shortOptLen]bool + + // Compare each element in a to each element in b + for i := range nRemain { + av := cmp(a.At(i + diffStart)) found := false - for j := diffStart; j < maxLen; j++ { - bv := cmp(b.At(j)) + for j := range nRemain { + // Skip elements in b that have already been + // used to match an item in a. + if bMatched[j] { + continue + } + + bv := cmp(b.At(j + diffStart)) if av == bv { + // Mark these elements as already + // matched, so that a future loop + // iteration (of a duplicate element) + // doesn't match it again. + aMatched[i] = true + bMatched[j] = true found = true break } @@ -402,6 +420,14 @@ func SliceEqualAnyOrderFunc[T any, V comparable](a, b Slice[T], cmp func(T) V) b return false } } + + // Verify all elements were matched exactly once. + for i := range nRemain { + if !aMatched[i] || !bMatched[i] { + return false + } + } + return true } diff --git a/types/views/views_test.go b/types/views/views_test.go index 70e021aa4..7837a89d6 100644 --- a/types/views/views_test.go +++ b/types/views/views_test.go @@ -197,6 +197,38 @@ func TestSliceEqualAnyOrderFunc(t *testing.T) { // Long difference; past the quadratic limit longDiff := ncFrom("b", "a", "c", "d", "e", "f", "g", "h", "i", "k") // differs at end c.Check(SliceEqualAnyOrderFunc(longSlice, longDiff, cmp), qt.Equals, false) + + // The short slice optimization had a bug where it wouldn't handle + // duplicate elements; test various cases here driven by code coverage. + shortTestCases := []struct { + name string + s1, s2 Slice[nc] + want bool + }{ + { + name: "duplicates_same_length", + s1: ncFrom("a", "a", "b"), + s2: ncFrom("a", "b", "b"), + want: false, + }, + { + name: "duplicates_different_matched", + s1: ncFrom("x", "y", "a", "a", "b"), + s2: ncFrom("x", "y", "b", "a", "a"), + want: true, + }, + { + name: "item_in_a_not_b", + s1: ncFrom("x", "y", "a", "b", "c"), + s2: ncFrom("x", "y", "b", "c", "q"), + want: false, + }, + } + for _, tc := range shortTestCases { + t.Run("short_"+tc.name, func(t *testing.T) { + c.Check(SliceEqualAnyOrderFunc(tc.s1, tc.s2, cmp), qt.Equals, tc.want) + }) + } } func TestSliceEqual(t *testing.T) {