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: Allow step debugging inside Brillig blocks #3155

Closed
wants to merge 4 commits into from

Conversation

ggiraldez
Copy link
Contributor

Description

When running the debugger and stepping through a Brillig block, execute each Brillig opcode step by step.

Part of #3015.

Problem

Before this PR, the debugger step command would execute the full Brillig block when stepping through it.

Summary

  • Adds support to ACVM to step into a Brillig block suspending execution after each Brillig opcode.
  • Adds a new method to ACVM to get the full opcode location to be executed/solved next, also when stepping inside a Brillig block.

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

I'm not 100% happy with the API, especially due to the use of boolean values to distinguish between the two modes of execution, but the resulting code is pretty compact and backwards compatible.

PR Checklist

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

@TomAFrench TomAFrench self-requested a review October 16, 2023 09:42
@TomAFrench
Copy link
Member

Will try to take a look at this today.

@ggiraldez
Copy link
Contributor Author

This functionality was implemented by #3259 taking a different approach.

@ggiraldez ggiraldez closed this Oct 25, 2023
@mverzilli mverzilli deleted the brillig-debug-stepping branch November 23, 2023 16:47
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.

2 participants