Preserve original lifetime when performing pin-projection #67

Aaron1011 · 2019-09-04T00:45:08Z

As described in issue #65, the project method currently loses lifetime information.

Specifically, given a &'a mut Pin<&'pin mut T>, we would like project to return a ProjectionType<'pin> - that is, a type with a lifetime tied to the lifetime of the inside of the original Pin.

However, project currently returns a ProjectionType<'a> - that is, a type with a lifetime tied to the lifetime of the 'outside' of the Pin. This means that pin-projected types will not live as long as the inside of the Pin they were derived from - which prevents returning fields from methods.

This PR lifts this limitation, by ensuring that the returning projection struct/enum has a lifetime of 'pin - that is, the lifetime in the Pin<&'pin mut T> that the projection was performed on.

Most of this PR is just boilerplate to make things work on arbitrary structs and enums. The important part is the additional unsafe code added. Ultimately, it relies on this code being valid:

use std::ops::DerefMut;

fn upgrade_deref_mut<'a, T>(mut val: &'a mut T) -> &'a mut T {
    let my_ref: &mut T = val.deref_mut();
    let transmuted: &'a mut T = unsafe { core::mem::transmute(my_ref) };
    transmuted
}

That is, we need to be able to 'upgrade' the lifetime of the mutable reference returned from the <&mut T as DerefMut>::deref_mut. I believe this is sound - DerefMut is essentially a no-op for mutable references. We're only adjusting the compiler's view of the lifetime, not performing any raw pointer tricks that might make Stacked Borrows angry.

That being said, I'm not 100% sure that this is correct.

@RalfJung: Would you be able to take a look at this?

Note that this only relies on the built-in DerefMut impl for &mut T, not any user-provided &mut DerefMut impls. I think that this kind of 'upgrading' logic could potentially be extended to user-provided types that fulfill the requirement of StableDeref - that is, the mutable references they return live for as long as the original type. However, pin-project doesn't need to care about this.

Aaron1011 · 2019-09-04T00:53:46Z

Assuming that this is indeed sound, it might be useful for Rust to provide this kind of upgrading behavior on Pin itself.

Making it would for arbitrary user types would require introducing a weaker form of StableDeref into the compiler (it only needs to allow transmuting mutable references, not any of the raw pointer operations that have given us trouble). However, it could be added only for Pin<&mut T>:

impl<'a, T> Pin<&'a mut T> {
	fn as_mut_better(&mut self) -> Pin<'a mut T> {
        /// The impl from this crate
    }
}

taiki-e · 2019-09-04T09:42:32Z

pin-project-internal/src/pin_project/mod.rs

+ fn proj_trait_generics(&self) -> ProjTraitGenerics<'_> {
+ //let mut orig_generics_for_impl = orig.clone();
+ //crate::utils::proj_generics(&mut orig_generics_for_impl, self.lifetime.clone());
+ //let (impl_generics, modified_ty_generics, _) = orig_generics_for_impl.split_for_impl();


Please remove the above 3 lines.

taiki-e · 2019-09-04T09:42:39Z

pin-project-internal/src/pin_project/mod.rs

- let (orig_generics, _, orig_where_clause) = self.generics.split_for_impl();
+ let mut trait_generics = self.generics.clone();
+ crate::utils::proj_generics(&mut trait_generics, lifetime.clone());
+ //trait_generics.params.push(GenericParam::Lifetime(LifetimeDef::new(lifetime.clone())));


Suggested change

//trait_generics.params.push(GenericParam::Lifetime(LifetimeDef::new(lifetime.clone())));

taiki-e · 2019-09-04T09:42:51Z

pin-project-internal/src/pin_project/structs.rs

+ crate::utils::proj_generics(&mut orig_generics_for_impl, lifetime.clone());
+ let (impl_generics, modified_ty_generics, _) = orig_generics_for_impl.split_for_impl();
+
+ let (_, ty_generics, where_clause) = item.generics.split_for_impl();*/


Please remove the above 5 lines.

taiki-e · 2019-09-04T09:43:06Z

src/lib.rs

+ #[allow(unsafe_code)]
+ let raw_mut: &mut T = unsafe { Pin::get_unchecked_mut(as_mut) };
+
+ // NExt, we transmute the mutable reference, changing its lifetime to 'a.


Suggested change

// NExt, we transmute the mutable reference, changing its lifetime to 'a.

// Next, we transmute the mutable reference, changing its lifetime to 'a.

taiki-e · 2019-09-04T09:43:40Z

src/lib.rs

+/// Normally, you'll never need to interact with this type
+/// directly - it's used internally by pin-project to allow expressing
+/// the proper lifetime.
+pub trait ProjectThrough<'a> {


Maybe this trait should be sealed?

mod private_project_through { use core::pin::Pin; pub trait Sealed {} impl<T> Sealed for Pin<&mut T> {} } pub trait ProjectThrough<'a>: private_project_through::Sealed { // ... }

taiki-e · 2019-09-04T10:04:39Z

pin-project-internal/src/pin_project/enums.rs

- impl #impl_generics #proj_trait #ty_generics for ::core::pin::Pin<&mut #orig_ident #ty_generics> #where_clause {
- fn project<#lifetime>(&#lifetime mut self) -> #proj_ident #proj_ty_generics #where_clause {
+ impl #impl_generics #proj_trait #ty_generics for ::core::pin::Pin<&#lifetime mut #orig_ident #orig_ty_generics> #where_clause {
+ fn project(&mut self) -> #proj_ident #proj_ty_generics #where_clause {


Suggested change

fn project(&mut self) -> #proj_ident #proj_ty_generics #where_clause {

fn project(&mut self) -> #proj_ident #proj_ty_generics {

Preexisting, but as it is already declared in the impl block, it looks unnecessary.

taiki-e · 2019-09-04T10:05:00Z

pin-project-internal/src/pin_project/mod.rs


 quote! {
 trait #proj_trait #orig_generics {
- fn project<#lifetime>(&#lifetime mut self) -> #proj_ident #proj_ty_generics #orig_where_clause;
+ fn project(&mut self) -> #proj_ident #proj_ty_generics #orig_where_clause;


Suggested change

fn project(&mut self) -> #proj_ident #proj_ty_generics #orig_where_clause;

fn project(&mut self) -> #proj_ident #proj_ty_generics;

taiki-e · 2019-09-04T10:05:18Z

pin-project-internal/src/pin_project/structs.rs

- impl #impl_generics #proj_trait #ty_generics for ::core::pin::Pin<&mut #orig_ident #ty_generics> #where_clause {
- fn project<#lifetime>(&#lifetime mut self) -> #proj_ident #proj_ty_generics #where_clause {
+ impl #impl_generics #proj_trait #ty_generics for ::core::pin::Pin<&#lifetime mut #orig_ident #orig_ty_generics> #where_clause {
+ fn project(&mut self) -> #proj_ident #proj_ty_generics #where_clause {


Suggested change

fn project(&mut self) -> #proj_ident #proj_ty_generics #where_clause {

fn project(&mut self) -> #proj_ident #proj_ty_generics {

taiki-e · 2019-09-04T10:07:20Z

src/lib.rs

+/// A helper trait to allow projecting through a mutable reference,
+/// while preserving lifetimes.
+///
+/// Normally, `Pin::as_mut` can be used to convert a '&mut Pin' to a owned 'Pin'


Suggested change

/// Normally, `Pin::as_mut` can be used to convert a '&mut Pin' to a owned 'Pin'

/// Normally, `Pin::as_mut` can be used to convert a '&mut Pin' to an owned 'Pin'

taiki-e · 2019-09-04T10:07:32Z

src/lib.rs

+/// while preserving lifetimes.
+///
+/// Normally, `Pin::as_mut` can be used to convert a '&mut Pin' to a owned 'Pin'
+/// However, since `Pin::as_mut'`needs to work with generic `DerefMut` impls,


Suggested change

/// However, since `Pin::as_mut'`needs to work with generic `DerefMut` impls,

/// However, since `Pin::as_mut` needs to work with generic `DerefMut` impls,

taiki-e · 2019-09-04T10:10:06Z

src/lib.rs

+ // We accomplish this in four steps:
+ //
+ // First, call Pin::as_mut(self). This
+ // converts our '&'p mut Pin<&'a mut T>` to


Suggested change

// converts our '&'p mut Pin<&'a mut T>` to

// converts our `&'p mut Pin<&'a mut T>` to

taiki-e · 2019-09-04T10:11:16Z

src/lib.rs

+ // described above.
+ //
+ // This is the core of this method - everything else is just to deconstruct
+ // and reconstrut a Pin


Suggested change

// and reconstrut a Pin

// and reconstruct a Pin.

Aaron1011 · 2019-09-04T14:30:44Z

It turns out that this is actually unsound - the output lifetime isn't tied to the input lifetime, so it's possible to create aliasing mutable references by calling project twice.

I'm investigating if this can be made sound.

Aaron1011 · 2019-09-04T17:33:18Z

Unfortunately, I think it's impossible to do this soundly in the project method.

There are two conflicting requirements:

The projection type returned from Project should not be required to outlive 'a in Pin<&'a mut MyType>. This allows us to call project() multiple times, and to use the original Pin<&'a mut MyType> after calling project().
THe projection type returned from Project should be required to outlive 'a in Pin<&'a mut MyType>. This allow us to write code like:

fn get_pin_mut<'a>(mut self: Pin<&'a mut Self>) -> Pin<&'a mut T> {
    self.project().pinned
}

where the returned type outlives the self parameter passed into the method.

