-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Some miri cleanups #46743
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
Some miri cleanups #46743
Conversation
let (ptr, ty) = ptr_ty?; | ||
let ecx = mk_eval_cx(tcx, instance.substs, param_env); |
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.
Is the new substs
field on EvalContext
just for this?
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.
jup. We used to push a stackframe just to have substs.
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.
I removed the substs part and just simplified the interface
47dc339
to
0d68080
Compare
Err(err) | ||
}, | ||
((Err(_), _), Err(err)) => Err(err), | ||
((Ok((miri_val, miri_ty)), mut ecx), Ok(ctfe)) => { | ||
(Err(_), Err(err)) => Err(err), |
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.
Heh strange. Why these two arms are not merged 😄
(_, Err(err)) => Err(err),
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.
Because the next commit will unmerge them because it's going to process the miri result ;)
✌️ @oli-obk can now approve this pull request |
@bors r=eddyb |
📌 Commit 2d161f1 has been approved by |
☀️ Test successful - status-appveyor, status-travis |
r? @eddyb