From 3667a8918a46017d4ea0a403b58459e36f0268f6 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 5 Sep 2024 14:11:51 +0100 Subject: [PATCH 01/14] feat: use Zac's quicksort algorithm in stdlib sorting --- noir_stdlib/src/array/check_shuffle.nr | 128 +++++++++++++++++++++ noir_stdlib/src/{array.nr => array/mod.nr} | 34 +++--- noir_stdlib/src/array/quicksort.nr | 39 +++++++ 3 files changed, 180 insertions(+), 21 deletions(-) create mode 100644 noir_stdlib/src/array/check_shuffle.nr rename noir_stdlib/src/{array.nr => array/mod.nr} (84%) create mode 100644 noir_stdlib/src/array/quicksort.nr diff --git a/noir_stdlib/src/array/check_shuffle.nr b/noir_stdlib/src/array/check_shuffle.nr new file mode 100644 index 00000000000..bd4dd28a6ad --- /dev/null +++ b/noir_stdlib/src/array/check_shuffle.nr @@ -0,0 +1,128 @@ +unconstrained fn __get_shuffle_indices( + lhs: [T; N], + rhs: [T; N] +) -> [Field; N] where T: std::cmp::Eq { + let mut shuffle_indices: [Field;N ] = [0; N]; + + let mut shuffle_mask: [bool; N] = [false; N]; + for i in 0..N { + let mut found = false; + for j in 0..N { + if ((shuffle_mask[j] == false) & (!found)) { + if (lhs[i] == rhs[j]) { + found = true; + shuffle_indices[i] = j as Field; + shuffle_mask[j] = true; + } + } + if (found) { + continue; + } + } + assert(found == true, "check_shuffle, lhs and rhs arrays do not contain equivalent values"); + } + + shuffle_indices +} + +unconstrained fn __get_index(indices: [Field; N], idx: Field) -> Field { + let mut result = 0; + for i in 0..N { + if (indices[i] == idx) { + result = i as Field; + break; + } + } + result +} + +pub(crate) fn check_shuffle(lhs: [T; N], rhs: [T; N]) where T: std::cmp::Eq { + let shuffle_indices = __get_shuffle_indices(lhs, rhs); + + for i in 0..N { + let idx = __get_index(shuffle_indices, i as Field); + assert_eq(shuffle_indices[idx], i as Field); + } + for i in 0..N { + let idx = shuffle_indices[i]; + let expected = rhs[idx]; + let result = lhs[i]; + assert_eq(expected, result); + } +} + +pub(crate) fn get_shuffle_indices(lhs: [T; N], rhs: [T; N]) -> [Field; N] where T: std::cmp::Eq { + let shuffle_indices = __get_shuffle_indices(lhs, rhs); + for i in 0..N { + let idx = __get_index(shuffle_indices, i as Field); + assert_eq(shuffle_indices[idx], i as Field); + } + for i in 0..N { + let idx = shuffle_indices[i]; + let expected = rhs[idx]; + let result = lhs[i]; + assert_eq(expected, result); + } + shuffle_indices +} + +mod test { + struct CompoundStruct { + a: bool, + b: Field, + c: u64 + } + impl std::cmp::Eq for CompoundStruct { + fn eq(self, other: Self) -> bool { + (self.a == other.a) & (self.b == other.b) & (self.c == other.c) + } + } + + use crate::check_shuffle; + #[test] + fn test_shuffle() { + let lhs: [Field; 5] = [0, 1, 2, 3, 4]; + let rhs: [Field; 5] = [2, 0, 3, 1, 4]; + check_shuffle(lhs, rhs); + } + + #[test] + fn test_shuffle_identity() { + let lhs: [Field; 5] = [0, 1, 2, 3, 4]; + let rhs: [Field; 5] = [0, 1, 2, 3, 4]; + check_shuffle(lhs, rhs); + } + + #[test(should_fail_with = "check_shuffle, lhs and rhs arrays do not contain equivalent values")] + fn test_shuffle_fail() { + let lhs: [Field; 5] = [0, 1, 2, 3, 4]; + let rhs: [Field; 5] = [0, 1, 2, 3, 5]; + check_shuffle(lhs, rhs); + } + + #[test(should_fail_with = "check_shuffle, lhs and rhs arrays do not contain equivalent values")] + fn test_shuffle_duplicates() { + let lhs: [Field; 5] = [0, 1, 2, 3, 4]; + let rhs: [Field; 5] = [0, 1, 2, 3, 3]; + check_shuffle(lhs, rhs); + } + + #[test] + fn test_shuffle_compound_struct() { + let lhs: [CompoundStruct; 5] = [ + CompoundStruct { a: false, b: 0, c: 12345 }, + CompoundStruct { a: false, b: -100, c: 54321 }, + CompoundStruct { a: true, b: 5, c: 0xffffffffffffffff }, + CompoundStruct { a: true, b: 9814, c: 0xeeffee0011001133 }, + CompoundStruct { a: false, b: 0x155, c: 0 } + ]; + let rhs: [CompoundStruct; 5] = [ + CompoundStruct { a: false, b: 0x155, c: 0 }, + CompoundStruct { a: false, b: 0, c: 12345 }, + CompoundStruct { a: false, b: -100, c: 54321 }, + CompoundStruct { a: true, b: 9814, c: 0xeeffee0011001133 }, + CompoundStruct { a: true, b: 5, c: 0xffffffffffffffff } + ]; + check_shuffle(lhs, rhs); + } +} diff --git a/noir_stdlib/src/array.nr b/noir_stdlib/src/array/mod.nr similarity index 84% rename from noir_stdlib/src/array.nr rename to noir_stdlib/src/array/mod.nr index 68e134b56fa..7824b72dc4f 100644 --- a/noir_stdlib/src/array.nr +++ b/noir_stdlib/src/array/mod.nr @@ -1,6 +1,9 @@ use crate::cmp::Ord; use crate::convert::From; +mod check_shuffle; +mod quicksort; + impl [T; N] { /// Returns the length of the slice. #[builtin(array_len)] @@ -11,29 +14,18 @@ impl [T; N] { } pub fn sort_via(self, ordering: fn[Env](T, T) -> bool) -> Self { - let sorted_index = unsafe { - // Safety: These indices are asserted to be the sorted element indices via `find_index` - let sorted_index: [u32; N] = self.get_sorting_index(ordering); - - for i in 0..N { - let pos = find_index(sorted_index, i); - assert(sorted_index[pos] == i); + unsafe { + // Safety: `sorted` array is checked to be: + // a. a permutation of `input`'s elements + // b. satisfying the predicate `ordering` + let sorted = quicksort::quicksort(input); + + for i in 0..N - 1 { + assert(ordering(sorted[i], sorted[i + 1])); } - - sorted_index - }; - - // Sort the array using the indexes - let mut result = self; - for i in 0..N { - result[i] = self[sorted_index[i]]; - } - // Ensure the array is sorted - for i in 0..N - 1 { - assert(ordering(result[i], result[i + 1])); + check_shuffle::check_shuffle(input, sorted); + sorted } - - result } /// Returns the index of the elements in the array that would sort it, using the provided custom sorting function. diff --git a/noir_stdlib/src/array/quicksort.nr b/noir_stdlib/src/array/quicksort.nr new file mode 100644 index 00000000000..9ee1bba8fb4 --- /dev/null +++ b/noir_stdlib/src/array/quicksort.nr @@ -0,0 +1,39 @@ +unconstrained fn partition(arr: &mut [T; N], low: u32, high: u32, sortfn: fn(T, T) -> bool) -> u32 { + let pivot = high; + let mut i = low; + for j in low..high { + if (sortfn(arr[j], arr[pivot])) { + let temp = arr[i]; + arr[i] = arr[j]; + arr[j] = temp; + i += 1; + } + } + let temp = arr[i]; + arr[i] = arr[pivot]; + arr[pivot] = temp; + i +} + +unconstrained fn quicksort_recursive( + arr: &mut [T; N], + low: u32, + high: u32, + sortfn: fn(T, T) -> bool +) { + if low < high { + let pivot_index = partition(arr, low, high, sortfn); + if pivot_index > 0 { + quicksort_recursive(arr, low, pivot_index - 1, sortfn); + } + quicksort_recursive(arr, pivot_index + 1, high, sortfn); + } +} + +unconstrained pub(crate) fn quicksort(_arr: [T; N], sortfn: fn(T, T) -> bool) -> [T; N] { + let mut arr: [T; N] = _arr; + if arr.len() <= 1 {} else { + quicksort_recursive(&mut arr, 0, arr.len() - 1, sortfn); + } + arr +} From 0b126994b75c363b5547d8000961b973696c6680 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 5 Sep 2024 14:13:46 +0100 Subject: [PATCH 02/14] chore: remove old helper functions --- noir_stdlib/src/array/mod.nr | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/noir_stdlib/src/array/mod.nr b/noir_stdlib/src/array/mod.nr index 7824b72dc4f..c1a6938fd17 100644 --- a/noir_stdlib/src/array/mod.nr +++ b/noir_stdlib/src/array/mod.nr @@ -28,28 +28,6 @@ impl [T; N] { } } - /// Returns the index of the elements in the array that would sort it, using the provided custom sorting function. - unconstrained fn get_sorting_index(self, ordering: fn[Env](T, T) -> bool) -> [u32; N] { - let mut result = [0; N]; - let mut a = self; - for i in 0..N { - result[i] = i; - } - for i in 1..N { - for j in 0..i { - if ordering(a[i], a[j]) { - let old_a_j = a[j]; - a[j] = a[i]; - a[i] = old_a_j; - let old_j = result[j]; - result[j] = result[i]; - result[i] = old_j; - } - } - } - result - } - #[builtin(as_slice)] pub fn as_slice(self) -> [T] {} @@ -113,18 +91,6 @@ impl [u8; N] { pub fn as_str_unchecked(self) -> str {} } -// helper function used to look up the position of a value in an array of Field -// Note that function returns 0 if the value is not found -unconstrained fn find_index(a: [u32; N], find: u32) -> u32 { - let mut result = 0; - for i in 0..a.len() { - if a[i] == find { - result = i; - } - } - result -} - impl From> for [u8; N] { fn from(s: str) -> Self { s.as_bytes() From f3dcdeaf460d87c320a7ffc718c33f4a2bdcde60 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 5 Sep 2024 14:20:00 +0100 Subject: [PATCH 03/14] . --- noir_stdlib/src/array/check_shuffle.nr | 46 ++++++++++++++------------ 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/noir_stdlib/src/array/check_shuffle.nr b/noir_stdlib/src/array/check_shuffle.nr index bd4dd28a6ad..967590387e8 100644 --- a/noir_stdlib/src/array/check_shuffle.nr +++ b/noir_stdlib/src/array/check_shuffle.nr @@ -37,33 +37,37 @@ unconstrained fn __get_index(indices: [Field; N], idx: Field) -> Fie } pub(crate) fn check_shuffle(lhs: [T; N], rhs: [T; N]) where T: std::cmp::Eq { - let shuffle_indices = __get_shuffle_indices(lhs, rhs); + unsafe { + let shuffle_indices = __get_shuffle_indices(lhs, rhs); - for i in 0..N { - let idx = __get_index(shuffle_indices, i as Field); - assert_eq(shuffle_indices[idx], i as Field); - } - for i in 0..N { - let idx = shuffle_indices[i]; - let expected = rhs[idx]; - let result = lhs[i]; - assert_eq(expected, result); + for i in 0..N { + let idx = __get_index(shuffle_indices, i as Field); + assert_eq(shuffle_indices[idx], i as Field); + } + for i in 0..N { + let idx = shuffle_indices[i]; + let expected = rhs[idx]; + let result = lhs[i]; + assert_eq(expected, result); + } } } pub(crate) fn get_shuffle_indices(lhs: [T; N], rhs: [T; N]) -> [Field; N] where T: std::cmp::Eq { - let shuffle_indices = __get_shuffle_indices(lhs, rhs); - for i in 0..N { - let idx = __get_index(shuffle_indices, i as Field); - assert_eq(shuffle_indices[idx], i as Field); - } - for i in 0..N { - let idx = shuffle_indices[i]; - let expected = rhs[idx]; - let result = lhs[i]; - assert_eq(expected, result); + unsafe { + let shuffle_indices = __get_shuffle_indices(lhs, rhs); + for i in 0..N { + let idx = __get_index(shuffle_indices, i as Field); + assert_eq(shuffle_indices[idx], i as Field); + } + for i in 0..N { + let idx = shuffle_indices[i]; + let expected = rhs[idx]; + let result = lhs[i]; + assert_eq(expected, result); + } + shuffle_indices } - shuffle_indices } mod test { From 3a6fac88dba6e727ca7c3d3204e2a182ea6fadae Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 5 Sep 2024 14:25:45 +0100 Subject: [PATCH 04/14] . --- noir_stdlib/src/array/mod.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noir_stdlib/src/array/mod.nr b/noir_stdlib/src/array/mod.nr index c1a6938fd17..cd1050231c4 100644 --- a/noir_stdlib/src/array/mod.nr +++ b/noir_stdlib/src/array/mod.nr @@ -18,12 +18,12 @@ impl [T; N] { // Safety: `sorted` array is checked to be: // a. a permutation of `input`'s elements // b. satisfying the predicate `ordering` - let sorted = quicksort::quicksort(input); + let sorted = quicksort::quicksort(self); for i in 0..N - 1 { assert(ordering(sorted[i], sorted[i + 1])); } - check_shuffle::check_shuffle(input, sorted); + check_shuffle::check_shuffle(self, sorted); sorted } } From 14b576002f0d9cb37808c6956e58460708e3baf8 Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 6 Sep 2024 13:19:31 +0100 Subject: [PATCH 05/14] . --- noir_stdlib/src/array/check_shuffle.nr | 32 +++++-------------- noir_stdlib/src/array/mod.nr | 43 ++++++++++++++------------ noir_stdlib/src/array/quicksort.nr | 16 +++++----- 3 files changed, 39 insertions(+), 52 deletions(-) diff --git a/noir_stdlib/src/array/check_shuffle.nr b/noir_stdlib/src/array/check_shuffle.nr index 967590387e8..26f5ced3467 100644 --- a/noir_stdlib/src/array/check_shuffle.nr +++ b/noir_stdlib/src/array/check_shuffle.nr @@ -1,7 +1,6 @@ -unconstrained fn __get_shuffle_indices( - lhs: [T; N], - rhs: [T; N] -) -> [Field; N] where T: std::cmp::Eq { +use crate::cmp::Eq; + +unconstrained fn __get_shuffle_indices(lhs: [T; N], rhs: [T; N]) -> [Field; N] where T: Eq { let mut shuffle_indices: [Field;N ] = [0; N]; let mut shuffle_mask: [bool; N] = [false; N]; @@ -36,7 +35,7 @@ unconstrained fn __get_index(indices: [Field; N], idx: Field) -> Fie result } -pub(crate) fn check_shuffle(lhs: [T; N], rhs: [T; N]) where T: std::cmp::Eq { +pub(crate) fn check_shuffle(lhs: [T; N], rhs: [T; N]) where T: Eq { unsafe { let shuffle_indices = __get_shuffle_indices(lhs, rhs); @@ -53,36 +52,21 @@ pub(crate) fn check_shuffle(lhs: [T; N], rhs: [T; N]) where T: st } } -pub(crate) fn get_shuffle_indices(lhs: [T; N], rhs: [T; N]) -> [Field; N] where T: std::cmp::Eq { - unsafe { - let shuffle_indices = __get_shuffle_indices(lhs, rhs); - for i in 0..N { - let idx = __get_index(shuffle_indices, i as Field); - assert_eq(shuffle_indices[idx], i as Field); - } - for i in 0..N { - let idx = shuffle_indices[i]; - let expected = rhs[idx]; - let result = lhs[i]; - assert_eq(expected, result); - } - shuffle_indices - } -} - mod test { + use super::check_shuffle; + use crate::cmp::Eq; + struct CompoundStruct { a: bool, b: Field, c: u64 } - impl std::cmp::Eq for CompoundStruct { + impl Eq for CompoundStruct { fn eq(self, other: Self) -> bool { (self.a == other.a) & (self.b == other.b) & (self.c == other.c) } } - use crate::check_shuffle; #[test] fn test_shuffle() { let lhs: [Field; 5] = [0, 1, 2, 3, 4]; diff --git a/noir_stdlib/src/array/mod.nr b/noir_stdlib/src/array/mod.nr index cd1050231c4..0b22b5170cf 100644 --- a/noir_stdlib/src/array/mod.nr +++ b/noir_stdlib/src/array/mod.nr @@ -1,4 +1,4 @@ -use crate::cmp::Ord; +use crate::cmp::{Eq, Ord}; use crate::convert::From; mod check_shuffle; @@ -9,25 +9,6 @@ impl [T; N] { #[builtin(array_len)] pub fn len(self) -> u32 {} - pub fn sort(self) -> Self where T: Ord { - self.sort_via(|a: T, b: T| a <= b) - } - - pub fn sort_via(self, ordering: fn[Env](T, T) -> bool) -> Self { - unsafe { - // Safety: `sorted` array is checked to be: - // a. a permutation of `input`'s elements - // b. satisfying the predicate `ordering` - let sorted = quicksort::quicksort(self); - - for i in 0..N - 1 { - assert(ordering(sorted[i], sorted[i + 1])); - } - check_shuffle::check_shuffle(self, sorted); - sorted - } - } - #[builtin(as_slice)] pub fn as_slice(self) -> [T] {} @@ -84,6 +65,28 @@ impl [T; N] { } } +impl [T; N] where T: Ord + Eq { + + pub fn sort(self) -> Self { + self.sort_via(|a: T, b: T| a <= b) + } + + pub fn sort_via(self, ordering: fn[Env](T, T) -> bool) -> Self { + unsafe { + // Safety: `sorted` array is checked to be: + // a. a permutation of `input`'s elements + // b. satisfying the predicate `ordering` + let sorted = quicksort::quicksort(self, ordering); + + for i in 0..N - 1 { + assert(ordering(sorted[i], sorted[i + 1])); + } + check_shuffle::check_shuffle(self, sorted); + sorted + } + } +} + impl [u8; N] { /// Convert a sequence of bytes as-is into a string. /// This function performs no UTF-8 validation or similar. diff --git a/noir_stdlib/src/array/quicksort.nr b/noir_stdlib/src/array/quicksort.nr index 9ee1bba8fb4..6a54ed246f5 100644 --- a/noir_stdlib/src/array/quicksort.nr +++ b/noir_stdlib/src/array/quicksort.nr @@ -1,4 +1,9 @@ -unconstrained fn partition(arr: &mut [T; N], low: u32, high: u32, sortfn: fn(T, T) -> bool) -> u32 { +unconstrained fn partition( + arr: &mut [T; N], + low: u32, + high: u32, + sortfn: fn[Env](T, T) -> bool +) -> u32 { let pivot = high; let mut i = low; for j in low..high { @@ -15,12 +20,7 @@ unconstrained fn partition(arr: &mut [T; N], low: u32, high: u32, i } -unconstrained fn quicksort_recursive( - arr: &mut [T; N], - low: u32, - high: u32, - sortfn: fn(T, T) -> bool -) { +unconstrained fn quicksort_recursive(arr: &mut [T; N], low: u32, high: u32, sortfn: fn[Env](T, T) -> bool) { if low < high { let pivot_index = partition(arr, low, high, sortfn); if pivot_index > 0 { @@ -30,7 +30,7 @@ unconstrained fn quicksort_recursive( } } -unconstrained pub(crate) fn quicksort(_arr: [T; N], sortfn: fn(T, T) -> bool) -> [T; N] { +unconstrained pub(crate) fn quicksort(_arr: [T; N], sortfn: fn[Env](T, T) -> bool) -> [T; N] { let mut arr: [T; N] = _arr; if arr.len() <= 1 {} else { quicksort_recursive(&mut arr, 0, arr.len() - 1, sortfn); From e1e1b047e2e2ebc710ca9d2aee0218a803ccc1cc Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 6 Sep 2024 13:44:20 +0100 Subject: [PATCH 06/14] . --- noir_stdlib/src/array/mod.nr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/noir_stdlib/src/array/mod.nr b/noir_stdlib/src/array/mod.nr index 0b22b5170cf..0131ddf32a4 100644 --- a/noir_stdlib/src/array/mod.nr +++ b/noir_stdlib/src/array/mod.nr @@ -66,10 +66,12 @@ impl [T; N] { } impl [T; N] where T: Ord + Eq { - pub fn sort(self) -> Self { self.sort_via(|a: T, b: T| a <= b) } +} + +impl [T; N] where T: Eq { pub fn sort_via(self, ordering: fn[Env](T, T) -> bool) -> Self { unsafe { From 6d7fcc03489d5e5ac5c8c54db00a103158233832 Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 6 Sep 2024 13:56:32 +0100 Subject: [PATCH 07/14] . --- noir_stdlib/src/array/mod.nr | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/noir_stdlib/src/array/mod.nr b/noir_stdlib/src/array/mod.nr index 0131ddf32a4..e57abd621c8 100644 --- a/noir_stdlib/src/array/mod.nr +++ b/noir_stdlib/src/array/mod.nr @@ -1,5 +1,6 @@ use crate::cmp::{Eq, Ord}; use crate::convert::From; +use std::runtime::is_unconstrained; mod check_shuffle; mod quicksort; @@ -80,10 +81,12 @@ impl [T; N] where T: Eq { // b. satisfying the predicate `ordering` let sorted = quicksort::quicksort(self, ordering); - for i in 0..N - 1 { - assert(ordering(sorted[i], sorted[i + 1])); + if !is_unconstrained() { + for i in 0..N - 1 { + assert(ordering(sorted[i], sorted[i + 1])); + } + check_shuffle::check_shuffle(self, sorted); } - check_shuffle::check_shuffle(self, sorted); sorted } } From d3f4357cdee1976352226c436f8902aa183bd954 Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 6 Sep 2024 13:59:50 +0100 Subject: [PATCH 08/14] . --- noir_stdlib/src/array/mod.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir_stdlib/src/array/mod.nr b/noir_stdlib/src/array/mod.nr index e57abd621c8..3b94de2c586 100644 --- a/noir_stdlib/src/array/mod.nr +++ b/noir_stdlib/src/array/mod.nr @@ -1,6 +1,6 @@ use crate::cmp::{Eq, Ord}; use crate::convert::From; -use std::runtime::is_unconstrained; +use crate::runtime::is_unconstrained; mod check_shuffle; mod quicksort; From 194fbdd5430150e82bcf5f66f1ebccec5ff10b7e Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 9 Sep 2024 13:06:27 +0000 Subject: [PATCH 09/14] chore: add doc comment explaining requirements on `ordering`. --- noir_stdlib/src/array/mod.nr | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/noir_stdlib/src/array/mod.nr b/noir_stdlib/src/array/mod.nr index 3b94de2c586..1f95ed530f6 100644 --- a/noir_stdlib/src/array/mod.nr +++ b/noir_stdlib/src/array/mod.nr @@ -74,6 +74,11 @@ impl [T; N] where T: Ord + Eq { impl [T; N] where T: Eq { + /// Sorts the array using a custom predicate function `ordering`. + /// + /// # Safety + /// The `ordering` function must be designed to return `true` for equal valued inputs + /// If this is not done, `sort_via` will fail to sort inputs with duplicated elements. pub fn sort_via(self, ordering: fn[Env](T, T) -> bool) -> Self { unsafe { // Safety: `sorted` array is checked to be: @@ -83,7 +88,7 @@ impl [T; N] where T: Eq { if !is_unconstrained() { for i in 0..N - 1 { - assert(ordering(sorted[i], sorted[i + 1])); + assert(ordering(sorted[i], sorted[i + 1]), "Array has not been sorted correctly according to `ordering`."); } check_shuffle::check_shuffle(self, sorted); } From a1b883e2f60475102df3f175fc9a067601277f9f Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 9 Sep 2024 13:08:49 +0000 Subject: [PATCH 10/14] chore: update docs --- docs/docs/noir/concepts/data_types/arrays.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/noir/concepts/data_types/arrays.md b/docs/docs/noir/concepts/data_types/arrays.md index e5e9f5a1d3b..0cde06717f8 100644 --- a/docs/docs/noir/concepts/data_types/arrays.md +++ b/docs/docs/noir/concepts/data_types/arrays.md @@ -128,7 +128,7 @@ fn main() { ### sort_via -Sorts the array with a custom comparison function +Sorts the array with a custom comparison function. The ordering function must return true if the first argument should be sorted to be before the second argument, otherwise it should return false. ```rust fn sort_via(self, ordering: fn(T, T) -> bool) -> [T; N] From fdae9e716c3434d579ebb77f765f61b3e824d928 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 9 Sep 2024 14:09:43 +0100 Subject: [PATCH 11/14] Update noir_stdlib/src/array/mod.nr --- noir_stdlib/src/array/mod.nr | 1 - 1 file changed, 1 deletion(-) diff --git a/noir_stdlib/src/array/mod.nr b/noir_stdlib/src/array/mod.nr index 1f95ed530f6..fb6d707e86a 100644 --- a/noir_stdlib/src/array/mod.nr +++ b/noir_stdlib/src/array/mod.nr @@ -76,7 +76,6 @@ impl [T; N] where T: Eq { /// Sorts the array using a custom predicate function `ordering`. /// - /// # Safety /// The `ordering` function must be designed to return `true` for equal valued inputs /// If this is not done, `sort_via` will fail to sort inputs with duplicated elements. pub fn sort_via(self, ordering: fn[Env](T, T) -> bool) -> Self { From ec7859b541f1b71cef086120f1f6f12d0d89681f Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 9 Sep 2024 13:30:59 +0000 Subject: [PATCH 12/14] . --- noir_stdlib/src/array/mod.nr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/noir_stdlib/src/array/mod.nr b/noir_stdlib/src/array/mod.nr index fb6d707e86a..f3cf6b78081 100644 --- a/noir_stdlib/src/array/mod.nr +++ b/noir_stdlib/src/array/mod.nr @@ -87,7 +87,9 @@ impl [T; N] where T: Eq { if !is_unconstrained() { for i in 0..N - 1 { - assert(ordering(sorted[i], sorted[i + 1]), "Array has not been sorted correctly according to `ordering`."); + assert( + ordering(sorted[i], sorted[i + 1]), "Array has not been sorted correctly according to `ordering`." + ); } check_shuffle::check_shuffle(self, sorted); } From 44d2cafb566c495fb06ec775c08c8a9d39879ba3 Mon Sep 17 00:00:00 2001 From: jfecher Date: Tue, 10 Sep 2024 09:36:32 -0500 Subject: [PATCH 13/14] Update docs/docs/noir/concepts/data_types/arrays.md --- docs/docs/noir/concepts/data_types/arrays.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/docs/noir/concepts/data_types/arrays.md b/docs/docs/noir/concepts/data_types/arrays.md index 0cde06717f8..c08eeb1501a 100644 --- a/docs/docs/noir/concepts/data_types/arrays.md +++ b/docs/docs/noir/concepts/data_types/arrays.md @@ -128,7 +128,9 @@ fn main() { ### sort_via -Sorts the array with a custom comparison function. The ordering function must return true if the first argument should be sorted to be before the second argument, otherwise it should return false. +Sorts the array with a custom comparison function. The ordering function must return true if the first argument should be sorted to be before the second argument or is equal to the second argument. + +Using this method with an operator like `<` that does not return `true` for equal values will result in an assertion failure for arrays with equal elements. ```rust fn sort_via(self, ordering: fn(T, T) -> bool) -> [T; N] From a32912389ac5137b9cebec930cb632e83145f564 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 11 Sep 2024 12:19:01 +0100 Subject: [PATCH 14/14] chore: replace strict equalities in documentation of `sort_via` --- docs/docs/noir/concepts/data_types/arrays.md | 4 ++-- .../version-v0.17.0/language_concepts/data_types/04_arrays.md | 4 ++-- .../version-v0.19.0/language_concepts/data_types/04_arrays.md | 4 ++-- .../version-v0.19.1/language_concepts/data_types/04_arrays.md | 4 ++-- .../version-v0.19.2/language_concepts/data_types/04_arrays.md | 4 ++-- .../version-v0.19.3/language_concepts/data_types/04_arrays.md | 4 ++-- .../version-v0.19.4/language_concepts/data_types/04_arrays.md | 4 ++-- .../version-v0.22.0/noir/syntax/data_types/arrays.md | 4 ++-- .../version-v0.23.0/noir/concepts/data_types/arrays.md | 4 ++-- .../version-v0.24.0/noir/concepts/data_types/arrays.md | 4 ++-- .../version-v0.25.0/noir/concepts/data_types/arrays.md | 4 ++-- .../version-v0.26.0/noir/concepts/data_types/arrays.md | 4 ++-- .../version-v0.27.0/noir/concepts/data_types/arrays.md | 4 ++-- .../version-v0.28.0/noir/concepts/data_types/arrays.md | 4 ++-- .../version-v0.29.0/noir/concepts/data_types/arrays.md | 4 ++-- .../version-v0.30.0/noir/concepts/data_types/arrays.md | 4 ++-- .../version-v0.31.0/noir/concepts/data_types/arrays.md | 4 ++-- .../version-v0.32.0/noir/concepts/data_types/arrays.md | 4 ++-- .../version-v0.33.0/noir/concepts/data_types/arrays.md | 4 ++-- .../execution_success/higher_order_functions/src/main.nr | 2 +- 20 files changed, 39 insertions(+), 39 deletions(-) diff --git a/docs/docs/noir/concepts/data_types/arrays.md b/docs/docs/noir/concepts/data_types/arrays.md index c08eeb1501a..d0d8eb70aa6 100644 --- a/docs/docs/noir/concepts/data_types/arrays.md +++ b/docs/docs/noir/concepts/data_types/arrays.md @@ -141,10 +141,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.17.0/language_concepts/data_types/04_arrays.md b/docs/versioned_docs/version-v0.17.0/language_concepts/data_types/04_arrays.md index 1424ca2df14..3c84da3f8ad 100644 --- a/docs/versioned_docs/version-v0.17.0/language_concepts/data_types/04_arrays.md +++ b/docs/versioned_docs/version-v0.17.0/language_concepts/data_types/04_arrays.md @@ -130,10 +130,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.19.0/language_concepts/data_types/04_arrays.md b/docs/versioned_docs/version-v0.19.0/language_concepts/data_types/04_arrays.md index 1424ca2df14..3c84da3f8ad 100644 --- a/docs/versioned_docs/version-v0.19.0/language_concepts/data_types/04_arrays.md +++ b/docs/versioned_docs/version-v0.19.0/language_concepts/data_types/04_arrays.md @@ -130,10 +130,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.19.1/language_concepts/data_types/04_arrays.md b/docs/versioned_docs/version-v0.19.1/language_concepts/data_types/04_arrays.md index 1424ca2df14..3c84da3f8ad 100644 --- a/docs/versioned_docs/version-v0.19.1/language_concepts/data_types/04_arrays.md +++ b/docs/versioned_docs/version-v0.19.1/language_concepts/data_types/04_arrays.md @@ -130,10 +130,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.19.2/language_concepts/data_types/04_arrays.md b/docs/versioned_docs/version-v0.19.2/language_concepts/data_types/04_arrays.md index 1424ca2df14..3c84da3f8ad 100644 --- a/docs/versioned_docs/version-v0.19.2/language_concepts/data_types/04_arrays.md +++ b/docs/versioned_docs/version-v0.19.2/language_concepts/data_types/04_arrays.md @@ -130,10 +130,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.19.3/language_concepts/data_types/04_arrays.md b/docs/versioned_docs/version-v0.19.3/language_concepts/data_types/04_arrays.md index 1424ca2df14..3c84da3f8ad 100644 --- a/docs/versioned_docs/version-v0.19.3/language_concepts/data_types/04_arrays.md +++ b/docs/versioned_docs/version-v0.19.3/language_concepts/data_types/04_arrays.md @@ -130,10 +130,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.19.4/language_concepts/data_types/04_arrays.md b/docs/versioned_docs/version-v0.19.4/language_concepts/data_types/04_arrays.md index 5b4a544cf37..36410468ec1 100644 --- a/docs/versioned_docs/version-v0.19.4/language_concepts/data_types/04_arrays.md +++ b/docs/versioned_docs/version-v0.19.4/language_concepts/data_types/04_arrays.md @@ -130,10 +130,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.22.0/noir/syntax/data_types/arrays.md b/docs/versioned_docs/version-v0.22.0/noir/syntax/data_types/arrays.md index 4c80d50ed01..addda1da8bc 100644 --- a/docs/versioned_docs/version-v0.22.0/noir/syntax/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.22.0/noir/syntax/data_types/arrays.md @@ -131,10 +131,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.23.0/noir/concepts/data_types/arrays.md b/docs/versioned_docs/version-v0.23.0/noir/concepts/data_types/arrays.md index d95346454a9..fb81dcc3970 100644 --- a/docs/versioned_docs/version-v0.23.0/noir/concepts/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.23.0/noir/concepts/data_types/arrays.md @@ -137,10 +137,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.24.0/noir/concepts/data_types/arrays.md b/docs/versioned_docs/version-v0.24.0/noir/concepts/data_types/arrays.md index ca54d82b26b..9a4c485feb1 100644 --- a/docs/versioned_docs/version-v0.24.0/noir/concepts/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.24.0/noir/concepts/data_types/arrays.md @@ -139,10 +139,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.25.0/noir/concepts/data_types/arrays.md b/docs/versioned_docs/version-v0.25.0/noir/concepts/data_types/arrays.md index ca54d82b26b..9a4c485feb1 100644 --- a/docs/versioned_docs/version-v0.25.0/noir/concepts/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.25.0/noir/concepts/data_types/arrays.md @@ -139,10 +139,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.26.0/noir/concepts/data_types/arrays.md b/docs/versioned_docs/version-v0.26.0/noir/concepts/data_types/arrays.md index 95d749053e2..9b02d52e8a8 100644 --- a/docs/versioned_docs/version-v0.26.0/noir/concepts/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.26.0/noir/concepts/data_types/arrays.md @@ -139,10 +139,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.27.0/noir/concepts/data_types/arrays.md b/docs/versioned_docs/version-v0.27.0/noir/concepts/data_types/arrays.md index 95d749053e2..9b02d52e8a8 100644 --- a/docs/versioned_docs/version-v0.27.0/noir/concepts/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.27.0/noir/concepts/data_types/arrays.md @@ -139,10 +139,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.28.0/noir/concepts/data_types/arrays.md b/docs/versioned_docs/version-v0.28.0/noir/concepts/data_types/arrays.md index 95d749053e2..9b02d52e8a8 100644 --- a/docs/versioned_docs/version-v0.28.0/noir/concepts/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.28.0/noir/concepts/data_types/arrays.md @@ -139,10 +139,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.29.0/noir/concepts/data_types/arrays.md b/docs/versioned_docs/version-v0.29.0/noir/concepts/data_types/arrays.md index 95d749053e2..9b02d52e8a8 100644 --- a/docs/versioned_docs/version-v0.29.0/noir/concepts/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.29.0/noir/concepts/data_types/arrays.md @@ -139,10 +139,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.30.0/noir/concepts/data_types/arrays.md b/docs/versioned_docs/version-v0.30.0/noir/concepts/data_types/arrays.md index 95d749053e2..9b02d52e8a8 100644 --- a/docs/versioned_docs/version-v0.30.0/noir/concepts/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.30.0/noir/concepts/data_types/arrays.md @@ -139,10 +139,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.31.0/noir/concepts/data_types/arrays.md b/docs/versioned_docs/version-v0.31.0/noir/concepts/data_types/arrays.md index 95d749053e2..9b02d52e8a8 100644 --- a/docs/versioned_docs/version-v0.31.0/noir/concepts/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.31.0/noir/concepts/data_types/arrays.md @@ -139,10 +139,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.32.0/noir/concepts/data_types/arrays.md b/docs/versioned_docs/version-v0.32.0/noir/concepts/data_types/arrays.md index 9a4ab5d3c1f..d26f6dff070 100644 --- a/docs/versioned_docs/version-v0.32.0/noir/concepts/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.32.0/noir/concepts/data_types/arrays.md @@ -139,10 +139,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/docs/versioned_docs/version-v0.33.0/noir/concepts/data_types/arrays.md b/docs/versioned_docs/version-v0.33.0/noir/concepts/data_types/arrays.md index 9a4ab5d3c1f..d26f6dff070 100644 --- a/docs/versioned_docs/version-v0.33.0/noir/concepts/data_types/arrays.md +++ b/docs/versioned_docs/version-v0.33.0/noir/concepts/data_types/arrays.md @@ -139,10 +139,10 @@ example ```rust fn main() { let arr = [42, 32] - let sorted_ascending = arr.sort_via(|a, b| a < b); + let sorted_ascending = arr.sort_via(|a, b| a <= b); assert(sorted_ascending == [32, 42]); // verifies - let sorted_descending = arr.sort_via(|a, b| a > b); + let sorted_descending = arr.sort_via(|a, b| a >= b); assert(sorted_descending == [32, 42]); // does not verify } ``` diff --git a/test_programs/execution_success/higher_order_functions/src/main.nr b/test_programs/execution_success/higher_order_functions/src/main.nr index 6583f961d58..0a498e74ad1 100644 --- a/test_programs/execution_success/higher_order_functions/src/main.nr +++ b/test_programs/execution_success/higher_order_functions/src/main.nr @@ -63,7 +63,7 @@ fn test_array_functions() { // // opened #2121 for it // https://github.com/noir-lang/noir/issues/2121 - // let descending = myarray.sort_via(|a, b| a > b); + // let descending = myarray.sort_via(|a, b| a >= b); // assert(descending == [3, 2, 1]); assert(evens.map(|n| n / 2) == myarray); assert(evens.map(|n| n / two) == myarray);