Browse Source

Merge pull request #3770 from skaman/fix-slice-unique

Fix `slice.unique` wrong result
Jeroen van Rijn 1 year ago
parent
commit
339bafe6ff
2 changed files with 69 additions and 5 deletions
  1. 8 4
      core/slice/slice.odin
  2. 61 1
      tests/core/slice/test_core_slice.odin

+ 8 - 4
core/slice/slice.odin

@@ -495,8 +495,10 @@ unique :: proc(s: $S/[]$T) -> S where intrinsics.type_is_comparable(T) #no_bound
 	}
 	i := 1
 	for j in 1..<len(s) {
-		if s[j] != s[j-1] && i != j {
-			s[i] = s[j]
+		if s[j] != s[j-1] {
+			if i != j {
+				s[i] = s[j]
+			}
 			i += 1
 		}
 	}
@@ -513,8 +515,10 @@ unique_proc :: proc(s: $S/[]$T, eq: proc(T, T) -> bool) -> S #no_bounds_check {
 	}
 	i := 1
 	for j in 1..<len(s) {
-		if !eq(s[j], s[j-1]) && i != j {
-			s[i] = s[j]
+		if !eq(s[j], s[j-1]) {
+			if i != j {
+				s[i] = s[j]
+			}
 			i += 1
 		}
 	}

+ 61 - 1
tests/core/slice/test_core_slice.odin

@@ -203,7 +203,7 @@ test_permutation_iterator :: proc(t: ^testing.T) {
 	permutations_counted: int
 	for slice.permute(&iter) {
 		n := 0
-		for item, index in s {
+		for item in s {
 			n *= 10
 			n += item
 		}
@@ -218,3 +218,63 @@ test_permutation_iterator :: proc(t: ^testing.T) {
 	testing.expect_value(t, len(seen), FAC_5)
 	testing.expect_value(t, permutations_counted, FAC_5)
 }
+
+// Test inputs from #3276 and #3769
+UNIQUE_TEST_VECTORS :: [][2][]int{
+	{{2,2,2},             {2}},
+	{{1,1,1,2,2,3,3,3,3}, {1,2,3}},
+	{{1,2,4,4,5},         {1,2,4,5}},
+}
+
+@test
+test_unique :: proc(t: ^testing.T) {
+	for v in UNIQUE_TEST_VECTORS {
+		assorted := v[0]
+		expected := v[1]
+
+		uniq := slice.unique(assorted)
+		testing.expectf(t, slice.equal(uniq, expected), "Expected slice.uniq(%v) == %v, got %v", v[0], v[1], uniq)
+	}
+
+	for v in UNIQUE_TEST_VECTORS {
+		assorted := v[0]
+		expected := v[1]
+
+		uniq := slice.unique_proc(assorted, proc(a, b: int) -> bool {
+			return a == b
+		})
+		testing.expectf(t, slice.equal(uniq, expected), "Expected slice.unique_proc(%v, ...) == %v, got %v", v[0], v[1], uniq)
+	}
+
+	r := rand.create(t.seed)
+	context.random_generator = rand.default_random_generator(&r)
+
+	// 10_000 random tests
+	for _ in 0..<10_000 {
+		assorted: [dynamic]i64
+		expected: [dynamic]i64
+
+		// Prime with 1 value
+		old := rand.int63()
+		append(&assorted, old)
+		append(&expected, old)
+
+		// Add 99 additional random values
+		for _ in 1..<100 {
+			new := rand.int63()
+			append(&assorted, new)
+			if old != new {
+				append(&expected, new)
+			}
+			old = new
+		}
+
+		original := slice.clone(assorted[:])
+		uniq := slice.unique(assorted[:])
+		testing.expectf(t, slice.equal(uniq, expected[:]), "Expected slice.uniq(%v) == %v, got %v", original, expected, uniq)
+
+		delete(assorted)
+		delete(original)
+		delete(expected)
+	}
+}