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: array concat method #7199

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

feat: array concat method #7199

wants to merge 6 commits into from

Conversation

iAmMichaelConnor
Copy link
Collaborator

@iAmMichaelConnor iAmMichaelConnor commented Jan 27, 2025

Description

Add a concat method to the stdlib for the array type.

Where are the array tests? Where should I put a test for this method? Edit: I've put the test in the same file (despite not seeing tests for the other array methods there)

Problem*

Summary*

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jan 27, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: d161a0d Previous: 4fe4cc6 Ratio
sha256_regression 0.076 s 0.051 s 1.49

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Copy link
Contributor

github-actions bot commented Jan 27, 2025

@iAmMichaelConnor iAmMichaelConnor mentioned this pull request Jan 27, 2025
5 tasks
Comment on lines +261 to +272

```rust
fn concat<let M: u32>(self, array2: [T; M]) -> [T; N + M]
```

```rust
fn main() {
let arr1 = [1, 2, 3, 4];
let arr2 = [6, 7, 8, 9, 10, 11];
let concatenated_arr = arr1.concat(arr2);
assert(concatenated_arr == [1, 2, 3, 4, 6, 7, 8, 9, 10, 11]);
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you snippet-ify this like in the aztec docs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I couldn't tell if you had some kind of clever code snippet stuff in the Noir repo, so I hand-wrote this. Do you have something? Or are you asking if it'd be possible to set something up like the snippet-ifying that we do in aztec?

Copy link
Member

Choose a reason for hiding this comment

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

We pulled across the same snippet code that you added to the aztec-packages docs a while back. You can use the exact same syntax as in the aztec docs to insert snippets of noir code into the docs here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perhaps that's one for a separate, dedicated PR, because I don't think any of noir_stdlib/src/array/mod.nr is using that code snippet tool yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Contributor

FYI @noir-lang/developerrelations on Noir doc changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants