Support break and continue in for loops
#4259
Comments
I don't think implementing |
|
These could potentially be considered as an unconstrained-only feature. |
|
This was discused offline a bit. One concern is that Concensus however is to add this both to ACIR and Brillig. Noir's goal is to be easy to write and have a low barrier to entry. If someone wants to then optimize their circuit they can move away from something like |
|
I think this may be potentially tricky to implement since we want two completely different behaviors depending on the runtime:
Notably, acir functions can also be run in an unconstrained context, where they should presumably have the first behavior instead. This means we'll probably need new, dedicated continue & break instructions in ssa, but these add complexity since they're new terminator instructions. Additionally, translating these to the intermediate variable and if statement over the loop body is non-trivial I think. |
|
Would this easier if we restricted the usage of |
|
@TomAFrench yes, quite a bit I think. |
|
Cool, I think that it would probably be a good initial step then. We'll get the benefits of smaller bytecode for the public VM in the short term and then we can revisit whether it makes sense to have this syntax in ACIR purely for DX later on. |
|
on a related note, we could also allow usage of the |
|
In that case I'd consider having a constrained |
|
Yep, we should do that. |
# Description ## Problem\* Resolves #4259 ## Summary\* Implements break & continue in for loops in unconstrained code only. When attempted to use outside of a loop or in a constrained function, an error is issued. ## Additional Context I didn't put a link to the docs in an error message, should I? It seems like this may be something we may want to do in other errors as well. This PR is currently missing documentation. The test failures are from the change to terminator instructions. This was needed since otherwise when break/continue set the terminator it was being overridden when `codegen_if` would set the terminator instruction for the same block afterward. One somewhat messy way to fix this is to switch to an empty, unreachable block. Then the if's terminator would set on that block instead. But if we push any instructions in between those would be lost too and we'd be none the wiser. Although pushing instructions after a break is usually a logic error in the compiler anyway. ## Documentation\* Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
Problem
Loops in Noir do not currently support
breakandcontinuewhich can lead to awkward boolean flags to try to get around this. These boolean flags are often less efficient since Noir is still performing each iteration and checking a boolean flag on each iteration instead of stopping early.Happy Case
We add
breakandcontinueto Noir, following the semantics ofbreakandcontinuein Rust.Alternatives Considered
No response
Additional Context
This was somewhat inspired by #4242 which uses a HashMap implementation where the
getmethod is less efficient because of the lack ofbreak.Would you like to submit a PR for this Issue?
No
Support Needs
No response
The text was updated successfully, but these errors were encountered: