From 8e92124f83698e08dcf1ea27a9484e17b3371e71 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Tue, 12 Nov 2019 13:36:32 -0800 Subject: [PATCH] Refine Filter::or to propagate Never more eagerly Closes #109 --- src/filter/and.rs | 5 ++-- src/filter/and_then.rs | 4 +-- src/filter/or.rs | 4 +-- src/filters/cors.rs | 6 ++--- src/reject.rs | 57 +++++++++++++++++++++++++++++++++--------- 5 files changed, 54 insertions(+), 22 deletions(-) diff --git a/src/filter/and.rs b/src/filter/and.rs index dfe618048..7a7914e9f 100644 --- a/src/filter/and.rs +++ b/src/filter/and.rs @@ -24,7 +24,7 @@ where U::Error: CombineRejection, { type Extract = <<::HList as Combine<::HList>>::Output as HList>::Tuple; - type Error = >::Rejection; + type Error = >::One; type Future = AndFuture; fn filter(&self) -> Self::Future { @@ -52,13 +52,12 @@ impl Future for AndFuture where T: Filter, U: Filter, - //T::Extract: Combine, ::HList: Combine<::HList> + Send, U::Error: CombineRejection, { type Output = Result< <<::HList as Combine<::HList>>::Output as HList>::Tuple, - >::Rejection>; + >::One>; #[project] fn poll(mut self: Pin<&mut Self>, cx: &mut Context) -> Poll { diff --git a/src/filter/and_then.rs b/src/filter/and_then.rs index 2c45dc25d..f1860a56b 100644 --- a/src/filter/and_then.rs +++ b/src/filter/and_then.rs @@ -22,7 +22,7 @@ where ::Error: CombineRejection, { type Extract = (::Ok,); - type Error = <::Error as CombineRejection>::Rejection; + type Error = <::Error as CombineRejection>::One; type Future = AndThenFuture; #[inline] fn filter(&self) -> Self::Future { @@ -66,7 +66,7 @@ where ::Error: CombineRejection, { type Output = Result<(::Ok,), - <::Error as CombineRejection>::Rejection>; + <::Error as CombineRejection>::One>; #[project] fn poll(mut self: Pin<&mut Self>, cx: &mut Context) -> Poll { diff --git a/src/filter/or.rs b/src/filter/or.rs index e20313c93..1edfb481b 100644 --- a/src/filter/or.rs +++ b/src/filter/or.rs @@ -23,7 +23,7 @@ where U::Error: CombineRejection, { type Extract = (Either,); - type Error = >::Rejection; + type Error = >::Combined; type Future = EitherFuture; fn filter(&self) -> Self::Future { @@ -65,7 +65,7 @@ where U: Filter, U::Error: CombineRejection, { - type Output = Result<(Either,), >::Rejection>; + type Output = Result<(Either,), >::Combined>; #[project] fn poll(mut self: Pin<&mut Self>, cx: &mut Context) -> Poll { diff --git a/src/filters/cors.rs b/src/filters/cors.rs index fb5dd6a9c..28d169345 100644 --- a/src/filters/cors.rs +++ b/src/filters/cors.rs @@ -247,7 +247,7 @@ where F: Filter + Clone + Send + Sync + 'static, F::Extract: Reply, F::Error: CombineRejection, - >::Rejection: CombineRejection, + >::One: CombineRejection, { type Wrapped = CorsFilter; @@ -443,7 +443,7 @@ mod internal { { type Extract = One, One>, F::Extract>>>>; - type Error = >::Rejection; + type Error = >::One; type Future = future::Either< future::Ready>, WrappedFuture, @@ -526,7 +526,7 @@ mod internal { F: TryFuture, F::Error: CombineRejection, { - type Output = Result, One>, F::Ok>>>>, >::Rejection>; + type Output = Result, One>, F::Ok>>>>, >::One>; fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll { diff --git a/src/reject.rs b/src/reject.rs index 219585741..dd647b30e 100644 --- a/src/reject.rs +++ b/src/reject.rs @@ -657,6 +657,9 @@ mod sealed { use std::convert::Infallible; use std::fmt; + // This sealed trait exists to allow Filters to return either `Rejection` + // or `Never` (to be replaced with `!`). There are no other types that make + // sense, and so it is sealed. pub trait Reject: fmt::Debug + Send + Sync { fn status(&self) -> StatusCode; fn into_response(&self) -> crate::reply::Response; @@ -669,16 +672,43 @@ mod sealed { fn _assert(_: &dyn Reject) {} } + // This weird trait is to allow optimizations of propagating when a + // rejection can *never* happen (currently with the `Never` type, + // eventually to be replaced with `!`). + // + // Using this trait means the `Never` gets propagated to chained filters, + // allowing LLVM to eliminate more code paths. Without it, such as just + // requiring that `Rejection::from(Never)` were used in those filters, + // would mean that links later in the chain may assume a rejection *could* + // happen, and no longer eliminate those branches. pub trait CombineRejection: Send + Sized { - type Rejection: Reject + From + From + Into; - - fn combine(self, other: E) -> Self::Rejection; + /// The type that should be returned when only 1 of the two + /// "rejections" occurs. + /// + /// # For example: + /// + /// `warp::any().and(warp::path("foo")` has the following steps: + /// + /// 1. Since this is `and`, only **one** of the rejections will occur, + /// and as soon as it does, it will be returned. + /// 2. `warp::any()` rejects with `Never`. So, it will never return `Never`. + /// 3. `warp::path()` rejects with `Rejection`. It may return `Rejection`. + /// + /// Thus, if the above filter rejects, it will definitely be `Rejection`. + type One: Reject + From + From + Into; + + /// The type that should be returned when both rejections occur, + /// and need to be combined. + type Combined: Reject; + + fn combine(self, other: E) -> Self::Combined; } impl CombineRejection for Rejection { - type Rejection = Rejection; + type One = Rejection; + type Combined = Rejection; - fn combine(self, other: Rejection) -> Self::Rejection { + fn combine(self, other: Rejection) -> Self::Combined { let reason = match (self.reason, other.reason) { (Reason::Other(left), Reason::Other(right)) => { Reason::Other(Box::new(Rejections::Combined(left, right))) @@ -696,25 +726,28 @@ mod sealed { } impl CombineRejection for Rejection { - type Rejection = Rejection; + type One = Rejection; + type Combined = Infallible; - fn combine(self, other: Infallible) -> Self::Rejection { + fn combine(self, other: Infallible) -> Self::Combined { match other {} } } impl CombineRejection for Infallible { - type Rejection = Rejection; + type One = Rejection; + type Combined = Infallible; - fn combine(self, _: Rejection) -> Self::Rejection { + fn combine(self, _: Rejection) -> Self::Combined { match self {} } } - impl CombineRejection for Infallible { - type Rejection = Infallible ; + impl CombineRejection for Infallible { + type One = Infallible; + type Combined = Infallible; - fn combine(self, _: Infallible) -> Self::Rejection { + fn combine(self, _: Infallible) -> Self::Combined { match self {} } }