futures-rs icon indicating copy to clipboard operation
futures-rs copied to clipboard

Add `as_pin_ref` and `as_pin_mut` to `Either`

Open thomaseizinger opened this issue 3 years ago • 6 comments

Over at either, we are discussing to add structural projection.

Would that be a safe thing to do? What are the reasons why it is not public here in futures::Either?

Any advice is greatly appreciated.

thomaseizinger avatar Jul 27 '22 05:07 thomaseizinger

Option has as_pin_ref and as_pin_mut for this purpose. It's okay to provide Either::{as_pin_ref, as_pin_mut}, as these are sound unless you write other (wrong) unsafe code.

taiki-e avatar Jul 27 '22 05:07 taiki-e

Thank you for answering so quickly and thoroughly :)

This is resolved from my end unless you want to keep it open to maybe also expose futures::Either::project publicly?

thomaseizinger avatar Jul 27 '22 06:07 thomaseizinger

Re-opening -- I think it is reasonable to have such methods in futures.

taiki-e avatar Aug 21 '22 12:08 taiki-e

Would it make sense to depend on either instead?

thomaseizinger avatar Aug 21 '22 19:08 thomaseizinger

It's possible, but that would be a breaking change to combine them, as one can currently have both impl LocalTrait for either::Either and for futures::future::Either.

cuviper avatar Aug 22 '22 17:08 cuviper

Would it make sense to depend on either instead?

In addition to it being a breaking change, we are also implementing stream and async io traits on our Either type, so we do not plan to depend on either crate. (e.g., it makes https://github.com/rust-lang/futures-rs/issues/2362 impossible)

taiki-e avatar Aug 24 '22 02:08 taiki-e