It is impossible for one lifetime that both outlives and does not outlive another lifetime.

Instead, I think we need to add a new method: project_into. This method will take self by value (effectively adding the pre-#47 behavior back). Users will choose between project or project_into based on their needs.

Aaron1011 · 2019-09-04T17:33:24Z

Unfortunately, I think it's impossible to do this soundly in the project method.

There are two conflicting requirements:

The projection type returned from Project should not be required to outlive 'a in Pin<&'a mut MyType>. This allows us to call project() multiple times, and to use the original Pin<&'a mut MyType> after calling project().
The projection type returned from Project should be required to outlive 'a in Pin<&'a mut MyType>. This allow us to write code like:

fn get_pin_mut<'a>(mut self: Pin<&'a mut Self>) -> Pin<&'a mut T> {
    self.project().pinned
}

where the returned type outlives the self parameter passed into the method.

It is impossible for one lifetime to both outlive and not outlive another lifetime.

Instead, I think we need to add a new method: project_into. This method will take self by value (effectively adding the pre-#47 behavior back). Users will choose between project or project_into based on their needs.

Aaron1011 · 2019-09-04T17:34:43Z

Superseded by #69

RalfJung · 2019-09-11T16:50:59Z

I am very confused by this PR, this seems entirely unnecessary -- why don't you reborrow the pinned reference? See what I wrote at #47 (comment).

Aaron1011 added 8 commits September 3, 2019 19:59

Some refactoring

eaea854

Get enums working

5f75e3f

Silence warnings

e2f266b

Use get_unchecked_mut instead of into_inner_unchecked

d68ce6e

Add test for 'lifetime projection'

c02874c

Add explanation of unsafety

f962788

Run 'cargo fmt'

b0b6dd7

Fix clippy warnings

6a53578

Fix one other clippy lint

0264272

taiki-e reviewed Sep 4, 2019

View reviewed changes

taiki-e added this to the v0.4 milestone Sep 4, 2019

Aaron1011 closed this Sep 4, 2019

Aaron1011 deleted the fix/proj-lifetime branch September 4, 2019 17:35

taiki-e added the A-pin-projection Area: #[pin_project] label Sep 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preserve original lifetime when performing pin-projection #67

Preserve original lifetime when performing pin-projection #67

Aaron1011 commented Sep 4, 2019

Aaron1011 commented Sep 4, 2019

taiki-e Sep 4, 2019

taiki-e Sep 4, 2019

taiki-e Sep 4, 2019

taiki-e Sep 4, 2019

taiki-e Sep 4, 2019 •

edited

Loading

taiki-e Sep 4, 2019

taiki-e Sep 4, 2019

taiki-e Sep 4, 2019

taiki-e Sep 4, 2019

taiki-e Sep 4, 2019 •

edited

Loading

taiki-e Sep 4, 2019

taiki-e Sep 4, 2019

Aaron1011 commented Sep 4, 2019 •

edited

Loading

Aaron1011 commented Sep 4, 2019

Aaron1011 commented Sep 4, 2019 •

edited

Loading

Aaron1011 commented Sep 4, 2019 •

edited

Loading

RalfJung commented Sep 11, 2019

	// NExt, we transmute the mutable reference, changing its lifetime to 'a.
	// Next, we transmute the mutable reference, changing its lifetime to 'a.

	fn project(&mut self) -> #proj_ident #proj_ty_generics #where_clause {
	fn project(&mut self) -> #proj_ident #proj_ty_generics {

	/// Normally, `Pin::as_mut` can be used to convert a '&mut Pin' to a owned 'Pin'
	/// Normally, `Pin::as_mut` can be used to convert a '&mut Pin' to an owned 'Pin'

	/// However, since `Pin::as_mut'`needs to work with generic `DerefMut` impls,
	/// However, since `Pin::as_mut` needs to work with generic `DerefMut` impls,

	// converts our '&'p mut Pin<&'a mut T>` to
	// converts our `&'p mut Pin<&'a mut T>` to

Preserve original lifetime when performing pin-projection #67

Preserve original lifetime when performing pin-projection #67

Conversation

Aaron1011 commented Sep 4, 2019

Aaron1011 commented Sep 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taiki-e Sep 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taiki-e Sep 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaron1011 commented Sep 4, 2019 • edited Loading

Aaron1011 commented Sep 4, 2019

Aaron1011 commented Sep 4, 2019 • edited Loading

Aaron1011 commented Sep 4, 2019 • edited Loading

RalfJung commented Sep 11, 2019

taiki-e Sep 4, 2019 •

edited

Loading

taiki-e Sep 4, 2019 •

edited

Loading

Aaron1011 commented Sep 4, 2019 •

edited

Loading

Aaron1011 commented Sep 4, 2019 •

edited

Loading

Aaron1011 commented Sep 4, 2019 •

edited

Loading