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: enable dynamic arrays #1271

Merged
merged 4 commits into from
May 3, 2023
Merged

feat: enable dynamic arrays #1271

merged 4 commits into from
May 3, 2023

Conversation

guipublic
Copy link
Contributor

@guipublic guipublic commented May 2, 2023

Related issue(s)

Resolves #1011

Description

Summary of changes

Enable dynamic arrays in noir. It was disabled because of a bug in the backend which is finally solved.

Test additions / changes

The integration test is uncomment and can be verified successfully.

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.

Documentation needs

  • This PR requires documentation updates when merged.

Comptime types are not required anymore when indexing an array.

Additional context

Note that this does not use the UP RAM lookup table. It will be added in another PR.

BEGIN_COMMIT_OVERRIDE
feat: Enable dynamic arrays (#1271)
END_COMMIT_OVERRIDE

Copy link
Contributor

@kevaundray kevaundray left a comment

Choose a reason for hiding this comment

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

Looks good overall -- can you make a separate project for dynamic arrays since we may want to add regression tests for it and it may start to become large

@guipublic
Copy link
Contributor Author

could you clarify what is the project you mean?

@kevaundray
Copy link
Contributor

could you clarify what is the project you mean?

Just another noir example project, so instead of 6_array, we could have array_dynamic

@kevaundray kevaundray added this pull request to the merge queue May 3, 2023
Merged via the queue into master with commit 9f43450 May 3, 2023
@kevaundray kevaundray deleted the gd/enable_dyn_array branch May 3, 2023 10:28
@Savio-Sou
Copy link
Collaborator

Oh just realized from this PR that using "[X]" instead of "[x]" for checkboxes won't trigger the doc needed labeling GH action lol

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.

Enable dynamic arrays
3 participants