Skip to content
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

Tracking Issue for pointer methods returning MaybeUninit<T> #75402

Open
3 tasks
TimDiekmann opened this issue Aug 11, 2020 · 6 comments
Open
3 tasks

Tracking Issue for pointer methods returning MaybeUninit<T> #75402

TimDiekmann opened this issue Aug 11, 2020 · 6 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. S-tracking-design-concerns Status: There are blocking ❌ design concerns.

Comments

@TimDiekmann
Copy link
Member

TimDiekmann commented Aug 11, 2020

This is a tracking issue for methods on pointer types returning MaybeUninit: as_uninit_ref/mut and as_uninit_slice(_mut) on mutable and const raw pointers and NonNull.
The feature gate for the issue is #![feature(ptr_as_uninit)].

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also uses as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

Implementation history

@TimDiekmann TimDiekmann added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Aug 11, 2020
@RalfJung
Copy link
Member

A possible concern for stabilization is that these methods will be somewhat redundant if we end up deciding that references do not have to point to valid data -- though maybe reflecting the initialization state is a good idea even in that case? Also it seems likely that some T will not allow uninitialized references, i.e., there is demand for making &! uninhabited.

JohnTitor added a commit to JohnTitor/rust that referenced this issue Aug 18, 2020
…r=RalfJung

Add `as_uninit`-like methods to pointer types and unify documentation of `as_ref` methods

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: rust-lang#75402

r? @RalfJung
@Kixunil
Copy link
Contributor

Kixunil commented Feb 4, 2022

Having such cast on pointers rather than references could be useful in situations like in #93653 (although having cast on Box is even better in that case).

@RalfJung any good resource explaining why it'd be useful to allow uninitialized references? Anyway, I do like tracking (un)initializedness in type even for documentation value.

@RalfJung
Copy link
Member

RalfJung commented Feb 4, 2022

For documentation, it is the safety invariant of a type that is most relevant, and those certainly still apply recursively below references.

For the validity invariant, see rust-lang/unsafe-code-guidelines#77 for the discussion. Basically, specifying the language becomes a hell of a lot simpler if the validity invariant depends solely on the bit pattern of the value itself, and not on other state of the system such as the current contents of memory. I would argue that directly translates to making life easier for unsafe code authors (as one of the consumers of such a specification). Also generally I think we should require reasons to make something UB, the default should be not-UB. :)

@Raekye
Copy link
Contributor

Raekye commented Apr 16, 2022

I noticed that as_uninit_* on pointers take self by value (and pointers are Copy), e.g. see as_uninit_mut.

However, on NonNull, these functions take self by reference, e.g. see the function with the same name by for NonNull: as_uninit_mut takes self by mutable reference. Even more inconsistent, as_uninit_slice_mut returns a mutable reference, but takes self by immutable reference.

I think these methods should take self by value for consistency. The returned lifetime is unbounded anyways and not tied to the pointer/NonNull value anyways

@RalfJung
Copy link
Member

RalfJung commented Apr 16, 2022

NonNull::as_ref takes &self. It would seem odd for NonNull::as_uninit_ref to diverge from that?

bors added a commit to rust-lang-ci/rust that referenced this issue May 23, 2022
Change `NonNull::as_uninit_*` to take self by value (as opposed to reference), matching primitive pointers.

Copied from my comment on [rust-lang#75402](rust-lang#75402 (comment)):

> I noticed that `as_uninit_*` on pointers take `self` by value (and pointers are `Copy`), e.g. see [`as_uninit_mut`](https://doc.rust-lang.org/core/primitive.pointer.html#method.as_uninit_mut).
>
> However, on `NonNull`, these functions take `self` by reference, e.g. see the function with the same name by for `NonNull`: [`as_uninit_mut`](https://doc.rust-lang.org/std/ptr/struct.NonNull.html#method.as_uninit_mut) takes `self` by mutable reference. Even more inconsistent, [`as_uninit_slice_mut`](https://doc.rust-lang.org/std/ptr/struct.NonNull.html#method.as_uninit_slice_mut) returns a mutable reference, but takes `self` by immutable reference.
>
> I think these methods should take `self` by value for consistency. The returned lifetime is unbounded anyways and not tied to the pointer/NonNull value anyways

I realized the change is trivial (if desired) so here I am creating my first PR. I think it's not a breaking change since (it's on nightly and) `NonNull` is `Copy`; all previous usages of these methods taking `self` by reference should continue to compile. However, it might cause warnings to appear on usages of `NonNull::as_uninit_mut`, which used to require the the `NonNull` variable be declared `mut`, but now it's not necessary.
workingjubilee pushed a commit to tcdi/postgrestd that referenced this issue Sep 15, 2022
Change `NonNull::as_uninit_*` to take self by value (as opposed to reference), matching primitive pointers.

Copied from my comment on [#75402](rust-lang/rust#75402 (comment)):

> I noticed that `as_uninit_*` on pointers take `self` by value (and pointers are `Copy`), e.g. see [`as_uninit_mut`](https://doc.rust-lang.org/core/primitive.pointer.html#method.as_uninit_mut).
>
> However, on `NonNull`, these functions take `self` by reference, e.g. see the function with the same name by for `NonNull`: [`as_uninit_mut`](https://doc.rust-lang.org/std/ptr/struct.NonNull.html#method.as_uninit_mut) takes `self` by mutable reference. Even more inconsistent, [`as_uninit_slice_mut`](https://doc.rust-lang.org/std/ptr/struct.NonNull.html#method.as_uninit_slice_mut) returns a mutable reference, but takes `self` by immutable reference.
>
> I think these methods should take `self` by value for consistency. The returned lifetime is unbounded anyways and not tied to the pointer/NonNull value anyways

I realized the change is trivial (if desired) so here I am creating my first PR. I think it's not a breaking change since (it's on nightly and) `NonNull` is `Copy`; all previous usages of these methods taking `self` by reference should continue to compile. However, it might cause warnings to appear on usages of `NonNull::as_uninit_mut`, which used to require the the `NonNull` variable be declared `mut`, but now it's not necessary.
@pnkfelix
Copy link
Member

pnkfelix commented Sep 23, 2022

Visiting for T-compiler backlog bonanza

@RalfJung 's comments above were somewhat echo'ed by @m-ou-se in the meeting

@rustbot label: S-tracking-design-concerns

@rustbot rustbot added the S-tracking-design-concerns Status: There are blocking ❌ design concerns. label Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. S-tracking-design-concerns Status: There are blocking ❌ design concerns.
Projects
None yet
Development

No branches or pull requests

6 participants