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

Open
2 of 3 tasks
jonhoo opened this issue Jul 6, 2021 · 10 comments
Open
2 of 3 tasks

Tracking Issue for pin_deref_mut #86918

jonhoo opened this issue Jul 6, 2021 · 10 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. 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: A tracking issue for an RFC or an unstable feature. 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
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. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants