Skip to content

feat: add array range and reverse functions#7159

Open
DeVikingMark wants to merge 4 commits intonoir-lang:masterfrom
DeVikingMark:denmarkfixes
Open

feat: add array range and reverse functions#7159
DeVikingMark wants to merge 4 commits intonoir-lang:masterfrom
DeVikingMark:denmarkfixes

Conversation

@DeVikingMark
Copy link
Contributor

Description:

This PR adds two utility functions to the array module:

  • range(size: Field) - creates an array of numbers from 0 to size-1
  • reverse() - returns a new array with elements in reverse order

Resolves TODO comments from test_programs/noir_test_success/brillig_oracle/src/main.nr

Both functions are fully tested and documented with examples.

@github-actions
Copy link
Contributor

Thank you for your contribution to the Noir language.

Please do not force push to this branch after the Noir team have started review of this PR. Doing so will only delay us merging your PR as we will need to start the review process from scratch.

Thanks for your understanding.

/// assert(arr == [0, 1, 2, 3, 4]);
/// }
/// ```
pub fn range<T>(size: Field) -> [Field] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The T generic parameter is not used, and technically this returns a slice, rather than an array.
size is usually expressed as u32.
Returning Field seems a bit specific; the library so far is completely generic.

Perhaps something like std::array::from_fn would be a more flexible alternative to make an actual array of a given size, with content created for each index given to a lambda as u32 between 0 and N. You could use it to cast to Field, or map it to a range other than 0..N, e.g. by an offset.

@DeVikingMark DeVikingMark requested a review from aakoshh February 23, 2025 23:50
@DeVikingMark
Copy link
Contributor Author

@aakoshh hey Akosh, I've made an update, just curious what are ur thoughts abt it?

@aakoshh
Copy link
Contributor

aakoshh commented Mar 19, 2025

@DeVikingMark sorry for the late reply. Thanks for the changes, it's looking better!

All that's left is this part:

and technically this returns a slice, rather than an array.

To returned an array (which are fixed size), the functions signatures should look like this:

pub fn from_fn<T, let N: u32>(size: u32, f: fn[Env](u32) -> T) -> [T; N];
pub fn range<let N: u32>(size: u32) -> [u32; N];

It will figure out the N from the type you're trying to assign to.

EDIT: Initially I thought that size is redundant, because we have N, but the len function reminds me that N is just the maximum capacity, the length can be less than that, so size can partially fill the array.

@aakoshh
Copy link
Contributor

aakoshh commented Mar 19, 2025

Another remark is that these functions could be added to BoundedVec as well, and even slices, like in #7705

@jfecher
Copy link
Contributor

jfecher commented Mar 19, 2025

EDIT: Initially I thought that size is redundant, because we have N, but the len function reminds me that N is just the maximum capacity, the length can be less than that, so size can partially fill the array.

size is redundant. Arrays only have a length, not a capacity so N == array.len() always. This is only not true for BoundedVec

@DeVikingMark
Copy link
Contributor Author

@aakoshh made some changes, there are probably some small fixes need to be done, but I just wanna know what do you think about it

/// assert(arr == [0, 2, 4, 6, 8]);
/// }
/// ```
pub fn from_fn<T>(size: u32, f: fn(u32) -> T) -> [T] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function does not return an array, they return a slice. I suggest we remove it, and rename the from_fn_fixed to from_fn instead.

/// assert(arr == [0, 1, 2, 3, 4]);
/// }
/// ```
pub fn range(size: u32) -> [u32] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: it returns a slice with a dynamic length. Should be removed in favour of renaming range_fixed to range.

Comment on lines +46 to +57
/// fn main() {
/// let arr: [Field; 5] = std::array::from_fn_fixed(5, |i| i as Field);
/// assert(arr == [0, 1, 2, 3, 4]);
/// }
/// ```
pub fn from_fn_fixed<T, let N: u32>(size: u32, f: fn(u32) -> T) -> [T; N] {
assert(size <= N, "Size cannot exceed array capacity N");

let mut result = [crate::mem::zeroed(); N];
for i in 0..size {
result[i] = f(i);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// fn main() {
/// let arr: [Field; 5] = std::array::from_fn_fixed(5, |i| i as Field);
/// assert(arr == [0, 1, 2, 3, 4]);
/// }
/// ```
pub fn from_fn_fixed<T, let N: u32>(size: u32, f: fn(u32) -> T) -> [T; N] {
assert(size <= N, "Size cannot exceed array capacity N");
let mut result = [crate::mem::zeroed(); N];
for i in 0..size {
result[i] = f(i);
}
/// fn main() {
/// let arr: [Field; 5] = std::array::from_fn(|i| i as Field);
/// assert(arr == [0, 1, 2, 3, 4]);
/// }
/// ```
pub fn from_fn<T, let N: u32>(f: fn(u32) -> T) -> [T; N] {
let mut result = [crate::mem::zeroed(); N];
for i in 0..N {
result[i] = f(i);
}

It's simpler this way, I don't think the partial fill was a good suggestion on my part, sorry.

Comment on lines +63 to +74
/// The size parameter can be less than N to partially fill the array.
///
/// Example:
/// ```noir
/// fn main() {
/// let arr: [u32; 5] = std::array::range_fixed(5);
/// assert(arr == [0, 1, 2, 3, 4]);
/// }
/// ```
pub fn range_fixed<let N: u32>(size: u32) -> [u32; N] {
from_fn_fixed(size, |i| i)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// The size parameter can be less than N to partially fill the array.
///
/// Example:
/// ```noir
/// fn main() {
/// let arr: [u32; 5] = std::array::range_fixed(5);
/// assert(arr == [0, 1, 2, 3, 4]);
/// }
/// ```
pub fn range_fixed<let N: u32>(size: u32) -> [u32; N] {
from_fn_fixed(size, |i| i)
}
///
/// Example:
/// ```noir
/// fn main() {
/// let arr: [u32; 5] = std::array::range();
/// assert(arr == [0, 1, 2, 3, 4]);
/// }
/// ```
pub fn range<let N: u32>() -> [u32; N] {
from_fn(|i| i)
}


/// Creates a new fixed-size array by applying a function to each index.
/// This is a utility function to create arrays with custom initialization.
/// The size parameter can be less than N to partially fill the array.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// The size parameter can be less than N to partially fill the array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants