-
Notifications
You must be signed in to change notification settings - Fork 580
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
no snapshot peeling from type_enriched_members key. #7229
Conversation
Before the fix. because it assumed @s3 has the same deref as S3. Suggestion:
|
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 all commit messages.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @ilyalesokhin-starkware)
crates/cairo-lang-semantic/src/expr/compute.rs
line 3073 at r1 (raw file):
ctx.resolver.inference().solve().ok(); ctx.resolver.inference().internal_rewrite(&mut long_ty).no_err(); let members =
shouldn't the key be rewritten as well?
Code quote:
let (_, mut long_ty) = peel_snapshots(ctx.db, expr.ty());
// Save some work. ignore the result. The error, if any, will be reported later.
ctx.resolver.inference().solve().ok();
ctx.resolver.inference().internal_rewrite(&mut long_ty).no_err();
let members =
Previously, orizi wrote…
I don't now if you are more likely to look for the original key or the rewritten key. |
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 2 files at r1.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @ilyalesokhin-starkware)
crates/cairo-lang-semantic/src/expr/compute.rs
line 3073 at r1 (raw file):
Previously, ilyalesokhin-starkware wrote…
I don't now if you are more likely to look for the original key or the rewritten key.
probably the rewritten - as you'd probably have the most "solved" possible value for this lookup.
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 2 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @ilyalesokhin-starkware)
02a63dd
to
cde0273
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: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @orizi)
crates/cairo-lang-semantic/src/expr/compute.rs
line 3073 at r1 (raw file):
Previously, orizi wrote…
probably the rewritten - as you'd probably have the most "solved" possible value for this lookup.
Done.
cde0273
to
58f8801
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 r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @gilbens-starkware)
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 r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware)
58f8801
to
f50a0c2
Compare
f50a0c2
to
d7a5797
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 2 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware)
No description provided.