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 pin_deref_mut #86918

Closed
2 of 3 tasks
jonhoo opened this issue Jul 6, 2021 · 12 comments · Fixed by #129424
Closed
2 of 3 tasks

Tracking Issue for pin_deref_mut #86918

jonhoo opened this issue Jul 6, 2021 · 12 comments · Fixed by #129424
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@jonhoo
Copy link
Contributor

jonhoo commented Jul 6, 2021

Feature gate: #![feature(pin_deref_mut)]

This is a tracking issue for Pin::as_deref_mut.

The feature adds the method as_deref_mut to Pin, which allow a safe transformation from a Pin<&mut Pin<P>> to a Pin<&mut P::Target>.

Public API

impl<Ptr> Pin<Ptr> {
    pub fn as_deref_mut(self: Pin<&mut Pin<Ptr>>) -> Pin<&mut Ptr::Target>
    where
        Ptr: DerefMut;
}

Steps / History

Unresolved Questions

  • None yet.
@jonhoo jonhoo added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 6, 2021
@RalfJung

This comment was marked as resolved.

@jonhoo

This comment was marked as resolved.

@coolreader18
Copy link
Contributor

Are there any remaining blockers on this? It's a pretty useful function for blanket impls for traits with poll methods (e.g. Future already uses it in its impl<P> Future for Pin<P> impl).

@jonhoo
Copy link
Contributor Author

jonhoo commented Nov 18, 2023

Not that I know of — given it touches the Pin machinery, I suspect it should go through an actual FCP, but I'll let others be the judge of that :)

@dtolnay
Copy link
Member

dtolnay commented Jun 25, 2024

Currently this method is located in its own solitary impl block:

impl<'a, Ptr: DerefMut> Pin<&'a mut Pin<Ptr>> {
    pub fn as_deref_mut(self) -> Pin<&'a mut Ptr::Target>;
}

I'd like to consider changing it to:

impl<Ptr> Pin<Ptr> {
    pub fn as_deref_mut(self: Pin<&mut Pin<Ptr>>) -> Pin<&mut Ptr::Target>
    where
        Ptr: DerefMut;
}

Some advantages:

  • Synergy with the existing as_ref and as_mut signatures (stable since Rust 1.33)
  • Lifetime elision reduces noise in the signature
  • Turbofish less verbose: Pin::<&mut T>::as_deref_mut vs Pin::<&mut Pin<&mut T>>::as_deref_mut
impl<Ptr> Pin<Ptr> {
    pub fn as_ref(&self) -> Pin<&Ptr::Target>
    where
        Ptr: Deref;

    pub fn as_mut(&mut self) -> Pin<&mut Ptr::Target>
    where
        Ptr: DerefMut;
}

@thynson
Copy link

thynson commented Jul 11, 2024

Any concern blocks this from stablization?

@jonhoo
Copy link
Contributor Author

jonhoo commented Jul 15, 2024

@dtolnay I agree — that is better 👍

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Aug 24, 2024
…ature, r=dtolnay

Put Pin::as_deref_mut in impl Pin<Ptr> / rearrange Pin methods

Tracking issue: rust-lang#86918

Based on the suggestion in rust-lang#86918 (comment)

> Some advantages:
>
>     * Synergy with the existing `as_ref` and `as_mut` signatures (stable since Rust 1.33)
>
>     * Lifetime elision reduces noise in the signature
>
>     * Turbofish less verbose: `Pin::<&mut T>::as_deref_mut` vs `Pin::<&mut Pin<&mut T>>::as_deref_mut`

The comment seemed to imply that `Pin::as_ref` and `Pin::as_mut` already share an impl block, which they don't. So, I rearranged it so that they do, and we can see which looks better in the docs.

<details><summary><b>Docs screenshots</b></summary>

