-
Notifications
You must be signed in to change notification settings - Fork 556
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
Added panic print in executable run. #7131
Conversation
cb43895
to
5064702
Compare
This is going to break if someone else uses markers. Suggestion: [.., start_marker, end_marker] = &hint_processor.markers[..] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @ilyalesokhin-starkware)
crates/bin/cairo-execute/src/main.rs
line 248 at r1 (raw file):
Previously, ilyalesokhin-starkware wrote…
This is going to break if someone else uses markers.
Do you want the markers to be that generic?
if so maybe take the last two markers?
i like it - Done
5064702
to
d96c10f
Compare
022c26c
to
b25d7ff
Compare
Suggestion: /// stores a marker in the HintProcessor. Useful for debugging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 9 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: 6 of 9 files reviewed, 2 unresolved discussions (waiting on @orizi)
crates/cairo-lang-runnable-utils/src/builder.rs
line 425 at r1 (raw file):
let ret = CellExpression::Deref(deref!([ap - unprocessed_return_size])); unprocessed_return_size -= 1; ret
ret
is a bit confusing.
consider deref
or res
Suggestion:
let deref = CellExpression::Deref(deref!([ap - unprocessed_return_size]));
unprocessed_return_size -= 1;
deref
d96c10f
to
f16d777
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 5 of 9 files reviewed, 2 unresolved discussions (waiting on @ilyalesokhin-starkware)
crates/cairo-lang-runnable-utils/src/builder.rs
line 425 at r1 (raw file):
Previously, ilyalesokhin-starkware wrote…
ret
is a bit confusing.
considerderef
orres
Done.
crates/cairo-lang-casm/src/hints/mod.rs
line 357 at r1 (raw file):
#[cfg_attr(feature = "parity-scale-codec", codec(index = 1))] WriteRunParam { index: ResOperand, dst: CellRef }, /// Writes a marker into the VM. Useful for debugging.
Done.
f16d777
to
c761a9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: 7 of 9 files reviewed, all discussions resolved
fddb99b
to
9e78ef6
Compare
c761a9f
to
7f623ad
Compare
Stack: