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.

Sign up for GitHub

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

Jump to bottom

feat: adds debugging #79

Merged
merged 1 commit into from
Mar 16, 2023
Merged

feat: adds debugging #79

merged 1 commit into from
Mar 16, 2023

Conversation

Dominik1999
Copy link
Contributor

@Dominik1999 Dominik1999 commented Mar 13, 2023 •
edited
Loading

closes #68

This PR enables the frontend to run the Rust program debug_program, which executes miden_vm::execute_iter.

Screen.Recording.2023-03-14.at.17.11.56.mov

OK, now The playground uses the same debugging methods as the Miden CLI, see here.

For now, I have only implemented 5 methods in the fronend

    PlayAll,
    Play(usize),
    RewindAll,
    Rewind(usize),
    PrintState,

but all methods are exposed in the backend.

We should refactor the lib.rs and CodingEnvironment.tsx, both grow way too big, but that might be for the next PR.

@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch from 7b11229 to 1822f3b Compare March 13, 2023 11:08
@Dominik1999 Dominik1999 force-pushed the dominik_refactor_constants branch from a0f18f9 to cedded3 Compare March 13, 2023 11:24
@Dominik1999 Dominik1999 changed the base branch from dominik_refactor_constants to next March 13, 2023 11:26
@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch 3 times, most recently from bb7f332 to 7edcc28 Compare March 13, 2023 12:26
@Dominik1999 Dominik1999 requested a review from grjte March 13, 2023 13:06
@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch from 7edcc28 to 03ff43a Compare March 13, 2023 13:06
@Dominik1999 Dominik1999 changed the title feat: adds debugging with no UI [WIP] feat: adds debugging with no UI Mar 13, 2023
@grjte grjte requested a review from hackaugusto March 13, 2023 14:19
@grjte grjte removed their request for review March 13, 2023 15:54
@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch from 03ff43a to c85bf12 Compare March 14, 2023 16:11
@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch from c85bf12 to a7ffced Compare March 14, 2023 16:18
@Dominik1999 Dominik1999 changed the title feat: adds debugging with no UI feat: adds debugging Mar 14, 2023
@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch from a7ffced to 74b8b8d Compare March 14, 2023 16:32
@Dominik1999 Dominik1999 requested review from bitwalker and removed request for bitwalker March 14, 2023 17:01
@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch from 74b8b8d to e9ff091 Compare March 14, 2023 17:26
bitwalker
bitwalker approved these changes Mar 14, 2023
View reviewed changes
Copy link

@bitwalker bitwalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No notes, LGTM!

@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch from e9ff091 to 749e856 Compare March 14, 2023 18:35
@Dominik1999 Dominik1999 mentioned this pull request Mar 14, 2023
Closed
@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch 2 times, most recently from 7df9145 to 84db9d7 Compare March 14, 2023 19:07
hackaugusto
hackaugusto reviewed Mar 15, 2023
View reviewed changes
playground/miden-wasm/src/lib.rs Outdated Show resolved Hide resolved
playground/miden-wasm/src/lib.rs Outdated Show resolved Hide resolved
playground/miden-wasm/src/lib.rs Outdated Show resolved Hide resolved
playground/miden-wasm/src/lib.rs Outdated Show resolved Hide resolved
playground/miden-wasm/src/lib.rs Outdated Show resolved Hide resolved
playground/miden-wasm/src/lib.rs Outdated Show resolved Hide resolved
playground/miden-wasm/src/lib.rs Outdated Show resolved Hide resolved
playground/src/pages/CodingEnvironment.tsx Outdated Show resolved Hide resolved
playground/src/pages/CodingEnvironment.tsx Outdated Show resolved Hide resolved
playground/src/utils/helper_functions.tsx Show resolved Hide resolved
@Dominik1999 Dominik1999 requested a review from hackaugusto March 15, 2023 19:26
@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch from 84db9d7 to c550b45 Compare March 15, 2023 19:30
@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch from c550b45 to b67197d Compare March 16, 2023 08:11
@Dominik1999 Dominik1999 force-pushed the dominik_add_debugging branch from b67197d to 6b3c8f1 Compare March 16, 2023 14:09
hackaugusto
hackaugusto approved these changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@hackaugusto hackaugusto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, remaining comments are nits and can be done on follow ups. I ran it locally and tested with the comparison example program

playground/miden-wasm/src/lib.rs
/// executes a debug command against the vm in it's current state.
pub fn execute(&mut self, command: DebugCommand, param: Option<u64>) -> String {
match command {
//DebugCommand::PlayAll
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

later on these comments can go, one of the purposes of using the enum is to have self documenting code

playground/src/pages/CodingEnvironment.tsx
icon="Stack"
onClick={() => executeDebug(DebugCommand.PrintState)}
/>
<DebugButton
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be nice to show a text when hovering on these buttons later on. they are pretty intuitive but that wont hurt (also maybe have aria stuff in there would be a nice thing to do)

@Dominik1999 Dominik1999 merged commit 2bb4934 into next Mar 16, 2023
@Dominik1999 Dominik1999 mentioned this pull request Mar 16, 2023
Closed
@hackaugusto hackaugusto deleted the dominik_add_debugging branch March 16, 2023 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bitwalker bitwalker bitwalker approved these changes

@hackaugusto hackaugusto hackaugusto approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Dominik1999 @hackaugusto @bitwalker