Current nightly:
![image](https://github.com/user-attachments/assets/b432cb82-8f4b-48ae-bafc-2fe49d0ad48c)

`Pin::as_deref_mut` moved into the same block as `as_mut`:
![image](https://github.com/user-attachments/assets/f9b35722-6a88-4465-ad1c-28d8e91902ac)

`Pin::as_ref`, `as_mut`, and `as_deref_mut` all in the same block:
![image](https://github.com/user-attachments/assets/9a1b2bf0-70a6-4751-b13f-390f1d575244)

</details>

I think I like the last one the most; obviously I'm biased since I'm the one who rearranged it, but it doesn't make sense to me to have `as_ref` methods split up by an `into_inner` method.

r? dtolnay
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Aug 24, 2024
…ature, r=dtolnay

Put Pin::as_deref_mut in impl Pin<Ptr> / rearrange Pin methods

Tracking issue: rust-lang#86918

Based on the suggestion in rust-lang#86918 (comment)

> Some advantages:
>
>     * Synergy with the existing `as_ref` and `as_mut` signatures (stable since Rust 1.33)
>
>     * Lifetime elision reduces noise in the signature
>
>     * Turbofish less verbose: `Pin::<&mut T>::as_deref_mut` vs `Pin::<&mut Pin<&mut T>>::as_deref_mut`

The comment seemed to imply that `Pin::as_ref` and `Pin::as_mut` already share an impl block, which they don't. So, I rearranged it so that they do, and we can see which looks better in the docs.

<details><summary><b>Docs screenshots</b></summary>

Current nightly:
![image](https://github.com/user-attachments/assets/b432cb82-8f4b-48ae-bafc-2fe49d0ad48c)

`Pin::as_deref_mut` moved into the same block as `as_mut`:
![image](https://github.com/user-attachments/assets/f9b35722-6a88-4465-ad1c-28d8e91902ac)

`Pin::as_ref`, `as_mut`, and `as_deref_mut` all in the same block:
![image](https://github.com/user-attachments/assets/9a1b2bf0-70a6-4751-b13f-390f1d575244)

</details>

I think I like the last one the most; obviously I'm biased since I'm the one who rearranged it, but it doesn't make sense to me to have `as_ref` methods split up by an `into_inner` method.

r? dtolnay
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Aug 24, 2024
…ature, r=dtolnay

Put Pin::as_deref_mut in impl Pin<Ptr> / rearrange Pin methods

Tracking issue: rust-lang#86918

Based on the suggestion in rust-lang#86918 (comment)

> Some advantages:
>
>     * Synergy with the existing `as_ref` and `as_mut` signatures (stable since Rust 1.33)
>
>     * Lifetime elision reduces noise in the signature
>
>     * Turbofish less verbose: `Pin::<&mut T>::as_deref_mut` vs `Pin::<&mut Pin<&mut T>>::as_deref_mut`

The comment seemed to imply that `Pin::as_ref` and `Pin::as_mut` already share an impl block, which they don't. So, I rearranged it so that they do, and we can see which looks better in the docs.

<details><summary><b>Docs screenshots</b></summary>

Current nightly:
![image](https://github.com/user-attachments/assets/b432cb82-8f4b-48ae-bafc-2fe49d0ad48c)

`Pin::as_deref_mut` moved into the same block as `as_mut`:
![image](https://github.com/user-attachments/assets/f9b35722-6a88-4465-ad1c-28d8e91902ac)

`Pin::as_ref`, `as_mut`, and `as_deref_mut` all in the same block:
![image](https://github.com/user-attachments/assets/9a1b2bf0-70a6-4751-b13f-390f1d575244)

</details>

I think I like the last one the most; obviously I'm biased since I'm the one who rearranged it, but it doesn't make sense to me to have `as_ref` methods split up by an `into_inner` method.

r? dtolnay
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 24, 2024
…ature, r=dtolnay

Put Pin::as_deref_mut in impl Pin<Ptr> / rearrange Pin methods

Tracking issue: rust-lang#86918

Based on the suggestion in rust-lang#86918 (comment)

> Some advantages:
>
>     * Synergy with the existing `as_ref` and `as_mut` signatures (stable since Rust 1.33)
>
>     * Lifetime elision reduces noise in the signature
>
>     * Turbofish less verbose: `Pin::<&mut T>::as_deref_mut` vs `Pin::<&mut Pin<&mut T>>::as_deref_mut`

The comment seemed to imply that `Pin::as_ref` and `Pin::as_mut` already share an impl block, which they don't. So, I rearranged it so that they do, and we can see which looks better in the docs.

<details><summary><b>Docs screenshots</b></summary>

Current nightly:
![image](https://github.com/user-attachments/assets/b432cb82-8f4b-48ae-bafc-2fe49d0ad48c)

`Pin::as_deref_mut` moved into the same block as `as_mut`:
![image](https://github.com/user-attachments/assets/f9b35722-6a88-4465-ad1c-28d8e91902ac)

`Pin::as_ref`, `as_mut`, and `as_deref_mut` all in the same block:
![image](https://github.com/user-attachments/assets/9a1b2bf0-70a6-4751-b13f-390f1d575244)

</details>

I think I like the last one the most; obviously I'm biased since I'm the one who rearranged it, but it doesn't make sense to me to have `as_ref` methods split up by an `into_inner` method.

r? dtolnay
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 24, 2024
…ature, r=dtolnay

Put Pin::as_deref_mut in impl Pin<Ptr> / rearrange Pin methods

Tracking issue: rust-lang#86918

Based on the suggestion in rust-lang#86918 (comment)

> Some advantages:
>
>     * Synergy with the existing `as_ref` and `as_mut` signatures (stable since Rust 1.33)
>
>     * Lifetime elision reduces noise in the signature
>
>     * Turbofish less verbose: `Pin::<&mut T>::as_deref_mut` vs `Pin::<&mut Pin<&mut T>>::as_deref_mut`

The comment seemed to imply that `Pin::as_ref` and `Pin::as_mut` already share an impl block, which they don't. So, I rearranged it so that they do, and we can see which looks better in the docs.

<details><summary><b>Docs screenshots</b></summary>

Current nightly:
![image](https://github.com/user-attachments/assets/b432cb82-8f4b-48ae-bafc-2fe49d0ad48c)

`Pin::as_deref_mut` moved into the same block as `as_mut`:
![image](https://github.com/user-attachments/assets/f9b35722-6a88-4465-ad1c-28d8e91902ac)

`Pin::as_ref`, `as_mut`, and `as_deref_mut` all in the same block:
![image](https://github.com/user-attachments/assets/9a1b2bf0-70a6-4751-b13f-390f1d575244)

</details>

I think I like the last one the most; obviously I'm biased since I'm the one who rearranged it, but it doesn't make sense to me to have `as_ref` methods split up by an `into_inner` method.

r? dtolnay
tgross35 added a commit to tgross35/rust that referenced this issue Aug 25, 2024
…ature, r=dtolnay

Put Pin::as_deref_mut in impl Pin<Ptr> / rearrange Pin methods

Tracking issue: rust-lang#86918

Based on the suggestion in rust-lang#86918 (comment)

> Some advantages:
>
>  * Synergy with the existing `as_ref` and `as_mut` signatures (stable since Rust 1.33)
>
>  * Lifetime elision reduces noise in the signature
>
>  * Turbofish less verbose: `Pin::<&mut T>::as_deref_mut` vs `Pin::<&mut Pin<&mut T>>::as_deref_mut`

The comment seemed to imply that `Pin::as_ref` and `Pin::as_mut` already share an impl block, which they don't. So, I rearranged it so that they do, and we can see which looks better in the docs.

<details><summary><b>Docs screenshots</b></summary>

Current nightly:
![image](https://github.com/user-attachments/assets/b432cb82-8f4b-48ae-bafc-2fe49d0ad48c)

`Pin::as_deref_mut` moved into the same block as `as_mut`:
![image](https://github.com/user-attachments/assets/f9b35722-6a88-4465-ad1c-28d8e91902ac)

`Pin::as_ref`, `as_mut`, and `as_deref_mut` all in the same block:
![image](https://github.com/user-attachments/assets/9a1b2bf0-70a6-4751-b13f-390f1d575244)

</details>

I think I like the last one the most; obviously I'm biased since I'm the one who rearranged it, but it doesn't make sense to me to have `as_ref` methods split up by an `into_inner` method.

r? dtolnay
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Aug 25, 2024
Rollup merge of rust-lang#129449 - coolreader18:pin-as_deref_mut-signature, r=dtolnay

Put Pin::as_deref_mut in impl Pin<Ptr> / rearrange Pin methods

Tracking issue: rust-lang#86918

Based on the suggestion in rust-lang#86918 (comment)

> Some advantages:
>
>  * Synergy with the existing `as_ref` and `as_mut` signatures (stable since Rust 1.33)
>
>  * Lifetime elision reduces noise in the signature
>
>  * Turbofish less verbose: `Pin::<&mut T>::as_deref_mut` vs `Pin::<&mut Pin<&mut T>>::as_deref_mut`

The comment seemed to imply that `Pin::as_ref` and `Pin::as_mut` already share an impl block, which they don't. So, I rearranged it so that they do, and we can see which looks better in the docs.

<details><summary><b>Docs screenshots</b></summary>

Current nightly:
![image](https://github.com/user-attachments/assets/b432cb82-8f4b-48ae-bafc-2fe49d0ad48c)

`Pin::as_deref_mut` moved into the same block as `as_mut`:
![image](https://github.com/user-attachments/assets/f9b35722-6a88-4465-ad1c-28d8e91902ac)

`Pin::as_ref`, `as_mut`, and `as_deref_mut` all in the same block:
![image](https://github.com/user-attachments/assets/9a1b2bf0-70a6-4751-b13f-390f1d575244)

</details>

I think I like the last one the most; obviously I'm biased since I'm the one who rearranged it, but it doesn't make sense to me to have `as_ref` methods split up by an `into_inner` method.

r? dtolnay
@coolreader18
Copy link
Contributor

Would it be possible to start an FCP on this?

@dtolnay
Copy link
Member

dtolnay commented Aug 27, 2024

@rust-lang/libs-api:
@rfcbot fcp merge

.as_deref_mut() is like .get_mut().as_mut(), but without enforcing an unnecessary Ptr: Unpin.
It is like unsafe { .get_unchecked_mut() }.as_mut(), but is unconditionally safe.

The use case arises in real-world code when you are implementing a trait that takes self: Pin<&mut Self> where the Self type of your impl is Pin<Ptr>. So what you have is something like Pin<&mut Pin<Box<T>>>, and want to delegate to the impl for T which takes Pin<&mut T>. Some examples:

In tokio: impl<P> AsyncRead for Pin<P>

In hyper: impl<P> Read for Pin<P>

In futures: impl<P> Stream for Pin<P>

In actix: impl<P, A> ActorFuture<A> for Pin<P>

In libcore:

impl<P> Future for Pin<P>
where
P: ops::DerefMut<Target: Future>,
{
type Output = <<P as ops::Deref>::Target as Future>::Output;
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {

impl<P> AsyncIterator for Pin<P>
where
P: DerefMut,
P::Target: AsyncIterator,
{
type Item = <P::Target as AsyncIterator>::Item;
fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {

@rfcbot
Copy link

rfcbot commented Aug 27, 2024

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Aug 27, 2024
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Sep 24, 2024
@rfcbot
Copy link

rfcbot commented Sep 24, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label Oct 4, 2024
@rfcbot rfcbot added to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Oct 4, 2024
@rfcbot
Copy link

rfcbot commented Oct 4, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Oct 9, 2024
…f_mut, r=dtolnay

Stabilize `Pin::as_deref_mut()`

Tracking issue: rust-lang#86918

Stabilizing the following API:

```rust
impl<Ptr: DerefMut> Pin<Ptr> {
    pub fn as_deref_mut(self: Pin<&mut Pin<Ptr>>) -> Pin<&mut Ptr::Target>;
}
```

I know that an FCP has not been started yet, but this isn't a very complex stabilization, and I'm hoping this can motivate an FCP to *get* started - this has been pending for a while and it's a very useful function when writing Future impls.

r? `@jonhoo`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 9, 2024
…f_mut, r=dtolnay

Stabilize `Pin::as_deref_mut()`

Tracking issue: closes rust-lang#86918

Stabilizing the following API:

```rust
impl<Ptr: DerefMut> Pin<Ptr> {
    pub fn as_deref_mut(self: Pin<&mut Pin<Ptr>>) -> Pin<&mut Ptr::Target>;
}
```

I know that an FCP has not been started yet, but this isn't a very complex stabilization, and I'm hoping this can motivate an FCP to *get* started - this has been pending for a while and it's a very useful function when writing Future impls.

r? `@jonhoo`
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 9, 2024
…mut, r=dtolnay

Stabilize `Pin::as_deref_mut()`

Tracking issue: closes rust-lang#86918

Stabilizing the following API:

```rust
impl<Ptr: DerefMut> Pin<Ptr> {
    pub fn as_deref_mut(self: Pin<&mut Pin<Ptr>>) -> Pin<&mut Ptr::Target>;
}
```

I know that an FCP has not been started yet, but this isn't a very complex stabilization, and I'm hoping this can motivate an FCP to *get* started - this has been pending for a while and it's a very useful function when writing Future impls.

r? `@jonhoo`
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 9, 2024
…mut, r=dtolnay

Stabilize `Pin::as_deref_mut()`

Tracking issue: closes rust-lang#86918

Stabilizing the following API:

```rust
impl<Ptr: DerefMut> Pin<Ptr> {
    pub fn as_deref_mut(self: Pin<&mut Pin<Ptr>>) -> Pin<&mut Ptr::Target>;
}
```

I know that an FCP has not been started yet, but this isn't a very complex stabilization, and I'm hoping this can motivate an FCP to *get* started - this has been pending for a while and it's a very useful function when writing Future impls.

r? `@jonhoo`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 14, 2024
…f_mut, r=dtolnay

Stabilize `Pin::as_deref_mut()`

Tracking issue: closes rust-lang#86918

Stabilizing the following API:

```rust
impl<Ptr: DerefMut> Pin<Ptr> {
    pub fn as_deref_mut(self: Pin<&mut Pin<Ptr>>) -> Pin<&mut Ptr::Target>;
}
```

I know that an FCP has not been started yet, but this isn't a very complex stabilization, and I'm hoping this can motivate an FCP to *get* started - this has been pending for a while and it's a very useful function when writing Future impls.

r? `@jonhoo`
@bors bors closed this as completed in 7ed6d1c Oct 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 14, 2024
Rollup merge of rust-lang#129424 - coolreader18:stabilize-pin_as_deref_mut, r=dtolnay

Stabilize `Pin::as_deref_mut()`

Tracking issue: closes rust-lang#86918

Stabilizing the following API:

```rust
impl<Ptr: DerefMut> Pin<Ptr> {
    pub fn as_deref_mut(self: Pin<&mut Pin<Ptr>>) -> Pin<&mut Ptr::Target>;
}
```

I know that an FCP has not been started yet, but this isn't a very complex stabilization, and I'm hoping this can motivate an FCP to *get* started - this has been pending for a while and it's a very useful function when writing Future impls.

r? ``@jonhoo``
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants