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: dynamic arrays for experimental-ssa #1969

Merged
merged 24 commits into from
Jul 24, 2023
Merged

Conversation

guipublic
Copy link
Contributor

@guipublic guipublic commented Jul 19, 2023

Description

Problem*

Test case array-dynamic is now passing

Resolves #1964

Summary*

The PR is now ready for review:

For non-const index on array operation, we generate a new ACIR opcode corresponding to the operation. This allows ACVM solver to perform stepwise/in-order solving. The backend can easily create the block constraints from them.
Because an array-set generates a new ssa array, we need to copy the array into a new one also in acir-gen. This could be optimised out in case the array we copy is not re-used afterwards. This new array has no more values associated to it so it is mapped to a new AcirArray type.

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

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

@guipublic guipublic marked this pull request as ready for review July 24, 2023 10:46
@kevaundray kevaundray enabled auto-merge July 24, 2023 14:32
@kevaundray kevaundray added this pull request to the merge queue Jul 24, 2023
@vezenovm vezenovm mentioned this pull request Jul 24, 2023
5 tasks
Merged via the queue into master with commit 08d199a Jul 24, 2023
@kevaundray kevaundray deleted the gd/witness_indices branch July 24, 2023 15:01
@Savio-Sou
Copy link
Collaborator

@kevaundray / @guipublic seems that this is a bug fix? What needs to be documented exactly?

@guipublic
Copy link
Contributor Author

@kevaundray / @guipublic seems that this is a bug fix? What needs to be documented exactly?

I don't think it is needed to be documented, as it is adding the dynamic arrays feature to the experimental-sea. It should be transparent for the user.

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.

"Expected array index to be a known constant" within an if statement
3 participants