Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add set and set_unchecked methods to Vec and BoundedVec #5241

Merged
merged 6 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions docs/docs/noir/standard_library/containers/boundedvec.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -80,7 +80,7 @@ fn foo<N>(v: BoundedVec<u32, N>) {
### 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
Expand All @@ -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<N>(v: BoundedVec<u32, N>) {
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

Expand Down
19 changes: 19 additions & 0 deletions docs/docs/noir/standard_library/containers/vec.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,25 @@ let vector: Vec<Field> = 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<Field> = 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.
Expand Down
59 changes: 56 additions & 3 deletions noir_stdlib/src/collections/bounded_vec.nr
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,35 @@ impl<T, MaxLen> BoundedVec<T, MaxLen> {
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");

Expand Down Expand Up @@ -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<Field, 5> = 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;

Expand Down
39 changes: 39 additions & 0 deletions noir_stdlib/src/collections/vec.nr
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ impl<T> Vec<T> {
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.
Expand Down Expand Up @@ -57,3 +63,36 @@ impl<T> Vec<T> {
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));
}
}
27 changes: 27 additions & 0 deletions test_programs/noir_test_success/bounded_vec/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,33 @@ fn sum_of_first_three<N>(v: BoundedVec<u32, N>) -> u32 {
}
// docs:end:get_unchecked_example

#[test(should_fail)]
// docs:start:set_unchecked_example
fn set_unchecked_example() {
let mut vec: BoundedVec<u32, 5> = 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
Expand Down
85 changes: 84 additions & 1 deletion tooling/nargo_cli/tests/stdlib-tests.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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() {
Expand All @@ -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(),
};

Expand Down Expand Up @@ -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");
}
}
Loading