Allow pin projection

[thomaseizinger]

Not sure whether this is in scope for this library but in rust-libp2p, we have a custom Either type because we need pin projection.

Would you accept a PR for a feature-flagged pin projection implementation using the pin-project library?

[cuviper]

Are there clear semantics for this? It seems that #[pin] could be on Left, Right, or both variants.

But maybe we could have manual methods for each? Something like:

impl<L, R> Either<L, R> {
    pub fn project(self: Pin<&mut Self>) -> Either<Pin<&mut L>, Pin<&mut R>>;
    pub fn project_left(self: Pin<&mut Self>) -> Either<Pin<&mut L>, &mut R>
    pub fn project_right(self: Pin<&mut Self>) -> Either<&mut L, Pin<&mut R>>;
}

I'm not enough of a Pin expert to know whether those bare references are okay, but pin_project appears to allow it...

[thomaseizinger]

I have not thought about this actually. In our usage, we always have it on both because we implement traits on our Either types that require a pinned receiver.

Having said that, Either is probably more general than that and there are likely cases where you'd want to pin only one of the two.

The question then is, should we even use the macro or implement it ourselves? I think that will require unsafe code.

[cuviper]

I notice that futures::future::Either doesn't do any pin projection, despite this being closer to their domain for async code. They do implement Future, of course, but we lack that here.

[thomaseizinger]

It does have a project function but it is private: https://docs.rs/futures-util/latest/src/futures_util/future/either.rs.html#36

I just re-read the following section on pinning and I think it makes sense to include it here: https://doc.rust-lang.org/std/pin/index.html#projections-and-structural-pinning

Esp. the last part:

When implementing a Future combinator, you will usually need structural pinning for the nested futures, as you need to get pinned references to them to call poll. But if your combinator contains any other data that does not need to be pinned, you can make those fields not structural and hence freely access them with a mutable reference even when you just have Pin<&mut Self> (such as in your own poll implementation).

Looking at the list of requirements for structural pinning, I think we can guarantee all of these? https://doc.rust-lang.org/std/pin/index.html#pinning-is-structural-for-field

[cuviper]

It does have a project function but it is private:

I see, and then it uses that for a match in all its self: Pin<&mut Self> methods, like Future::poll.

But reading more about structural pinning, I think we should not do the left/right variations, because it would be unsafe to allow arbitrary combinations of these to be in use. The full project may be the only safe way to do this generically.

[thomaseizinger]

Would it?

Either cannot have both of its variants set at once so having only one pinned should be fine because either "all fields" project the pinning or none because they are not set.

Also, going through the requirements list:

1. We do not manually implement Unpin.
2. We do not have a destructor.
3. We can uphold the Drop guarantee. There is nothing in Either that would cause Drop to not propagate.
4. We don't offer any way of otherwise moving the data: Functions like left_and_then require a self receiver and can't be called on a pinned Either.

We could also expose an unsafe project function to be really sure although from my understanding, it is safe.

I wonder why the futures one is not public. I'll ask there!

[taiki-e]

I think we should not do the left/right variations, because it would be unsafe to allow arbitrary combinations of these to be in use. The full project may be the only safe way to do this generically.

Yes, there is no sound way to provide multiple variations as a safe API (without Unpin bound).

Either cannot have both of its variants set at once so having only one pinned should be fine because either "all fields" project the pinning or none because they are not set.

The problem is that a field can be projected as a pinned field and then as an unpinned field. For example, it would allow for a future to be moved after polling.

1. pin Either<L, R: Future + !Unpin>
2. call project_right or project and poll (pinned) R.
3. call project_left and take (unpinned) R from Either::Right. -- pointer to R created in 2. becomes invalid
4. pin R again and poll it. -- dereference an invalid pointer

[thomaseizinger]

The only bit I don't understand here is how you can call project_left after calling project_right when both functions consume self?

Also, assuming the entire Either type is pinned, how do you "take out" R from Right in an unpinned way? You can't match on it because you don't own Self.

[taiki-e]

The only bit I don't understand here is how you can call project_left after calling project_right when both functions consume self?

You can reborrow self by using as_mut.

Also, assuming the entire Either type is pinned, how do you "take out" R from Right in an unpinned way? You can't match on it because you don't own Self.

If there is a way to create a new R:

let r = match either.project_left() {
    Either::Right(r /* &mut R */) => std::mem::replace(r, R::new()),
    _ => panic!(),
};

Otherwise, the same can be done by using Option<R> instead of R:

let r = match either.project_left() {
    Either::Right(r /* &mut Option<R> */) => r.take(),
    _ => panic!(),
};

[thomaseizinger]

Got it, thank you for explaining in detail :)

I conclude that exposing a project function for both would be safe then?

[cuviper]

It seems like a good idea to mimic the stable Option names of as_pin_ref and as_pin_mut. Nothing in std uses the name project AFAICS -- are there other crates that do have that in public API?
(Of course #[pin_project] gives you fn project, but that's not pub.)

Yes, there is no sound way to provide multiple variations as a safe API (without Unpin bound).

Hmm, but it would be ok to have that proposed project_left with R: Unpin, and vice versa, right? Although if you have that type safety, you could just project().map_right(Pin::get_mut), and users can also unsafely map the unchecked version.

[thomaseizinger]

It seems like a good idea to mimic the stable Option names of as_pin_ref and as_pin_mut. Nothing in std uses the name project AFAICS -- are there other crates that do have that in public API?

Not that I am aware but I agree that we should use std naming.

Yes, there is no sound way to provide multiple variations as a safe API (without Unpin bound).

Hmm, but it would be ok to have that proposed project_left with R: Unpin, and vice versa, right? Although if you have that type safety, you could just project().map_right(Pin::get_mut), and users can also unsafely map the unchecked version.

I think it is a better tradeoff in terms of API surface and orthogonality to only provide as_pin_ref and as_pin_mut and let users compose with the APIs on Pin which have Unpin bounds.

[taiki-e]

It seems like a good idea to mimic the stable Option names of as_pin_ref and as_pin_mut. Nothing in std uses the name project AFAICS -- are there other crates that do have that in public API?

Not that I am aware but I agree that we should use std naming.

I agree that as_pin_{ref,mut} is the preferable name.

Hmm, but it would be ok to have that proposed project_left with R: Unpin, and vice versa, right?

Yeah, it needs Unpin bounds on the unpinned fields.

you could just project().map_right(Pin::get_mut)

If T is Unpin, Pin<&mut T> implements DerefMut, so get_mut is also unnecessary in many cases.