use MaybeUninit instead of mem::uninitialized for Windows Mutex #56275
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
src/libstd/sys/windows/mutex.rs
Outdated
| pub unsafe fn uninitialized() -> ReentrantMutex { | ||
| mem::uninitialized() | ||
| pub fn uninitialized() -> ReentrantMutex { | ||
| MaybeUninit::uninitialized() |
There was a problem hiding this comment.
Looks like you forgot to wrap this in ReentrantMutex.
There was a problem hiding this comment.
I forgot indeed, thanks!
This comment has been minimized.
This comment has been minimized.
|
FWIW, you should be able to run the mingw-check builder locally ( Code-wise this seems fine but someone else should probably review as well as I'm not up to date on |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@SimonSapin could you have a look? |
src/libcore/fmt/float.rs
Outdated
| @@ -22,6 +22,9 @@ fn float_to_decimal_common_exact<T>(fmt: &mut Formatter, num: &T, | |||
| unsafe { | |||
| let mut buf = MaybeUninit::<[u8; 1024]>::uninitialized(); // enough for f32 and f64 | |||
| let mut parts = MaybeUninit::<[flt2dec::Part; 4]>::uninitialized(); | |||
| // FIXME: Technically, this is calling `get_mut` on an uninitialized | |||
There was a problem hiding this comment.
Maybe file an issue about this specifically, and mention that issue number in code comments for all known occurences? That way when the question is resolved we can grep for the issue number to find places to fix (or to remove outdated FIXME comments).
There was a problem hiding this comment.
Wouldn't this usually be part of the discussion in the tracking issue?
There was a problem hiding this comment.
Ok, the tracking issue number could serve as well. My comment was more about having something to grep for to find known occurrences.
There was a problem hiding this comment.
I agree. Is like this okay? This covers all get_mut uses that got added as part of my PR series. I grepped the codebase for other uses of MaybeUninit and found none.
|
Looks good, thanks! @bors r+ |
|
📌 Commit ebe69c0 has been approved by |
bors
added
S-waiting-on-bors
use MaybeUninit instead of mem::uninitialized for Windows Mutex I hope this builds, I do not have a Windows machine to test...
|
☀️ Test successful - status-appveyor, status-travis |
I hope this builds, I do not have a Windows machine to test...