diff --git a/docs/docs/noir/standard_library/containers/boundedvec.md b/docs/docs/noir/standard_library/containers/boundedvec.md index ccce62562f8..c476c877d87 100644 --- a/docs/docs/noir/standard_library/containers/boundedvec.md +++ b/docs/docs/noir/standard_library/containers/boundedvec.md @@ -59,7 +59,7 @@ but for now make sure to use type annotations when using bounded vectors. Otherw ### get ```rust -pub fn get(mut self: Self, index: u64) -> T { +pub fn get(self, index: u64) -> T { ``` Retrieves an element from the vector at the given index, starting from zero. @@ -80,7 +80,7 @@ fn foo(v: BoundedVec) { ### get_unchecked ```rust -pub fn get_unchecked(mut self: Self, index: u64) -> T { +pub fn get_unchecked(self, index: u64) -> T { ``` Retrieves an element from the vector at the given index, starting from zero, without @@ -93,6 +93,42 @@ Example: #include_code get_unchecked_example test_programs/noir_test_success/bounded_vec/src/main.nr rust +### set + +```rust +pub fn set(&mut self: Self, index: u64, value: T) { +``` + +Writes an element to the vector at the given index, starting from zero. + +If the given index is equal to or greater than the length of the vector, this will issue a constraint failure. + +Example: + +```rust +fn foo(v: BoundedVec) { + let first = v.get(0); + assert(first != 42); + v.set(0, 42); + let new_first = v.get(0); + assert(new_first == 42); +} +``` + +### set_unchecked + +```rust +pub fn set_unchecked(&mut self: Self, index: u64, value: T) -> T { +``` + +Writes an element to the vector at the given index, starting from zero, without performing a bounds check. + +Since this function does not perform a bounds check on length before accessing the element, it is unsafe! Use at your own risk! + +Example: + +#include_code set_unchecked_example test_programs/noir_test_success/bounded_vec/src/main.nr rust + ### push diff --git a/docs/docs/noir/standard_library/containers/vec.mdx b/docs/docs/noir/standard_library/containers/vec.mdx index fcfd7e07aa0..475011922f8 100644 --- a/docs/docs/noir/standard_library/containers/vec.mdx +++ b/docs/docs/noir/standard_library/containers/vec.mdx @@ -84,6 +84,25 @@ let vector: Vec = Vec::from_slice(&[10, 20, 30]); assert(vector.get(1) == 20); ``` +### set + +```rust +pub fn set(&mut self: Self, index: u64, value: T) { +``` + +Writes an element to the vector at the given index, starting from zero. + +Panics if the index points beyond the vector's end. + +Example: + +```rust +let vector: Vec = Vec::from_slice(&[10, 20, 30]); +assert(vector.get(1) == 20); +vector.set(1, 42); +assert(vector.get(1) == 42); +``` + ### push Adds a new element to the vector's end, returning a new vector with a length one greater than the original unmodified vector. diff --git a/noir_stdlib/src/collections/bounded_vec.nr b/noir_stdlib/src/collections/bounded_vec.nr index aae96e5943d..801fe3da75a 100644 --- a/noir_stdlib/src/collections/bounded_vec.nr +++ b/noir_stdlib/src/collections/bounded_vec.nr @@ -11,15 +11,35 @@ impl BoundedVec { BoundedVec { storage: [zeroed; MaxLen], len: 0 } } - pub fn get(mut self: Self, index: u32) -> T { + /// Get an element from the vector at the given index. + /// Panics if the given index points beyond the end of the vector (`self.len()`). + pub fn get(self, index: u32) -> T { assert(index < self.len); - self.storage[index] + self.get_unchecked(index) } - pub fn get_unchecked(mut self: Self, index: u32) -> T { + /// Get an element from the vector at the given index. + /// Responds with undefined data for `index` where `self.len < index < self.max_len()`. + pub fn get_unchecked(self, index: u32) -> T { self.storage[index] } + /// Write an element to the vector at the given index. + /// Panics if the given index points beyond the end of the vector (`self.len()`). + pub fn set(&mut self, index: u32, value: T) { + assert(index < self.len, "Attempted to write past end of BoundedVec"); + self.set_unchecked(index, value) + } + + /// Write an element to the vector at the given index. + /// Does not check whether the passed `index` is a valid index within the vector. + /// + /// Silently writes past the end of the vector for `index` where `self.len < index < self.max_len()` + /// Panics if the given index points beyond the maximum length of the vector (`self.max_len()`). + pub fn set_unchecked(&mut self, index: u32, value: T) { + self.storage[index] = value; + } + pub fn push(&mut self, elem: T) { assert(self.len < MaxLen, "push out of bounds"); @@ -142,6 +162,39 @@ mod bounded_vec_tests { assert(bounded_vec1 != bounded_vec2); } + mod set { + use crate::collections::bounded_vec::BoundedVec; + + #[test] + fn set_updates_values_properly() { + let mut vec = BoundedVec::from_array([0, 0, 0, 0, 0]); + + vec.set(0, 42); + assert_eq(vec.storage, [42, 0, 0, 0, 0]); + + vec.set(1, 43); + assert_eq(vec.storage, [42, 43, 0, 0, 0]); + + vec.set(2, 44); + assert_eq(vec.storage, [42, 43, 44, 0, 0]); + + vec.set(1, 10); + assert_eq(vec.storage, [42, 10, 44, 0, 0]); + + vec.set(0, 0); + assert_eq(vec.storage, [0, 10, 44, 0, 0]); + } + + #[test(should_fail_with = "Attempted to write past end of BoundedVec")] + fn panics_when_writing_elements_past_end_of_vec() { + let mut vec: BoundedVec = BoundedVec::new(); + vec.set(0, 42); + + // Need to use println to avoid DIE removing the write operation. + crate::println(vec.get(0)); + } + } + mod from_array { use crate::collections::bounded_vec::BoundedVec; diff --git a/noir_stdlib/src/collections/vec.nr b/noir_stdlib/src/collections/vec.nr index 18aaa8b9b3b..cedae7f5ce1 100644 --- a/noir_stdlib/src/collections/vec.nr +++ b/noir_stdlib/src/collections/vec.nr @@ -21,6 +21,12 @@ impl Vec { self.slice[index] } + /// Write an element to the vector at the given index. + /// Panics if the given index points beyond the end of the vector (`self.len()`). + pub fn set(&mut self, index: u32, value: T) { + self.slice[index] = value; + } + /// Push a new element to the end of the vector, returning a /// new vector with a length one greater than the /// original unmodified vector. @@ -57,3 +63,36 @@ impl Vec { self.slice.len() } } + +mod tests { + use crate::collections::vec::Vec; + + #[test] + fn set_updates_values_properly() { + let mut vec = Vec { slice: &[0, 0, 0, 0, 0] }; + + vec.set(0, 42); + assert_eq(vec.slice, &[42, 0, 0, 0, 0]); + + vec.set(1, 43); + assert_eq(vec.slice, &[42, 43, 0, 0, 0]); + + vec.set(2, 44); + assert_eq(vec.slice, &[42, 43, 44, 0, 0]); + + vec.set(1, 10); + assert_eq(vec.slice, &[42, 10, 44, 0, 0]); + + vec.set(0, 0); + assert_eq(vec.slice, &[0, 10, 44, 0, 0]); + } + + #[test(should_fail)] + fn panics_when_writing_elements_past_end_of_vec() { + let mut vec = Vec::new(); + vec.set(0, 42); + + // Need to use println to avoid DIE removing the write operation. + crate::println(vec.get(0)); + } +} diff --git a/test_programs/noir_test_success/bounded_vec/src/main.nr b/test_programs/noir_test_success/bounded_vec/src/main.nr index 22ec291f9d6..e5aa5f88a94 100644 --- a/test_programs/noir_test_success/bounded_vec/src/main.nr +++ b/test_programs/noir_test_success/bounded_vec/src/main.nr @@ -63,6 +63,33 @@ fn sum_of_first_three(v: BoundedVec) -> u32 { } // docs:end:get_unchecked_example +#[test(should_fail)] +// docs:start:set_unchecked_example +fn set_unchecked_example() { + let mut vec: BoundedVec = BoundedVec::new(); + vec.extend_from_array([1, 2]); + + // Here we're safely writing within the valid range of `vec` + // `vec` now has the value [42, 2] + vec.set_unchecked(0, 42); + + // We can then safely read this value back out of `vec`. + // Notice that we use the checked version of `get` which would prevent reading unsafe values. + assert_eq(vec.get(0), 42); + + // We've now written past the end of `vec`. + // As this index is still within the maximum potential length of `v`, + // it won't cause a constraint failure. + vec.set_unchecked(2, 42); + println(vec); + + // This will write past the end of the maximum potential length of `vec`, + // it will then trigger a constraint failure. + vec.set_unchecked(5, 42); + println(vec); +} +// docs:end:set_unchecked_example + #[test(should_fail_with = "push out of bounds")] fn push_docs_example() { // docs:start:bounded-vec-push-example diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index 0bb967e7502..70857b4b65e 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -1,6 +1,8 @@ +use std::io::Write; use std::{collections::BTreeMap, path::PathBuf}; use acvm::blackbox_solver::StubbedBlackBoxSolver; +use fm::FileManager; use noirc_driver::{check_crate, file_manager_with_stdlib, CompileOptions}; use noirc_frontend::hir::FunctionNameMatch; @@ -9,6 +11,7 @@ use nargo::{ package::{Package, PackageType}, parse_all, prepare_package, }; +use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor}; #[test] fn run_stdlib_tests() { @@ -23,7 +26,7 @@ fn run_stdlib_tests() { root_dir: PathBuf::from("."), package_type: PackageType::Binary, entry_path: PathBuf::from("main.nr"), - name: "dummy".parse().unwrap(), + name: "stdlib".parse().unwrap(), dependencies: BTreeMap::new(), }; @@ -58,5 +61,85 @@ fn run_stdlib_tests() { .collect(); assert!(!test_report.is_empty(), "Could not find any tests within the stdlib"); + display_test_report(&file_manager, &dummy_package, &CompileOptions::default(), &test_report); assert!(test_report.iter().all(|(_, status)| !status.failed())); } + +// This code is copied from `src/cli/test_cmd.rs`. +// This should be abstracted into a proper test runner at some point. +fn display_test_report( + file_manager: &FileManager, + package: &Package, + compile_options: &CompileOptions, + test_report: &[(String, TestStatus)], +) { + let writer = StandardStream::stderr(ColorChoice::Always); + let mut writer = writer.lock(); + + for (test_name, test_status) in test_report { + write!(writer, "[{}] Testing {test_name}... ", package.name) + .expect("Failed to write to stderr"); + writer.flush().expect("Failed to flush writer"); + + match &test_status { + TestStatus::Pass { .. } => { + writer + .set_color(ColorSpec::new().set_fg(Some(Color::Green))) + .expect("Failed to set color"); + writeln!(writer, "ok").expect("Failed to write to stderr"); + } + TestStatus::Fail { message, error_diagnostic } => { + writer + .set_color(ColorSpec::new().set_fg(Some(Color::Red))) + .expect("Failed to set color"); + writeln!(writer, "FAIL\n{message}\n").expect("Failed to write to stderr"); + if let Some(diag) = error_diagnostic { + noirc_errors::reporter::report_all( + file_manager.as_file_map(), + &[diag.clone()], + compile_options.deny_warnings, + compile_options.silence_warnings, + ); + } + } + TestStatus::CompileError(err) => { + noirc_errors::reporter::report_all( + file_manager.as_file_map(), + &[err.clone()], + compile_options.deny_warnings, + compile_options.silence_warnings, + ); + } + } + writer.reset().expect("Failed to reset writer"); + } + + write!(writer, "[{}] ", package.name).expect("Failed to write to stderr"); + + let count_all = test_report.len(); + let count_failed = test_report.iter().filter(|(_, status)| status.failed()).count(); + let plural = if count_all == 1 { "" } else { "s" }; + if count_failed == 0 { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Green))).expect("Failed to set color"); + write!(writer, "{count_all} test{plural} passed").expect("Failed to write to stderr"); + writer.reset().expect("Failed to reset writer"); + writeln!(writer).expect("Failed to write to stderr"); + } else { + let count_passed = count_all - count_failed; + let plural_failed = if count_failed == 1 { "" } else { "s" }; + let plural_passed = if count_passed == 1 { "" } else { "s" }; + + if count_passed != 0 { + writer + .set_color(ColorSpec::new().set_fg(Some(Color::Green))) + .expect("Failed to set color"); + write!(writer, "{count_passed} test{plural_passed} passed, ",) + .expect("Failed to write to stderr"); + } + + writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).expect("Failed to set color"); + writeln!(writer, "{count_failed} test{plural_failed} failed") + .expect("Failed to write to stderr"); + writer.reset().expect("Failed to reset writer"); + } +}