search

noir-lang / vscode-noir

Apache License 2.0
6 stars 7 forks source link

feat: add support for nargo DAP #51

Closed ggiraldez closed 5 months ago

ggiraldez commented 9 months ago

Description

:information_source: This PR should be applied after https://github.com/noir-lang/noir/pull/4185 since it builds on top of the features implemented there.

Adds support in the extension to debug Noir binary packages using the DAP subcommand in nargo, implemented by noir-lang/noir#3627.

Problem

Part of noir-lang/noir#3015

Summary

This PR adds the configuration necessary in package.json of the extension to allow debugging Noir binary packages from inside VS.Code using the Debug Adapter implemented in the above mentioned noir PR.

Supports settings breakpoints in Noir source code and opening a Disassembly view while debugging to display the compiled ACIR/Brillig opcodes. All stepping commands will produce the same result, ie. stepping to the next "statement" in the Noir program, ie. there is no concept of stepping into or out of a function yet. If using the commands from the disassembly view, the debugger will step to the next opcode instead.

Additional Context

PR Checklist*

kobyhallx commented 9 months ago

Usability Observation nr 1

No default launch - when I open debug panel and attempt to run noir program nothing will happen, instead I have to create launch config for any debug to take place.

kobyhallx commented 9 months ago

Usability Observation nr 2

Program by default stops at first opcode - while this is desired in REPL style debugger I believe it should just execute to the end when there's no breakpoint on the way.

kobyhallx commented 9 months ago

Usability Observation nr 3

If breakpoint is set in Function and user choses continue, execution does not stop and just continues to the end of program.

kobyhallx commented 9 months ago

Usability Observation nr 4

Step into is stepping over. (Note, I do realise that PR says otherwise but I believe this is limitation and breaks UX contract leading to users confusion)

kobyhallx commented 9 months ago

Usability Observation nr 5

No variables are being displayed in Variables Panel.

TomAFrench commented 9 months ago

Looks like there was some formatting changes from prettier which are included in here. I've fixed these issues in master but could you merge master back into this branch to reduce the diff @ggiraldez?

TomAFrench commented 9 months ago

~Actually, give me a couple of moments. I'm bringing across the linting config from the main repository so it can be enforced here.~

Good to merge it into this branch now.

ggiraldez commented 9 months ago

Usability Observation nr 1

No default launch - when I open debug panel and attempt to run noir program nothing will happen, instead I have to create launch config for any debug to take place.

This is a known issue, which shouldn't be hard to fix. But yes, agreed it makes the DX cumbersome.

ggiraldez commented 9 months ago

Usability Observation nr 2

Program by default stops at first opcode - while this is desired in REPL style debugger I believe it should just execute to the end when there's no breakpoint on the way.

That's a good point. What would you expect to be the behavior if there are no breakpoints set? It could get a bit confusing because I guess by default nothing will happen (at least nothing noticeable) when you hit the Run button.

ggiraldez commented 9 months ago

Usability Observation nr 3

If breakpoint is set in Function and user choses continue, execution does not stop and just continues to the end of program.

I guess the problem here is that the debugger is not being able to properly set the breakpoint but you're not getting feedback in the UI. I've encountered this issue as well, but haven't found a good solution yet.

ggiraldez commented 9 months ago

Usability Observation nr 4

Step into is stepping over. (Note, I do realise that PR says otherwise but I believe this is limitation and breaks UX contract leading to users confusion)

All stepping commands do the same, which is to continue execution until the source location changes (ie. execute as many opcodes as necessary for the the source location from the debug artifact to change). Depending on compiler optimizations and inlining it may be that it looks like a function call is skipped over.

We will try to address this issue by compiling to Brillig only (instead of ACIR/Brillig) when debugging. We expect that to give us a bit more control over stepping and the ability to get proper stacktraces.

ggiraldez commented 9 months ago

With observatioins shared in comments Usability Observation nr 5 is in my mind minimum to expose this through vscode.

Agreed. This is being worked on. There is some preliminary work in the https://github.com/manastech/noir/tree/dap-with-vars branch, but it depends on merging the debug variables instrumentation code.

TomAFrench commented 5 months ago

Thank you and sorry for this PR being stuck in purgatory!