-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
miri engine: Hooks for basic stacked borrows #55125
Conversation
@bors r+ |
📌 Commit 5b4c5de5f3c4a537d5dd8fc074e5619a47fd158c has been approved by |
21f80da
to
5b4c5de
Compare
@bors r=oli-obk (I had pushed something here but force-pushed it away again.) |
📌 Commit 5b4c5de5f3c4a537d5dd8fc074e5619a47fd158c has been approved by |
@oli-obk I pushed some more commits, matching the |
This comment has been minimized.
This comment has been minimized.
src/librustc_mir/interpret/cast.rs
Outdated
@@ -47,7 +46,19 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> | |||
|
|||
Misc => { | |||
let src = self.read_value(src)?; | |||
if self.type_is_fat_ptr(src_layout.ty) { | |||
|
|||
if src.layout.ty.is_region_ptr() && dest.layout.ty.is_unsafe_ptr() { |
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.
slightly unhappy about code that isn't needed during const eval but still will be compiled into const eval. Not sure what to do about that though
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 cannot make it depend on enforce_validity()
as that whitelists some functions in miri and I think my model might go crazy if references are only sometimes tracked.
I could add another const REF_TRACKING_HOOKS: bool
though.
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.
Good idea, not a blocker for this PR though
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'll forget it if I don't do it now, and this PR is still quite far up the queue anyway.^^
I might have to include a miri module update as well as we are nearing the week where we cannot break tools.
@bors r+ |
📌 Commit eb5c69b33c30bca5ebb954caae337c33748368eb has been approved by |
@bors r+ |
💡 This pull request was already approved, no need to approve it again.
|
📌 Commit eb5c69b33c30bca5ebb954caae337c33748368eb has been approved by |
Fixed your nit, and included rust-lang/miri#487 so that the miri submodule keeps working. |
@bors r+ |
📌 Commit 4e784583aab5ec6ff083e47cb0c3d7fa3518d330 has been approved by |
☔ The latest upstream changes (presumably #55171) made this pull request unmergeable. Please resolve the merge conflicts. |
4e78458
to
9282fd3
Compare
📌 Commit 50d5cde2dd6b1ae80c170ff873453699d061278b has been approved by |
⌛ Testing commit 50d5cde2dd6b1ae80c170ff873453699d061278b with merge c90f034f45dff5d7d4865259634139ca1cc03bf1... |
This comment has been minimized.
This comment has been minimized.
50d5cde
to
3f5b550
Compare
Fixed by disabling a new miri test on Windows and macOS, where I cannot test them. This can be much easier debugged on the miri side once things landed in rustc. @bors r=oli-obk |
📌 Commit 3f5b550 has been approved by |
⌛ Testing commit 3f5b550 with merge 3779b93f1bdcfa9c7ded680a7b2d715a24c688da... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
miri engine: Hooks for basic stacked borrows r? @oli-obk
☀️ Test successful - status-appveyor, status-travis |
r? @oli-obk