-
Notifications
You must be signed in to change notification settings - Fork 518
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
Made non-copy variable not be forwarded in const folding. #6324
Conversation
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 2 files reviewed, 1 unresolved discussion (waiting on @ilyalesokhin-starkware)
crates/cairo-lang-lowering/src/optimizations/test_data/const_folding
line 3019 at r1 (raw file):
(v3: (core::felt252, core::dict::Felt252Dict::<core::felt252>), v4: @(core::felt252, core::dict::Felt252Dict::<core::felt252>)) <- snapshot(v2) (v5: core::felt252, v6: core::dict::Felt252Dict::<core::felt252>) <- struct_destructure(v3) (v7: core::dict::SquashedFelt252Dict::<core::felt252>) <- core::dict::Felt252DictImpl::<core::felt252, core::Felt252Felt252DictValue>::squash(v6)
before the fix.
Suggestion:
(v7: core::dict::SquashedFelt252Dict::<core::felt252>) <- core::dict::Felt252DictImpl::<core::felt252, core::Felt252Felt252DictValue>::squash(v1)
commit-id:3e6f21de
commit-id:7ba5d6ef
c96d9ab
to
c1f8b63
Compare
is this comment still relevent? Code quote: /// Note: We don't keep track of types as we assume the usage is always correct. |
? Code quote: building constructing |
Maybe None or undefined? Code quote: Placeholder |
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 2 files reviewed, 4 unresolved discussions (waiting on @ilyalesokhin-starkware)
crates/cairo-lang-lowering/src/optimizations/const_folding.rs
line 29 at r2 (raw file):
Previously, ilyalesokhin-starkware wrote…
is this comment still relevent?
it didn't use to be - but it is now (since we added the snapshots)
crates/cairo-lang-lowering/src/optimizations/const_folding.rs
line 40 at r2 (raw file):
Previously, ilyalesokhin-starkware wrote…
?
Done.
crates/cairo-lang-lowering/src/optimizations/const_folding.rs
line 41 at r2 (raw file):
Previously, ilyalesokhin-starkware wrote…
Maybe None or undefined?
Done.
c1f8b63
to
bd9d06f
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 2 files reviewed, 3 unresolved discussions (waiting on @orizi)
crates/cairo-lang-lowering/src/optimizations/const_folding.rs
line 40 at r2 (raw file):
Previously, orizi wrote…
Done.
fix comment as well.
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 2 files reviewed, 3 unresolved discussions (waiting on @ilyalesokhin-starkware)
crates/cairo-lang-lowering/src/optimizations/const_folding.rs
line 40 at r2 (raw file):
Previously, ilyalesokhin-starkware wrote…
fix comment as well.
Done.
bd9d06f
to
f02212a
Compare
consider using option instead of untracked. Suggestion: Struct(Vec<Option<VarInfo>>), |
Suggestion: Enables building VarInfo::Structs where some of the members are untracked. |
Fixes #6321. commit-id:e27aec6c
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 2 files reviewed, 3 unresolved discussions (waiting on @ilyalesokhin-starkware)
crates/cairo-lang-lowering/src/optimizations/const_folding.rs
line 39 at r4 (raw file):
Previously, ilyalesokhin-starkware wrote…
consider using option instead of untracked.
Done.
crates/cairo-lang-lowering/src/optimizations/const_folding.rs
line 40 at r4 (raw file):
/// The variable is a struct of other variables. Struct(Vec<VarInfo>), /// A marker for inner `VarInfo`s to be untracked - enables building recursive `VarInfo`s
No longer relevant.
f02212a
to
7567292
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 1 files at r5.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion
Fixes #6321.
Stack:
mapped_text
based onInto<String>
. #6313This change is