-
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
Removed uninitialized_statics
field from Memory
struct in miri
#51428
Removed uninitialized_statics
field from Memory
struct in miri
#51428
Conversation
78469f9
to
d622eb0
Compare
@oli-obk All tests are passing locally, just waiting on Travis here. See my message on Discord re |
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 |
Could you also check how much of an impact this has on the miri submodule? |
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.
That was less fallout than I feared!
src/librustc_mir/interpret/memory.rs
Outdated
@@ -396,26 +376,23 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { | |||
MemoryKind::Machine(m) => format!(" ({:?})", m), | |||
}), | |||
// uninitialized static alloc? |
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.
leftover comment
src/librustc_mir/interpret/memory.rs
Outdated
Some(kind @ MemoryKind::Stack) | | ||
Some(kind @ MemoryKind::Machine(_)) => { | ||
kind @ MemoryKind::Stack | | ||
kind @ MemoryKind::Machine(_) => { | ||
self.alloc_map.insert(id, alloc); |
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 think these two maps can now be merged into one
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.
Oops, yep.
How do you mean, exactly? |
What's the Travis test failure due to, do you think? Not happening on my machine. |
d622eb0
to
8544115
Compare
Well... I guess we first need to fix miri on nightly before it would make sense to adjust it to the changes of this PR |
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 |
Not sure. Can you provide a |
@oli-obk The test passes locally though so... |
Oh, the errors are to do with |
Blocked on #51110. |
8544115
to
a254347
Compare
This comment has been minimized.
This comment has been minimized.
Err, looks like a transitory issue with CI. |
Manually restarted the build. |
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 |
Okay, that’s the same error I’m getting in the PR blocking this. Looks like something changed in master with the rebase. |
☔ The latest upstream changes (presumably #51884) made this pull request unmergeable. Please resolve the merge conflicts. |
a254347
to
6c22f1c
Compare
@oli-obk Okay, I've rebased over master now that #51110 has merged. I think we're ready to merge this too now! @pietroalbini I don't know if you need to remove the |
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 |
6c22f1c
to
d610840
Compare
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 |
use rustc::mir::interpret::FrameInfo; | ||
use syntax::codemap::{self, Span}; | ||
use syntax::ast::Mutability; | ||
||||||| parent of a2543471ec... Removed `uninitialized_statics` field from `Memory` struct in miri. |
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.
rebase fallout
src/librustc_mir/interpret/memory.rs
Outdated
use syntax::ast::Mutability; | ||
|
||
use rustc_data_structures::fx::{FxHashSet, FxHashMap}; | ||
||||||| parent of a2543471ec... Removed `uninitialized_statics` field from `Memory` struct in miri. |
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.
more rebase fallout
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 fixed this though! WTF?
d610840
to
505ac90
Compare
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 |
505ac90
to
16984ed
Compare
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 |
Refactored code accordingly.
16984ed
to
ea806e7
Compare
@bors r+ |
📌 Commit 6660c25 has been approved by |
…ification, r=oli-obk Removed `uninitialized_statics` field from `Memory` struct in miri **IMPORTANT: Do not merge until rust-lang#51110 is merged** r? @oli-obk CC @eddyb
☀️ Test successful - status-appveyor, status-travis |
Tested on commit rust-lang/rust@81d5c3e. Direct link to PR: <rust-lang/rust#51428> 🎉 miri on windows: build-fail → test-pass. 🎉 miri on linux: build-fail → test-pass.
based on #51110
r? @oli-obk
CC @eddyb