-
Notifications
You must be signed in to change notification settings - Fork 220
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
chore: Add abstraction layer around ACIR gen of quotient directives #1900
chore: Add abstraction layer around ACIR gen of quotient directives #1900
Conversation
The bigger issue here is the fact that we are liberally creating witnesses out of thin air. An abstraction over this like |
…hub.com:noir-lang/noir into kw/solve-quotient-before-using-it-in-constraint
I can add this in this PR if you're not working on this already. |
Sorry forgot to reply -- yeah this would be good, I have a few things in the backlog, so it would be quicker if you did it |
@TomAFrench do you want to add it here, or should we merge this and add it in a separate PR? (Either way is fine with me, my suggestion above would affect more than just the quotient directive I think, so we would need to rename the PR appropriately) |
Easiest to do it here. Reordering the opcodes is only a substantive change in the context of #1729 so we can just make this PR be primarily about the new abstraction layer. |
@kevaundray if you give an ok in the comments here, I can approve + merge. |
Looks like we should add the range constraints into the helper as well. |
LGTM |
Description
Problem*
Resolves
Summary*
Documentation
This PR requires documentation updates when merged.
Additional Context
PR Checklist*
cargo fmt
on default settings.