-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add MaybeUninit support #16
Conversation
Okay, all CI builds and tests pass now. :) The 1.15.0 warning about |
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.
Thanks! Could you explain a bit about the motivation for this? Is there a concern about correctness of our use of mem::uninitialized, or is this more just because mem::uninitialized is deprecated now?
Both!
|
Ah I didn't know about this. Could you share a link to where you found this? |
It was attempted in rust-lang/rust#62150, and then backed out in rust-lang/rust#63343, as the new |
Those implement |
No, in general we don't remove previously stable things from the standard library. Deprecation means there is something else you should probably use instead for new code, but existing code needs to continue to compile. Deprecation does not mean something is scheduled for removal. |
Published in 1.0.1. 🍻 |
Hello!
This PR adds use of
MaybeUninit
everywhere thatmem::uninitialized
was previously used.The first commit does it in a breaking way, and the second commit only does it on Rust 1.36+, where
MaybeUninit
is stable, falling back onmem::uninitialized
. I'll squash them, but I find it more readable to be able to see them separately for review purposes.I'm not sure if I've done it 100% correctly, so I'd appreciate a review. It did require a large helping of
#[cfg]
, but I'm not sure how else to handle it.Tests all pass and benchmarks appear unaffected.
I have a few concerns:
MaybeUninit
specially, meaning I'm not dropping it. I'm fairly certain that this is fine because it uses primitive types and doesn't allocate on the heap.f.write_to_ryu_buffer
? If so, no problem, but if not, this accesses uninitialised memory.Thanks!
(Edit: Sorry for all the pushes. 1.15.0 won't update the registry on my laptop at home for some reason.)