-
Notifications
You must be signed in to change notification settings - Fork 596
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
add support for match on none felt252 numeric values #4764
Conversation
4ebbd9e
to
cadb43d
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 3 files at r1, all commit messages.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)
crates/cairo-lang-lowering/src/lower/mod.rs
line 1223 at r1 (raw file):
let ty = matched_expr.ty(); if ty == ctx.db.core_felt252_ty() {
can you make sure (in a test) what happens when we have typed literals? e.g.
match 5_u32 {
0_felt252 => {}
_ => {}
}
cadb43d
to
f157aaa
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: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @orizi)
crates/cairo-lang-lowering/src/lower/mod.rs
line 1223 at r1 (raw file):
Previously, orizi wrote…
can you make sure (in a test) what happens when we have typed literals? e.g.
match 5_u32 { 0_felt252 => {} _ => {} }
Done.
f157aaa
to
0f35606
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 1 of 5 files at r2.
Reviewable status: 1 of 6 files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)
examples/fib_match.cairo
line 2 at r2 (raw file):
// Calculates fib... fn fib(n: u32) -> u32 {
revert to felt - this makes it much more difficult to read the resulting code.
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 1 of 5 files at r2.
Reviewable status: 1 of 6 files reviewed, 2 unresolved discussions (waiting on @TomerStarkware)
crates/cairo-lang-lowering/src/test_data/match
line 298 at r2 (raw file):
_ => 9,
Suggestion:
0_u8 => 7,
1_u8 | 2_u8 => 8,
_ => 9,
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: 1 of 6 files reviewed, 3 unresolved discussions (waiting on @TomerStarkware)
crates/cairo-lang-lowering/src/test_data/match
line 388 at r2 (raw file):
_ => 9,
.
Suggestion:
1_felt252 | 2_u8 => 8,
_ => 9,
0f35606
to
73fdb4e
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: 0 of 6 files reviewed, 3 unresolved discussions (waiting on @orizi)
crates/cairo-lang-lowering/src/test_data/match
line 298 at r2 (raw file):
_ => 9,
Done.
crates/cairo-lang-lowering/src/test_data/match
line 388 at r2 (raw file):
Previously, orizi wrote…
.
Done.
examples/fib_match.cairo
line 2 at r2 (raw file):
Previously, orizi wrote…
revert to felt - this makes it much more difficult to read the resulting code.
Done.
1a4a5fb
to
f531b16
Compare
73fdb4e
to
b2f1543
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 1 of 3 files at r1, 1 of 5 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 3 of 3 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)
crates/cairo-lang-lowering/src/lower/mod.rs
line 1590 at r4 (raw file):
let ret_ty = corelib::core_felt252_ty(ctx.db.upcast()); // TODO(TomerStarkware): use the same type of literal as the input, without the cast to // felt252.
Suggestion:
// TODO(TomerStarkware): Use the same type of literal as the input, without the cast to
// felt252.
b2f1543
to
5685902
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 6 files reviewed, 1 unresolved discussion (waiting on @orizi)
crates/cairo-lang-lowering/src/lower/mod.rs
line 1590 at r4 (raw file):
let ret_ty = corelib::core_felt252_ty(ctx.db.upcast()); // TODO(TomerStarkware): use the same type of literal as the input, without the cast to // felt252.
Done.
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 1 of 1 files at r6, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware)
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:
complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware)
73e212c
to
388d593
Compare
5685902
to
74d27f4
Compare
74d27f4
to
9752c24
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 r7, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware)
This change is