-
Notifications
You must be signed in to change notification settings - Fork 170
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: Handle ACIR calls in the debugger #5051
Open
ggiraldez
wants to merge
22
commits into
noir-lang:master
Choose a base branch
from
manastech:feat/4824-multiple-acir-calls
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat: Handle ACIR calls in the debugger #5051
ggiraldez
wants to merge
22
commits into
noir-lang:master
from
manastech:feat/4824-multiple-acir-calls
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Returning serialized opcode locations is invalid according to the DAP specification and confuses VS.Code.
Make the code consistent with the DAP and enables to start implementing debugging for multiple circuits.
in addition to ACIR and Brillig indices.
Thank you for your contribution to the Noir language. Please do not force push to this branch after the Noir team have started review of this PR. Doing so will only delay us merging your PR as we will need to start the review process from scratch. Thanks for your understanding. |
Thank you for the PR @ggiraldez! I plan to look at this soon, just have been taken away by other tasks |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Problem*
Resolves #4824
With the introduction of ACIR calls to allow non-inlined ACIR functions, the debugger did not have proper support for programs that had them, neither when compiling in ACIR mode, nor in the default mode which compiles the full program to Brillig.
Summary*
This PR depends on #4941 and is built on top of it.
The changes allow compiling a program with
#[fold]
(and other non-inline) annotated functions in Brillig mode (the default when debugging) by forcing the selection of the Brillig runtime when compiling them. For inline functions, during SSA generation the runtime is not changed in order to allow some optimizations in the stdlib to still work. For example, some constant assertions don't work unless the function is inlined.When debugging in ACIR mode (with the
--acir-mode
CLI orgenerateAcir: true
DAP options), the debugger now properly handles programs with multiple circuits and ACIR calls by:Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmt
on default settings.