From 9ecc6b8d85c653febab13f82b787a8581c14ae92 Mon Sep 17 00:00:00 2001 From: Greg Pataky Date: Sun, 7 May 2023 22:31:22 -0700 Subject: [PATCH] Update Future combinators to preserve Clone This updates most `Future` combinators to preserve `Clone` when available on the input `Future`. For motivation, imagine you have some complicated `Future` that is not `Clone` and requires `.shared()` to properly share it. Then imagine you have a library function that is meant to bundle together a bunch of combinators to fulfill some semantic purpose. That library funciton will have to call `.shared()` if it wants to try to guarantee the return `Future` is `Clone`, but this might be suboptimal if the input `Future` was already `Clone`, plus it has the ability to obfuscate and hide the `.shared()` allocation. With this change, you can instead require `Future + Clone` on the input `Future` and have a guarantee the output will be `Clone` as well. The hold-out `Future` implementations are: - `Remote` / `RemoteHandle` due to their use of `futures_channel::oneshot::{Sender, Receiver}`. This seems like it is by design that these cannot be `Clone`. - `JoinAll` / `TryJoinAll` due to their use of `Stream` combinators, but also that it seems unlikely that people would expect them to offer `Clone` since they are used to performa a potentially costly sync barrier that would probably be desired to happen only once. For the hold-outs, the existing pattern of using `.shared()` allows for `Clone`, and follows the intended semantics of those combinators. Some combinators that might not make the most sense to preserve `Clone`: - `IntoStream` - `TryFlattenStream` If these changes make sense, I think it would also make sense to apply them to `Stream` combinators as well (although I don't see myself utilizing this property as much with them). If that is the case, these `Future` -> `Stream` combinators make sense to preserve `Clone`. Tested: - `cargo doc`. - `cargo fmt`. - `cargo test --all-features`. --- .../src/future/future/catch_unwind.rs | 2 +- futures-util/src/future/future/flatten.rs | 2 +- futures-util/src/future/future/fuse.rs | 2 +- futures-util/src/future/future/map.rs | 2 +- futures-util/src/future/future/mod.rs | 18 ++++++------ futures-util/src/future/join.rs | 12 ++++++++ futures-util/src/future/lazy.rs | 2 +- futures-util/src/future/maybe_done.rs | 2 +- futures-util/src/future/poll_fn.rs | 1 + futures-util/src/future/select.rs | 2 +- futures-util/src/future/select_all.rs | 2 +- futures-util/src/future/select_ok.rs | 2 +- .../src/future/try_future/into_future.rs | 2 +- futures-util/src/future/try_future/mod.rs | 28 +++++++++---------- .../src/future/try_future/try_flatten.rs | 2 +- .../src/future/try_future/try_flatten_err.rs | 2 +- futures-util/src/future/try_join.rs | 12 ++++++++ futures-util/src/future/try_maybe_done.rs | 2 +- futures-util/src/future/try_select.rs | 2 +- futures-util/src/lib.rs | 7 +++++ 20 files changed, 69 insertions(+), 37 deletions(-) diff --git a/futures-util/src/future/future/catch_unwind.rs b/futures-util/src/future/future/catch_unwind.rs index 0e09d6eeb0..2277792265 100644 --- a/futures-util/src/future/future/catch_unwind.rs +++ b/futures-util/src/future/future/catch_unwind.rs @@ -8,7 +8,7 @@ use pin_project_lite::pin_project; pin_project! { /// Future for the [`catch_unwind`](super::FutureExt::catch_unwind) method. - #[derive(Debug)] + #[derive(Clone, Debug)] #[must_use = "futures do nothing unless you `.await` or poll them"] pub struct CatchUnwind { #[pin] diff --git a/futures-util/src/future/future/flatten.rs b/futures-util/src/future/future/flatten.rs index 12105e6202..2dd93aef47 100644 --- a/futures-util/src/future/future/flatten.rs +++ b/futures-util/src/future/future/flatten.rs @@ -9,7 +9,7 @@ use pin_project_lite::pin_project; pin_project! { #[project = FlattenProj] - #[derive(Debug)] + #[derive(Clone, Debug)] pub enum Flatten { First { #[pin] f: Fut1 }, Second { #[pin] f: Fut2 }, diff --git a/futures-util/src/future/future/fuse.rs b/futures-util/src/future/future/fuse.rs index 2257906726..46420b632a 100644 --- a/futures-util/src/future/future/fuse.rs +++ b/futures-util/src/future/future/fuse.rs @@ -5,7 +5,7 @@ use pin_project_lite::pin_project; pin_project! { /// Future for the [`fuse`](super::FutureExt::fuse) method. - #[derive(Debug)] + #[derive(Clone, Debug)] #[must_use = "futures do nothing unless you `.await` or poll them"] pub struct Fuse { #[pin] diff --git a/futures-util/src/future/future/map.rs b/futures-util/src/future/future/map.rs index 7471aba000..97d64f4eab 100644 --- a/futures-util/src/future/future/map.rs +++ b/futures-util/src/future/future/map.rs @@ -10,7 +10,7 @@ pin_project! { /// Internal Map future #[project = MapProj] #[project_replace = MapProjReplace] - #[derive(Debug)] + #[derive(Clone, Debug)] #[must_use = "futures do nothing unless you `.await` or poll them"] pub enum Map { Incomplete { diff --git a/futures-util/src/future/future/mod.rs b/futures-util/src/future/future/mod.rs index 7fa2d70a70..7c599feb6e 100644 --- a/futures-util/src/future/future/mod.rs +++ b/futures-util/src/future/future/mod.rs @@ -30,7 +30,7 @@ delegate_all!( /// Future for the [`flatten`](super::FutureExt::flatten) method. Flatten( flatten::Flatten::Output> - ): Debug + Future + FusedFuture + New[|x: F| flatten::Flatten::new(x)] + ): Clone + Debug + Future + FusedFuture + New[|x: F| flatten::Flatten::new(x)] where F: Future ); @@ -38,7 +38,7 @@ delegate_all!( /// Stream for the [`flatten_stream`](FutureExt::flatten_stream) method. FlattenStream( flatten::Flatten::Output> - ): Debug + Sink + Stream + FusedStream + New[|x: F| flatten::Flatten::new(x)] + ): Clone + Debug + Sink + Stream + FusedStream + New[|x: F| flatten::Flatten::new(x)] where F: Future ); @@ -49,49 +49,49 @@ delegate_all!( /// Future for the [`map`](super::FutureExt::map) method. Map( map::Map - ): Debug + Future + FusedFuture + New[|x: Fut, f: F| map::Map::new(x, f)] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut, f: F| map::Map::new(x, f)] ); delegate_all!( /// Stream for the [`into_stream`](FutureExt::into_stream) method. IntoStream( crate::stream::Once - ): Debug + Stream + FusedStream + New[|x: F| crate::stream::Once::new(x)] + ): Clone + Debug + Stream + FusedStream + New[|x: F| crate::stream::Once::new(x)] ); delegate_all!( /// Future for the [`map_into`](FutureExt::map_into) combinator. MapInto( Map> - ): Debug + Future + FusedFuture + New[|x: Fut| Map::new(x, into_fn())] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut| Map::new(x, into_fn())] ); delegate_all!( /// Future for the [`then`](FutureExt::then) method. Then( flatten::Flatten, Fut2> - ): Debug + Future + FusedFuture + New[|x: Fut1, y: F| flatten::Flatten::new(Map::new(x, y))] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut1, y: F| flatten::Flatten::new(Map::new(x, y))] ); delegate_all!( /// Future for the [`inspect`](FutureExt::inspect) method. Inspect( map::Map> - ): Debug + Future + FusedFuture + New[|x: Fut, f: F| map::Map::new(x, inspect_fn(f))] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut, f: F| map::Map::new(x, inspect_fn(f))] ); delegate_all!( /// Future for the [`never_error`](super::FutureExt::never_error) combinator. NeverError( Map> - ): Debug + Future + FusedFuture + New[|x: Fut| Map::new(x, ok_fn())] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut| Map::new(x, ok_fn())] ); delegate_all!( /// Future for the [`unit_error`](super::FutureExt::unit_error) combinator. UnitError( Map> - ): Debug + Future + FusedFuture + New[|x: Fut| Map::new(x, ok_fn())] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut| Map::new(x, ok_fn())] ); #[cfg(feature = "std")] diff --git a/futures-util/src/future/join.rs b/futures-util/src/future/join.rs index 8c16c3e331..680aad7045 100644 --- a/futures-util/src/future/join.rs +++ b/futures-util/src/future/join.rs @@ -20,6 +20,18 @@ impl Join { } } +impl Clone for Join +where + Fut1: Future + Clone, + Fut1::Output: Clone, + Fut2: Future + Clone, + Fut2::Output: Clone, +{ + fn clone(&self) -> Self { + Self { fut1: self.fut1.clone(), fut2: self.fut2.clone() } + } +} + impl fmt::Debug for Join where Fut1: Future + fmt::Debug, diff --git a/futures-util/src/future/lazy.rs b/futures-util/src/future/lazy.rs index e9a8cf2fa9..67105bdd4e 100644 --- a/futures-util/src/future/lazy.rs +++ b/futures-util/src/future/lazy.rs @@ -4,7 +4,7 @@ use futures_core::future::{FusedFuture, Future}; use futures_core::task::{Context, Poll}; /// Future for the [`lazy`] function. -#[derive(Debug)] +#[derive(Clone, Debug)] #[must_use = "futures do nothing unless you `.await` or poll them"] pub struct Lazy { f: Option, diff --git a/futures-util/src/future/maybe_done.rs b/futures-util/src/future/maybe_done.rs index 26e6c27588..e8a93b0337 100644 --- a/futures-util/src/future/maybe_done.rs +++ b/futures-util/src/future/maybe_done.rs @@ -10,7 +10,7 @@ use futures_core::task::{Context, Poll}; /// A future that may have completed. /// /// This is created by the [`maybe_done()`] function. -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum MaybeDone { /// A not-yet-completed future Future(/* #[pin] */ Fut), diff --git a/futures-util/src/future/poll_fn.rs b/futures-util/src/future/poll_fn.rs index 19311570b5..c9c8e4097b 100644 --- a/futures-util/src/future/poll_fn.rs +++ b/futures-util/src/future/poll_fn.rs @@ -8,6 +8,7 @@ use futures_core::task::{Context, Poll}; /// Future for the [`poll_fn`] function. #[must_use = "futures do nothing unless you `.await` or poll them"] +#[derive(Clone)] pub struct PollFn { f: F, } diff --git a/futures-util/src/future/select.rs b/futures-util/src/future/select.rs index 7e33d195f7..ccda2aa0ef 100644 --- a/futures-util/src/future/select.rs +++ b/futures-util/src/future/select.rs @@ -6,7 +6,7 @@ use futures_core::task::{Context, Poll}; /// Future for the [`select()`] function. #[must_use = "futures do nothing unless you `.await` or poll them"] -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct Select { inner: Option<(A, B)>, } diff --git a/futures-util/src/future/select_all.rs b/futures-util/src/future/select_all.rs index 0a51d0da6c..ac88aa2b70 100644 --- a/futures-util/src/future/select_all.rs +++ b/futures-util/src/future/select_all.rs @@ -8,7 +8,7 @@ use futures_core::future::Future; use futures_core::task::{Context, Poll}; /// Future for the [`select_all`] function. -#[derive(Debug)] +#[derive(Clone, Debug)] #[must_use = "futures do nothing unless you `.await` or poll them"] pub struct SelectAll { inner: Vec, diff --git a/futures-util/src/future/select_ok.rs b/futures-util/src/future/select_ok.rs index 5d5579930b..ced7441aca 100644 --- a/futures-util/src/future/select_ok.rs +++ b/futures-util/src/future/select_ok.rs @@ -8,7 +8,7 @@ use futures_core::future::{Future, TryFuture}; use futures_core::task::{Context, Poll}; /// Future for the [`select_ok`] function. -#[derive(Debug)] +#[derive(Clone, Debug)] #[must_use = "futures do nothing unless you `.await` or poll them"] pub struct SelectOk { inner: Vec, diff --git a/futures-util/src/future/try_future/into_future.rs b/futures-util/src/future/try_future/into_future.rs index 9f093d0e2e..93dd2b51df 100644 --- a/futures-util/src/future/try_future/into_future.rs +++ b/futures-util/src/future/try_future/into_future.rs @@ -5,7 +5,7 @@ use pin_project_lite::pin_project; pin_project! { /// Future for the [`into_future`](super::TryFutureExt::into_future) method. - #[derive(Debug)] + #[derive(Clone, Debug)] #[must_use = "futures do nothing unless you `.await` or poll them"] pub struct IntoFuture { #[pin] diff --git a/futures-util/src/future/try_future/mod.rs b/futures-util/src/future/try_future/mod.rs index e5bc700714..89b0b25cca 100644 --- a/futures-util/src/future/try_future/mod.rs +++ b/futures-util/src/future/try_future/mod.rs @@ -31,21 +31,21 @@ delegate_all!( /// Future for the [`try_flatten`](TryFutureExt::try_flatten) method. TryFlatten( try_flatten::TryFlatten - ): Debug + Future + FusedFuture + New[|x: Fut1| try_flatten::TryFlatten::new(x)] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut1| try_flatten::TryFlatten::new(x)] ); delegate_all!( /// Future for the [`try_flatten_err`](TryFutureExt::try_flatten_err) method. TryFlattenErr( try_flatten_err::TryFlattenErr - ): Debug + Future + FusedFuture + New[|x: Fut1| try_flatten_err::TryFlattenErr::new(x)] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut1| try_flatten_err::TryFlattenErr::new(x)] ); delegate_all!( /// Future for the [`try_flatten_stream`](TryFutureExt::try_flatten_stream) method. TryFlattenStream( try_flatten::TryFlatten - ): Debug + Sink + Stream + FusedStream + New[|x: Fut| try_flatten::TryFlatten::new(x)] + ): Clone + Debug + Sink + Stream + FusedStream + New[|x: Fut| try_flatten::TryFlatten::new(x)] where Fut: TryFuture ); @@ -55,49 +55,49 @@ delegate_all!( #[cfg_attr(docsrs, doc(cfg(feature = "sink")))] FlattenSink( try_flatten::TryFlatten - ): Debug + Sink + Stream + FusedStream + New[|x: Fut| try_flatten::TryFlatten::new(x)] + ): Clone + Debug + Sink + Stream + FusedStream + New[|x: Fut| try_flatten::TryFlatten::new(x)] ); delegate_all!( /// Future for the [`and_then`](TryFutureExt::and_then) method. AndThen( TryFlatten, Fut2> - ): Debug + Future + FusedFuture + New[|x: Fut1, f: F| TryFlatten::new(MapOk::new(x, f))] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut1, f: F| TryFlatten::new(MapOk::new(x, f))] ); delegate_all!( /// Future for the [`or_else`](TryFutureExt::or_else) method. OrElse( TryFlattenErr, Fut2> - ): Debug + Future + FusedFuture + New[|x: Fut1, f: F| TryFlattenErr::new(MapErr::new(x, f))] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut1, f: F| TryFlattenErr::new(MapErr::new(x, f))] ); delegate_all!( /// Future for the [`err_into`](TryFutureExt::err_into) method. ErrInto( MapErr> - ): Debug + Future + FusedFuture + New[|x: Fut| MapErr::new(x, into_fn())] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut| MapErr::new(x, into_fn())] ); delegate_all!( /// Future for the [`ok_into`](TryFutureExt::ok_into) method. OkInto( MapOk> - ): Debug + Future + FusedFuture + New[|x: Fut| MapOk::new(x, into_fn())] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut| MapOk::new(x, into_fn())] ); delegate_all!( /// Future for the [`inspect_ok`](super::TryFutureExt::inspect_ok) method. InspectOk( Inspect, InspectOkFn> - ): Debug + Future + FusedFuture + New[|x: Fut, f: F| Inspect::new(IntoFuture::new(x), inspect_ok_fn(f))] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut, f: F| Inspect::new(IntoFuture::new(x), inspect_ok_fn(f))] ); delegate_all!( /// Future for the [`inspect_err`](super::TryFutureExt::inspect_err) method. InspectErr( Inspect, InspectErrFn> - ): Debug + Future + FusedFuture + New[|x: Fut, f: F| Inspect::new(IntoFuture::new(x), inspect_err_fn(f))] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut, f: F| Inspect::new(IntoFuture::new(x), inspect_err_fn(f))] ); #[allow(unreachable_pub)] // https://github.com/rust-lang/rust/issues/57411 @@ -107,28 +107,28 @@ delegate_all!( /// Future for the [`map_ok`](TryFutureExt::map_ok) method. MapOk( Map, MapOkFn> - ): Debug + Future + FusedFuture + New[|x: Fut, f: F| Map::new(IntoFuture::new(x), map_ok_fn(f))] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut, f: F| Map::new(IntoFuture::new(x), map_ok_fn(f))] ); delegate_all!( /// Future for the [`map_err`](TryFutureExt::map_err) method. MapErr( Map, MapErrFn> - ): Debug + Future + FusedFuture + New[|x: Fut, f: F| Map::new(IntoFuture::new(x), map_err_fn(f))] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut, f: F| Map::new(IntoFuture::new(x), map_err_fn(f))] ); delegate_all!( /// Future for the [`map_ok_or_else`](TryFutureExt::map_ok_or_else) method. MapOkOrElse( Map, MapOkOrElseFn> - ): Debug + Future + FusedFuture + New[|x: Fut, f: F, g: G| Map::new(IntoFuture::new(x), map_ok_or_else_fn(f, g))] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut, f: F, g: G| Map::new(IntoFuture::new(x), map_ok_or_else_fn(f, g))] ); delegate_all!( /// Future for the [`unwrap_or_else`](TryFutureExt::unwrap_or_else) method. UnwrapOrElse( Map, UnwrapOrElseFn> - ): Debug + Future + FusedFuture + New[|x: Fut, f: F| Map::new(IntoFuture::new(x), unwrap_or_else_fn(f))] + ): Clone + Debug + Future + FusedFuture + New[|x: Fut, f: F| Map::new(IntoFuture::new(x), unwrap_or_else_fn(f))] ); impl TryFutureExt for Fut {} diff --git a/futures-util/src/future/try_future/try_flatten.rs b/futures-util/src/future/try_future/try_flatten.rs index 4587ae8493..90fe7d425e 100644 --- a/futures-util/src/future/try_future/try_flatten.rs +++ b/futures-util/src/future/try_future/try_flatten.rs @@ -9,7 +9,7 @@ use pin_project_lite::pin_project; pin_project! { #[project = TryFlattenProj] - #[derive(Debug)] + #[derive(Clone, Debug)] pub enum TryFlatten { First { #[pin] f: Fut1 }, Second { #[pin] f: Fut2 }, diff --git a/futures-util/src/future/try_future/try_flatten_err.rs b/futures-util/src/future/try_future/try_flatten_err.rs index 6b371644d5..7c3939031d 100644 --- a/futures-util/src/future/try_future/try_flatten_err.rs +++ b/futures-util/src/future/try_future/try_flatten_err.rs @@ -6,7 +6,7 @@ use pin_project_lite::pin_project; pin_project! { #[project = TryFlattenErrProj] - #[derive(Debug)] + #[derive(Clone, Debug)] pub enum TryFlattenErr { First { #[pin] f: Fut1 }, Second { #[pin] f: Fut2 }, diff --git a/futures-util/src/future/try_join.rs b/futures-util/src/future/try_join.rs index 05a6fd7001..589c74fdf6 100644 --- a/futures-util/src/future/try_join.rs +++ b/futures-util/src/future/try_join.rs @@ -20,6 +20,18 @@ impl TryJoin { } } +impl Clone for TryJoin +where + Fut1: TryFuture + Clone, + Fut1::Ok: Clone, + Fut2: TryFuture + Clone, + Fut2::Ok: Clone, +{ + fn clone(&self) -> Self { + Self { fut1: self.fut1.clone(), fut2: self.fut2.clone() } + } +} + impl fmt::Debug for TryJoin where Fut1: TryFuture + fmt::Debug, diff --git a/futures-util/src/future/try_maybe_done.rs b/futures-util/src/future/try_maybe_done.rs index 24044d2c27..97fcbe162d 100644 --- a/futures-util/src/future/try_maybe_done.rs +++ b/futures-util/src/future/try_maybe_done.rs @@ -10,7 +10,7 @@ use futures_core::task::{Context, Poll}; /// A future that may have completed with an error. /// /// This is created by the [`try_maybe_done()`] function. -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum TryMaybeDone { /// A not-yet-completed future Future(/* #[pin] */ Fut), diff --git a/futures-util/src/future/try_select.rs b/futures-util/src/future/try_select.rs index bc282f7db1..be7816006b 100644 --- a/futures-util/src/future/try_select.rs +++ b/futures-util/src/future/try_select.rs @@ -5,7 +5,7 @@ use futures_core::task::{Context, Poll}; /// Future for the [`try_select()`] function. #[must_use = "futures do nothing unless you `.await` or poll them"] -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct TrySelect { inner: Option<(A, B)>, } diff --git a/futures-util/src/lib.rs b/futures-util/src/lib.rs index e00faf4775..1536b3f77c 100644 --- a/futures-util/src/lib.rs +++ b/futures-util/src/lib.rs @@ -253,6 +253,13 @@ macro_rules! delegate_all { delegate_sink!(inner, _Item); } }; + (@trait Clone $name:ident < $($arg:ident),* > ($t:ty) $(where $($bound:tt)*)*) => { + impl<$($arg),*> Clone for $name<$($arg),*> where $t: Clone $(, $($bound)*)* { + fn clone(&self) -> Self { + Self { inner: Clone::clone(&self.inner) } + } + } + }; (@trait Debug $name:ident < $($arg:ident),* > ($t:ty) $(where $($bound:tt)*)*) => { impl<$($arg),*> core::fmt::Debug for $name<$($arg),*> where $t: core::fmt::Debug $(, $($bound)*)* { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {