Add as_uninit-like methods to pointer types and unify documentation of as_ref methods by TimDiekmann · Pull Request #75392 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation87 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This adds a convenient method to retrieve a &(mut) [MaybeUninit<T>] from slice pointers (*const [T], *mut [T], NonNull<[T]>). See also rust-lang/wg-allocators#66 (comment).
I'll add a tracking issue as soon as it's reviewed and CI passed.
Tracking Issue: #75402
r? @RalfJung
Is there any particular reason why you added this for slices, but not for general pointers? It seems just as useful for *const T/*mut T/NonNull<T>.
That would would result in MaybeUninit<[T]> rather than [MaybeUninit<T>].
Maybe we could also add as_uninit for non-slices?
That would would result in MaybeUninit<[T]> rather than [MaybeUninit].
We probably want a method that converts between those two as well, but I don't know a good way to add such a method. :/
Maybe we could also add as_uninit for non-slices?
That's what I meant -- something like as_uninit/as_uninit_ref or so.
We probably want a method that converts between those two as well, but I don't know a good way to add such a method. :/
Me neither, we probably want a method converting in both direction with a decent name...
That's what I meant -- something like
as_uninit/as_uninit_refor so.
I'd name them as_uninit_ref to make them analogue to as_ref. Do we want another pull request for this or should I just add this here, gated behind as_uninit_ref? Should I add both to the same tracking issue?
I'd make them all one PR and one feature.
Currently we have NonNull::as_ref and NonNull::as_mut taking &(mut) self (both stabilized - sadly). Shall I also alter as_uninit_slice to take &self and returning &[MaybeUninit] and add as_uninit_slice_mut? Since NonNull implements Copy I don't like passing it as a reference, as you will almost never have a reference to NonNull, especially not a &mut NonNull.
This btw. also applies to as_mut_ptr which converts a slice to a non-slice pointer. On nightly there is MaybeUninit::first_ptr(_mut) which maybe could also be used on slice pointers instead. With this naming we could keep a single naming scheme.
Shall I also alter as_uninit_slice to take &self and returning &[MaybeUninit] and add as_uninit_slice_mut?
I can do technical reviews but this is a question for @rust-lang/libs that I cannot help with, I am afraid.
I'll first implement the methods as they were already implemented with the pointer types. Since some methods are already stable, I think that a unified schema is the most important thing.
I just noticed, that pointer::as_ref returns Option<&T>.
Add Add as_uninit_slice to slice pointer typesas_uninit-like methods to pointer types
Is there any particular reason why you added this for slices, but not for general pointers? It seems just as useful for
*const T/*mut T/NonNull<T>.
One thing to note: MaybeUninit<[T]> isn't even possible right now, as MaybeUninit<T> requires T to be sized.
Noticed that when I implemented as_uninit_ref/mut :D
Ah good point, and we are unlikely to be able to relax that sized bound.
Those intra-doc links drive me crazy :D
CI should pass now.
In terms of text this is all fine (assuming it's the same text for correspond methods, which I did not check in detail everywhere). I just have some more typographic nits.
Thank you!
I'll double-check every text and it's method again and will push it later this day, probably tonight.
Add Add as_uninit-like methods to pointer typesas_uninit-like methods to pointer types and unify documentation of as_ref methods
All right, I think this is ready. :) Could you squash the commits?
… of as_ref methods
Fix example in NonNull::as_uninit_slice
Rename feature gate to "ptr_as_uninit"
Make methods more consistent with already stable methods
Make pointer::as_uninit_slice return an Option
Fix placement for // SAFETY section
Add as_uninit_ref and as_uninit_mut to pointers
Fix doctest
Update tracking issue
Fix doc links
Apply suggestions from review
Make wording about counterparts consistent
Fix doc links
Improve documentation
Fix doc-tests
Fix doc links ... again
Apply suggestions from review
Apply suggestions from Review
Apply suggestion from review to all affected files
Add missing words in safety sections in as_uninit_slice_mut
Fix safety-comment in NonNull::as_uninit_slice_mut
Done (in case GH does not send mails on force-pushes).
Done (in case GH does not send mails on force-pushes).
(it doesn't, except sometimes when it does... their support told me it only sends them when the parent of the new HEAD was already in the branch, aka when you just amended the last commit.)
Great, and thanks for enduring by never-ending stream of editorial comments. :)
@bors r+ rollup
📌 Commit 93e074b has been approved by RalfJung
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
This comment has been minimized.
This comment has been minimized.
Great, and thanks for enduring by never-ending stream of editorial comments. :)
I think the effort was worth it :)
bors added a commit to rust-lang-ci/rust that referenced this pull request
Rollup of 8 pull requests
Successful merges:
- rust-lang#75389 (attempt to improve span_label docs)
- rust-lang#75392 (Add
as_uninit-like methods to pointer types and unify documentation ofas_refmethods) - rust-lang#75464 (Move to intra doc links for ascii.rs and panic.rs)
- rust-lang#75578 (Allowing raw ptr dereference in const fn)
- rust-lang#75613 (Add explanation for
&mut selfmethod call when expecting-> Self) - rust-lang#75626 (Clean up E0754 explanation)
- rust-lang#75629 (Use intra-doc links in
std::env,std::allocandstd::error) - rust-lang#75634 (Mark x86_64-linux-kernel as *)
Failed merges:
r? @ghost
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.