rust
rust copied to clipboard
Nuke slice_as{,_mut}_ptr methods
These methods originate from the MaybeUninit RFC and were provided without justification. They are objectively bad because they misunderstand the point of having a slice of MaybeUninits: you're supposed to use the safer MaybeUninit methods instead of working with raw pointers. The proof of this misunderstanding is evidenced by the fixed usages in the stdlib from this PR: many usages work with unsafe raw pointers completely unnecessarily. It possible that raw pointers were used to avoid bounds checking performance hits, but minimal evidence of this being the case was found.
This PR adds MaybeUninit<T> pointer to T pointer conversions to guarantee the safety of pointer casts where necessary. These should be From impls (unless it is decided that the dangers of having a raw pointer to potentially uninitialized memory warrants a separate method), but aren't because I don't want to insta-stable. These impls could also not be added, but I worry about the possibility of incorrect casts.
Hey! It looks like you've submitted a new PR for the library teams!
If this PR contains changes to any rust-lang/rust
public library APIs then please comment with @rustbot label +T-libs-api -T-libs
to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.
Examples of T-libs-api
changes:
- Stabilizing library features
- Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
- Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
- Changing public documentation in ways that create new stability guarantees
- Changing observable runtime behavior of library APIs
r? @m-ou-se
(rust-highfive has picked a reviewer for you, use r? to override)
r? @RalfJung
The job mingw-check
failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
configure: rust.debug-assertions := True
configure: rust.overflow-checks := True
configure: llvm.assertions := True
configure: dist.missing-tools := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure:
configure: run `python /checkout/x.py --help`
Attempting with retry: make prepare
---
* highest error code: E0790
Found 506 error codes
Found 0 error(s) in error codes
Done!
tidy error: /checkout/library/core/src/fmt/num.rs:397: undocumented unsafe
tidy error: /checkout/library/core/src/fmt/num.rs:400: undocumented unsafe
tidy error: /checkout/library/core/src/mem/maybe_uninit.rs:1309: TODO is deprecated; use FIXME
some tidy checks failed
Build completed unsuccessfully in 0:00:09
I'm not a libs API person, so I'm the wrong reviewer here. r? libs
Right, keep forgetting, sorry! :)
If you have to add a completely new method to actually replace these methods, then maybe they aren't ready for removal imo. You should add the new method through the correct processes first, and then we can maybe remove these later.
Also I agree with Ralf that these should not be From impls at all.
Agreed. See https://github.com/rust-lang/libs-team/issues/122#issuecomment-1281408047. Let's let this sit until the other MaybeUninit stuff is resolved and I'll give this some more thought.
@rustbot author