[8/x] Add Emulator::print_trace option to print the current stack and memory on every step
#237
Conversation
greenhat
force-pushed
the
greenhat/emu-print-stack-option
branch
from
c06811c to
1ce9065
Compare
greenhat
force-pushed
the
greenhat/emu-print-stack-option
branch
from
1ce9065 to
79d1c24
Compare
greenhat
changed the title
Emulator::print_stack option to print the current stack on every stepEmulator::print_trace option to print the current stack and memory on every step
bitwalker
left a comment
bitwalker
left a comment
There was a problem hiding this comment.
Overall looks good! One minor suggestion about ordering of items in the output that you can take or leave based on your preference; and two small changes that I think we should make prior to merging.
codegen/masm/src/emulator/memory.rs
Outdated
|
|
||
| pub struct Memory { | ||
| memory: Vec<[Felt; 4]>, | ||
| set_memory_addrs: FxHashSet<usize>, |
There was a problem hiding this comment.
Suggestion: I would make this a BTreeSet, so that you can print the memory addresses in order, plus its probably faster than a hashset in this case, since memory addresses are totally ordered, and highly likely to be written to in order from least to greatest
There was a problem hiding this comment.
Good catch! I missed it since I only dealt with a couple of addresses, but the order is important.
Done in 8ca444b
hir/src/asm/stack.rs
Outdated
| self | ||
| } | ||
|
|
||
| pub fn pretty(&self) -> String { |
There was a problem hiding this comment.
I think we should make this a Display impl, and base it on the existing Debug impl. The only difference between this and Debug seems to be how we format each stack element, which is certainly useful, but I'd rather use Display for it, since it is far more ergonomic than building strings directly, and you get to_string for free.
greenhat
force-pushed
the
greenhat/fix-emu-mem-clean
branch
2 times, most recently
from
effc46b to
8aea8fe
Compare
and memory contents (only written to) on every step of the execution.
greenhat
force-pushed
the
greenhat/emu-print-stack-option
branch
from
79d1c24 to
fb59577
Compare
…lator Print the emulator's memory before the stack. Impl `Display` instead of `DebugStack::pretty`.
This PR is stacked on the #236 and should be merged after it