From 6da963366d61e32f333bf40eed180a8003dcf93c Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Tue, 15 Dec 2020 22:48:08 -0800 Subject: [PATCH 01/12] Try Trait v2 --- text/0000-try-trait-v2.md | 795 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 795 insertions(+) create mode 100644 text/0000-try-trait-v2.md diff --git a/text/0000-try-trait-v2.md b/text/0000-try-trait-v2.md new file mode 100644 index 00000000000..f2f61e5094e --- /dev/null +++ b/text/0000-try-trait-v2.md @@ -0,0 +1,795 @@ +- Feature Name: try_trait_v2 +- Start Date: 2020-12-12 +- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) +- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) + +# Summary +[summary]: #summary + +Replace [RFC #1859, `try_trait`](https://rust-lang.github.io/rfcs/1859-try-trait.html), +with a new design for the currently-unstable [`Try` trait](https://doc.rust-lang.org/nightly/std/ops/trait.Try.html) +and corresponding desugaring for the `?` operator. + +The new design supports all the currently-stable conversions (including the accidental ones), +while addressing the discovered shortcomings of the currently-implemented solution, +as well as enabling new scenarios. + +*This is forward-looking to be compatible with other features, +like `try {}` blocks or `yeet e` expressions or `Iterator::try_find`, +but the statuses of those features are **not** themselves impacted by this RFC.* + +# Motivation +[motivation]: #motivation + +The motivations from the previous RFC still apply (supporting more types, and restricted interconversion). +However, new information has come in since the previous RFC, making people wish for a different approach. + +- An [experience report](https://github.com/rust-lang/rust/issues/42327#issuecomment-366840247) in the tracking issue mentioned that it's annoying to need to make a residual type. +- The `try {}` conversations have wished for more source information to flow through `?` so that fewer annotations would be required. +- Similarly, it's no longer clear that `From` should be part of the `?` desugaring for _all_ types. It's both more flexible -- making inference difficult -- and more restrictive -- especially without specialization -- than is always desired. +- Various library methods, such as `try_map` for arrays ([PR #79713](https://github.com/rust-lang/rust/pull/79713#issuecomment-739075171)), would like to be able to do HKT-like things to produce their result types. (For example, `Iterator::try_find` wants to be able to return a `Foo` from a predicate that returned a `Foo`.) +- Using the "error" terminology is a poor fit for other potential implementations of the trait. +- It turned out that the current solution accidentally stabilized more interconversion than expected, so a more restricted form may be warranted. + +This RFC proposes a solution that _mixes_ the two major options considered last time. + +- Like the _reductionist_ approach, this RFC proposes an unparameterized trait with an _associated_ type for the "ok" part, so that the type produced from the `?` operator on a value is always the same. +- Like the [_essentialist_ approach](https://github.com/rust-lang/rfcs/blob/master/text/1859-try-trait.md#the-essentialist-approach), this RFC proposes a trait with a _generic_ parameter for "error" part, so that different types can be consumed. + + + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +## The `ops::ControlFlow` type + +This is a simple enum: +```rust +struct ControlFlow { + Break(B), + Continue(C), +} +``` + +Its purpose is to clearly communicate the desire to either short-circuit what's happening (`Break`), or just to go on as normal (`Continue`). + +For example, it can be used to early-exit in `Iterator::try_for_each`: +```rust +let y: ControlFlow = it.try_for_each(|x| { + if x % 100 == 99 { + return ControlFlow::Break(x); + } + + ControlFlow::Continue(()) +}); +``` +While one could also use `Result` to do this, it can be confusing to use `Err` for what one would mentally consider a _successful_ early exit. Using a different type without those extra associations can help avoid mental dissonance while reading the code. + +You might also use it when exposing similar things yourself, just as a graph traversal or visitor, where you want the user to be able to choose to break early. + +## Definining your own `Result`-like type + +We've seen elsewhere in the book that `Result` is just an enum. Let's define our own to learn more about how `?` works. + +To start with, let's use this type: +```rust +enum MyResult { + Awesome(T), + Terrible(U) +} +``` + +That lets us do all the pattern matching things, but let's implement some more traits to support additional operators. + +### Supporting `?` via `Bubble` + +`Bubble` lets us define which values of our type let execution go on normally, and which should result in a short circuit. + +Here's a full implementation: +```rust +use std::ops::{ControlFlow, Bubble}; +impl Bubble for MyResult { + type Continue = T; + type Holder = as Bubble>::Holder; + fn branch(self) -> ControlFlow { + match self { + MyResult::Awesome(v) => ControlFlow::Continue(v), + MyResult::Terrible(e) => ControlFlow::Break(Err(e)), + } + } + fn continue_with(v: T) -> Self { + MyResult::Awesome(v) + } +} +``` + +Taking each of those associated items individually: +- The `Continue` type is the type that comes out when applying the `?` operator. For us it's just one of our generic types. If there was only one value that represented success, though, it might just be `()`. +- The `Holder` type represents the other possible states. For now we'll just use `Result`'s holder type, but note that this depends only on `U`, not on `T` -- because anything `Awesome` will be in the `Continue` type, it'll never hold a `T`. +- The `branch` method tells the `?` operator whether or not we need to early-exit for a value. Here we've said that `?` should produce the value from the `Awesome` variant and short circuit for `Terrible` values. +- One can also create an instance of our type from a value of the `Continue` type using the `continue_with` constructor. + +Because we used `Result`'s holder type, this is enough to use `?` on our type in a method that returns an appropriate `Result`: +```rust +fn foo() -> Result<(), f32> { + let _: () = MyResult::Terrible(1.1)?; + Ok(()) +} +``` + +### Consuming `?` via `Try` + +If we change that function to return `MyResult`, however, we'll get an error: +```rust +error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `Try`) + --> C:\src\rust\src\test\ui\try-operator-custom-bubble-and-try.rs:29:17 + | +LL | / fn foo() -> MyResult<(), f32> { +LL | | let _: () = MyResult::Terrible(1.1)?; + | | ^^^^^^^^^^^^^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `MyResult<(), f32>` +LL | | MyResult::Awesome(()) +LL | | } + | |_- this function should return `Result` or `Option` to accept `?` + | + = help: the trait `Try>` is not implemented for `MyResult<(), f32>` + = note: required by `from_holder` +``` + +So let's implement that one: +```rust +use std::ops::Try; +impl Try for MyResult { + fn from_holder(h: Self::Holder) -> Self { + match h { + Err(e) => MyResult::Terrible(e), + Ok(v) => match v {}, + } + } +} +``` + +This is much simpler, with just the one associated function. Because the holder is always an error, we'll always produce a `Terrible` result. (The extra `match v {}` is because that's uninhabited, but [`exhaustive_patterns`](https://github.com/rust-lang/rust/issues/51085) is not yet stable, so we can't just omit the `Ok` arm.) + +With this we can now use `?` on both `MyResult`s and `Result`s in a function returning `MyResult`: +```rust +fn foo() -> MyResult<(), f32> { + let _: () = MyResult::Terrible(1.1)?; + MyResult::Awesome(()) +} + +fn bar() -> MyResult<(), f32> { + let _: () = Err(1.1)?; + MyResult::Awesome(()) +} +``` + +### Avoiding interconversion with a custom `Holder` + +While interconversion isn't a problem for our custom result-like type, one might not always want it. For example, you might be making a type that short-circuits on something you think of as success, or just doesn't make sense as pass/fail so there isn't a meaningful "error" to provide. So let's see how we'd make a custom holder to handle that. + +A "holder" type can always be associated back to its canonical `Try` (and `Bubble`) type. This allows generic code to keep the "result-ness" or "option-ness" of a type while changing the `Continue` type. So while we only need to store a `U`, we'll need some sort of wrapper around it to keep its "myresult-ness". + +Conveniently, though, we don't need to define a new type for that: we can use our enum, but with an uninhabited type on one side. As `!` isn't stable yet, we'll use `std::convert::Infallible` as a canonical uninhabited type. (You may have seen it before in `TryFrom`, with `u64: TryFrom` since that conversion cannot fail.) + +First we need to change the `Holder` type in our `Bubble` implementation, and change the body of `branch` accordingly: +```rust +use std::convert::Infallible; +impl Bubble for MyResult { + ... no changes here ... + type Holder = MyResult; + fn branch(self) -> ControlFlow { + match self { + MyResult::Awesome(v) => ControlFlow::Continue(v), + MyResult::Terrible(e) => ControlFlow::Break(MyResult::Terrible(e)), + } + } + ... no changes here ... +} +``` + +As well as update our `Try` implementation for the new holder type: +```rust +impl Try for MyResult { + fn from_holder(h: Self::Holder) -> Self { + match h { + MyResult::Terrible(e) => MyResult::Terrible(e), + MyResult::Awesome(v) => match v {}, + } + } +} +``` + +We're not quite done, though; the compiler will let us know that we have more work to do: +```rust +error[E0277]: the trait bound `MyResult: BreakHolder` is not satisfied + --> C:\src\rust\src\test\ui\try-operator-custom-v2.rs:17:5 + | +LL | type Holder = MyResult; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `BreakHolder` is not implemented for `MyResult` + | + ::: C:\src\rust\library\core\src\ops\try.rs:102:18 + | +LL | type Holder: BreakHolder; + | --------------------- required by this bound in `std::ops::Bubble::Holder` +``` + +But that's a simple one: +```rust +impl BreakHolder for MyResult { + type Output = MyResult; +} +``` + +You can think of this trait as bringing back together the `T` and the `U` that `Bubble` had split into different associated types. + +With that we can still use `?` in both directions as in `foo` previously. And it means that `Iterator::try_find` will give us back a `MyResult` if we return it in the predicate: +```rust +let x = [1, 2].iter().try_find(|&&x| { + if x < 0 { + MyResult::Terrible("uhoh") + } else { + MyResult::Awesome(x % 2 == 0) + } +}); +assert!(matches!(x, MyResult::Awesome(Some(2)))); +``` + +As expected, the mixing in `bar` no longer compiles: +``` +help: the trait `Try>` is not implemented for `MyResult<(), f32>` +``` + +### Enabling `Result`-like error conversion + +`Result` allows mismatched error types so long as it can convert the source one into the type on the function. But if we try that with our current type, it won't work: +```rust +fn qux() -> MyResult<(), i64> { + let _: () = MyResult::Terrible(3_u8)?; + MyResult::Awesome(()) +} +``` + +That help message in the error from the previous section gives us a clue, however. Thus far we've been using the default generic parameter on `Try` which only allows an exact match to our declared `Bubble::Holder` type. But we can be more general if we want. Let's use `From`, like `Result` does: +```rust +impl> Try> for MyResult { + fn from_holder(h: MyResult) -> Self { + match h { + MyResult::Terrible(e) => MyResult::Terrible(From::from(e)), + MyResult::Awesome(v) => match v {}, + } + } +} +``` + +With that the `qux` example starts compiling successfully. + +(This can also be used to allow interconversion with holder types from other type constructors, but that's left as an exercise for the reader.) + + + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +## `ops::ControlFlow` + +```rust +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ControlFlow { + /// Continue in the loop, using the given value for the next iteration + Continue(C), + /// Exit the loop, yielding the given value + Break(B), +} +``` + +## The traits + +```rust +trait Bubble { + type Continue; + type Holder: BreakHolder; + fn continue_with(c: Self::Continue) -> Self; + fn branch(self) -> ControlFlow; +} + +trait BreakHolder { + type Output: Try; +} + +trait Try::Holder>: Bubble { + fn from_holder(h: H) -> Self; +} +``` + +## Desugaring `?` + +The previous desugaring of `x?` was + +```rust +match Try::into_result(x) { + Ok(v) => v, + Err(e) => Try::from_error(From::from(e)), +} +``` + +The new one is very similar: + +```rust +match Bubble::branch(x) { + ControlFlow::Continue(v) => v, + ControlFlow::Break(h) => Try::from_holder(h), +} +``` + +It's just left conversion (such as `From::from`) up to the implementation instead of forcing it in the desugar. + +## Standard implementations + +### `Result` + +```rust +impl ops::Bubble for Result { + type Continue = T; + type Holder = Result; + + fn continue_with(c: T) -> Self { + Ok(c) + } + + fn branch(self) -> ControlFlow { + match self { + Ok(c) => ControlFlow::Continue(c), + Err(e) => ControlFlow::Break(Err(e)), + } + } +} + +impl ops::BreakHolder for Result { + type Output = Result; +} + +#[unstable(feature = "try_trait_v2", issue = "42327")] +impl> ops::Try2021> for Result { + fn from_holder(x: Result) -> Self { + match x { + Err(e) => Err(From::from(e)), + } + } +} +``` + +### `Option` + +```rust +impl ops::Bubble for Option { + type Continue = T; + type Holder = Option; + + #[inline] + fn continue_with(c: T) -> Self { + Some(c) + } + + #[inline] + fn branch(self) -> ControlFlow { + match self { + Some(c) => ControlFlow::Continue(c), + None => ControlFlow::Break(None), + } + } +} + +impl ops::BreakHolder for Option { + type Output = Option; +} + +impl ops::Try for Option { + fn from_holder(x: Self::Holder) -> Self { + match x { + None => None, + } + } +} +``` + +### `Poll` + +These reuse `Result`'s holder type, so don't need `BreakHolder` implementations of their own, nor additional `Try` implementations on `Result`. + +```rust +impl ops::Bubble for Poll> { + type Continue = Poll; + type Holder = as ops::Bubble>::Holder; + + fn continue_with(c: Self::Continue) -> Self { + c.map(Ok) + } + + fn branch(self) -> ControlFlow { + match self { + Poll::Ready(Ok(x)) => ControlFlow::Continue(Poll::Ready(x)), + Poll::Ready(Err(e)) => ControlFlow::Break(Err(e)), + Poll::Pending => ControlFlow::Continue(Poll::Pending), + } + } +} + +impl> ops::Try> for Poll> { + fn from_holder(x: Result) -> Self { + match x { + Err(e) => Poll::Ready(Err(From::from(e))), + } + } +} +``` + +```rust +impl ops::Bubble for Poll>> { + type Continue = Poll>; + type Holder = as ops::Bubble>::Holder; + + fn continue_with(c: Self::Continue) -> Self { + c.map(|x| x.map(Ok)) + } + + fn branch(self) -> ControlFlow { + match self { + Poll::Ready(Some(Ok(x))) => ControlFlow::Continue(Poll::Ready(Some(x))), + Poll::Ready(Some(Err(e))) => ControlFlow::Break(Err(e)), + Poll::Ready(None) => ControlFlow::Continue(Poll::Ready(None)), + Poll::Pending => ControlFlow::Continue(Poll::Pending), + } + } +} + +impl> ops::Try> for Poll>> { + fn from_holder(x: Result) -> Self { + match x { + Err(e) => Poll::Ready(Some(Err(From::from(e)))), + } + } +} +``` + +## Making the accidental `Option` interconversion continue to work + +This is done with an extra implementation: +```rust +mod sadness { + use super::*; + + /// This is a remnant of the old `NoneError`, and is never going to be stabilized. + /// It's here as a snapshot of an oversight that allowed this to work in the past, + /// so we're stuck supporting it even though we'd really rather not. + #[unstable(feature = "legacy_try_trait", issue = "none")] + #[derive(Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)] + pub struct PleaseCallTheOkOrMethodToUseQuestionMarkOnOptionsInAMethodThatReturnsResult; + + #[unstable(feature = "try_trait_v2", issue = "42327")] + impl ops::Try> for Result + where + E: From, + { + fn from_holder(x: Option) -> Self { + match x { + None => Err(From::from( + PleaseCallTheOkOrMethodToUseQuestionMarkOnOptionsInAMethodThatReturnsResult, + )), + } + } + } +} +``` + +## Use in `Iterator` + +The provided implementation of `try_fold` is already just using `?` and `try{}`, so doesn't change. The only difference is the name of the associated type in the bound: +```rust +fn try_fold(&mut self, init: B, mut f: F) -> R +where + Self: Sized, + F: FnMut(B, Self::Item) -> R, + R: Try, +{ + let mut accum = init; + while let Some(x) = self.next() { + accum = f(accum, x)?; + } + try { accum } +} +``` + +The current (unstable) `try_find` allows any `Try` type as the return type of its predicate, but always returns a `Result`. + +One can take advantage of `BreakHolder` to still return `Result` when the predicate returned `Result`, but also to return `Option` when the predicate returned `Option` (and so on): +```rust +fn try_find( + &mut self, + f: F, +) -> >>::Output +where + Self: Sized, + F: FnMut(&Self::Item) -> R, + R: ops::Try, + R::Holder: ops::BreakHolder>, +{ + #[inline] + fn check(mut f: F) -> impl FnMut((), T) -> ControlFlow> + where + F: FnMut(&T) -> R, + R: Try, + { + move |(), x| match f(&x).branch() { + ControlFlow::Continue(false) => ControlFlow::CONTINUE, + ControlFlow::Continue(true) => ControlFlow::Break(Ok(x)), + ControlFlow::Break(h) => ControlFlow::Break(Err(h)), + } + } + + match self.try_fold((), check(f)) { + ControlFlow::Continue(()) => ops::Bubble::continue_with(None), + ControlFlow::Break(Ok(x)) => ops::Bubble::continue_with(Some(x)), + ControlFlow::Break(Err(h)) => Try::from_holder(h), + } +} +``` + + + +# Drawbacks +[drawbacks]: #drawbacks + +- While this handles a known accidental stabilization, it's possible that there's something else unknown that will keep this from being doable while meeting Rust's stringent stability guarantees. +- The extra complexity of this approach, compared to either of the alternatives considered the last time around, might not be worth it. +- This is the fourth attempt at a design in this space, so it might not be the right one either. +- As with all overloadable operators, users might implement this to do something weird. +- In situations where extensive interconversion is desired, this requires more implementations. + + + +# Rationale and alternatives +[rationale-and-alternatives]: #rationale-and-alternatives + +## Methods on `ControlFlow` + +On nightly there are are a variety of methods available on `ControlFlow`. However, none of them are needed for the stabilization of the traits, so they left out of this RFC. They can be considered by libs at a later point. + +There's a basic set of simple ones that could be included if desired, though: +```rust +impl ControlFlow { + fn is_break(&self) -> bool; + fn is_continue(&self) -> bool; + fn break_value(self) -> Option; + fn continue_value(self) -> Option; +} +``` + +## Traits for `ControlFlow` + +`ControlFlow` derives a variety of traits where they have obvious behaviour. It does not, however, derive `PartialOrd`/`Ord`. They're left out as it's unclear which order, if any, makes sense between the variants. + +For `Option`s, `None < Some(_)`, but for `Result`s, `Ok(_) < Err(_)`. So there's no definition for `ControlFlow` that's consistent with the isomorphism to both types. + +Leaving it out also leaves us free to change the ordering of the variants in the definition in case doing so can allow us to optimize the `?` operator. (For a similar previous experiment, see [PR #49499](https://github.com/rust-lang/rust/pull/49499).) + +## Was this considered last time? + +Interestingly, a [previous version](https://github.com/rust-lang/rfcs/blob/f89568b1fe5db4d01c4668e0d334d4a5abb023d8/text/0000-try-trait.md#using-an-associated-type-for-the-success-value) of RFC #1859 _did_ actually mention a two-trait solution, splitting the "associated type for ok" and "generic type for error" like is done here. It's no longer mentioned in the version that was merged. To speculate, it may have been unpopular due to a thought that an extra traits just for the associated type wasn't worth it. + +Current desires for the solution, however, have more requirements than were included in the RFC at the time of that version. Notably, the stabilized `Iterator::try_fold` method depends on being able to create a `Try` type from the accumulator. Including such a constructor on the trait with the associated type helps that separate trait provide value. + +Also, ok-wrapping was decided [in #70941](https://github.com/rust-lang/rust/issues/70941), which needs such a constructor, making this ["much more appealing"](https://github.com/rust-lang/rust/issues/42327#issuecomment-379882998). + +## Trait naming + +Bikeshed away! + +## Why a "holder" type is better than an "error" type + +Most importantly, for any type generic in its "continue type" it's easy to produce the holder type using an uninhabited type. That works for `Option` -- no `NoneError` residual type needed -- as well as for the `StrandFail` type from the experience report. And thanks to enum layout optimizations, there's no space overhead to doing this: `Option` is a ZST, and `Result` is no larger than `E` itself. So most of the time one will not need to define anything additional. + +In those cases where a separate type *is* needed, it's still easier to make a holder type because they're transient and thus can be opaque: there's no point at which a user is expected to *do* anything with a holder type other than convert it back into a known `Try` type. This is different from the previous design, where less-restrictive interconversion meant that anything could be exposed via a `Result`. That has lead to requests, [such as for `NoneError` to implement `Error`](https://github.com/rust-lang/rust/issues/46871#issuecomment-618186642), that are perfectly understandable given that the instances are exposed in `Result`s. As holder types aren't ever exposed like that, it would be fine for them to implement nothing but `BreakHolder` (and probably `Debug`), making them cheap to define and maintain. + +## Use of `!` + +This RFC uses `!` to be concise. It would work fine with `convert::Infallible` instead if `!` has not yet stabilized, though a few more match arms would be needed in the implementations. (For example, `Option::from_holder` would need `Some(c) => match c {}`.) + +## Moving away from the `Option`→`Result` interconversion + +We could consider doing an edition switch to make this no longer allowed. + +For example, we could have a different, never-stable `Try`-like trait used in old editions for the `?` desugaring. It could then have a blanket impl, plus the extra interconversion one. + +It's unclear that that's worth the effort, however, so this RFC is currently written to continue to support it going forward. Notably, removing it isn't enough to solve the annotation requirements, so the opportunity cost feels low. + +## Bounds on `Bubble::Holder` + +The bound for this associated type could be tightened as follows: +```rust +type Holder: BreakHolder; +``` + +That forces every type to be in bijection with its holder; however it's not clear that such a restriction is valuable. + +The implementation for `Poll>`, for example, works well with reusing `Result`'s holder type. The type *wants* to support the interconversion anyway, so forcing it to define a newtype for the holder is extra busywork. It might not even be possible for types outside `core` due to coherence. + +The advantage of the bijection, though, is that methods like `Iterator::try_find` would always return the "same" thing conceptually. But in generic code that's mostly irrelevant, and even with known types it's not a big deal. The holder type will be the same, so `?` would still work to go back to the original type if needed. And of course the type can always define a distinct holder if they're worried about it. + +## `BreakHolder` vs using GATs + +Generic Associated Types (GATs) may not be stable at the time of writing this RFC, but it's natural to ask if we'd like to use them if available, and thus should wait for them. + +Types like `Result` and `Option` support any (sized) type as their continue type, so GATs would be reasonable for them. However, with other possible types that might not be the case. For example, imagine supporting `?` on `process::ExitStatus`. Its holder type would likely only be `BreakHolder<()>`, since it cannot hold a custom payload and for it "success is defined as a zero exit status" (per `ExitStatus::success`). So requiring that these types support an arbitrary generic continue type may be overly restrictive, and thus the trait approach -- allowing bounding to just the types needed -- seems reasonable. + +(Note that RFC #1598 only [included lifetime-GATs, not type-GATs](https://rust-lang.github.io/rfcs/1598-generic_associated_types.html#associated-type-constructors-of-type-arguments). So it's likely that it would also be a very long wait for type-GATs.) + +## Default `H` on `Try` + +The default here is provided to make the homogeneous case simple. Along with its supertrait, that greatly simplifies the bound on `Iterator::try_fold`, for example. Just requiring `R: Try` is enough to break apart and rebuild a value of that type (as was the case with the previous trait). + +It's also convenient when implementing the trait in cases that don't need conversion, as seen with `Option`. + +## `Try::from_holder` vs `Holder::into_try` + +Either of these directions could be made to work. Indeed, an early experiment while drafting this had a method on `BreakHolder` that created the `Bubble` type (not just the associated type). However that was removed as unnecessary once `Try::from_holder` was added. + +A major advantage of the `Try::from_holder` direction is that it's more flexible with coherence when it comes to allowing other things to be converted into a new type being defined. That does come at the cost of higher restriction on allowing the new type to be converted into other things, but reusing a holder can also be used for that scenario. + +Converting a known holder into a generic `Bubble` type seems impossible (unless it's uninhabited), but consuming arbitrary holders could work -- imagine something like +```rust +impl> Try for LogAndIgnoreErrors { + fn from_holder(h: H) -> Self { + dbg!(h); + Self + } +} +``` +(Not that that's necessarily a good idea -- it's plausibly *too* generic. This RFC definitely isn't proposing it for the standard library.) + +And, ignoring the coherence implications, a major difference between the two sides is that the target type is typically typed out visibly (in a return type) whereas the source type (going into the `?`) is often the result of some called function. So it's preferrable for any behaviour extensions to be on the type that can more easily be seen in the code. + + + +# Prior art +[prior-art]: #prior-art + +Previous approaches used on nightly +- The original [`Carrier` trait](https://doc.rust-lang.org/1.16.0/core/ops/trait.Carrier.html) +- The next design with a [`Try` trait](https://doc.rust-lang.org/1.32.0/core/ops/trait.Try.html) (different from the one here) + +Thinking from the perspective of a [monad](https://doc.rust-lang.org/1.32.0/core/ops/trait.Try.html), `Bubble::continue_with` is similar to `return`, and `>::Output` is its type constructor. + + + +# Unresolved questions +[unresolved-questions]: #unresolved-questions + +Scope: +- Is `BreakHolder` worth it? There are a bunch of scenarios that might be interested in it, but they're all either currently-unstable or just future possibilities. +- Should this bring in anything from the future section, such as more things about `try {}`? + +Bikesheds: +- I've long liked [parasyte's "bubble" suggestion](https://internals.rust-lang.org/t/bikeshed-a-consise-verb-for-the-operator/7289/29?u=scottmcm) as a name, but maybe there's a better option. +- The "holder" name is really vague, and `BreakHolder` isn't much better. +- This uses `Try` mostly because that meant not touching all the `try_fold` implementations in the prototype. It's possible that name fits better on a different trait, or none of them. That trait as `from_holder`, which is most related to "yeet", so a name related to that might fit better for it. + + + +# Future possibilities +[future-possibilities]: #future-possibilities + +## Possibilities for `try{}` + +A core problem with [try blocks](https://doc.rust-lang.org/nightly/unstable-book/language-features/try-blocks.html) as implemented in nightly, is that they require their contextual type to be known. + +That is, the following never compiles, no matter the types of `x` and `y`: +```rust +let _ = try { + foo(x?); + bar(y?); + z +}; +``` + +This usually isn't a problem on stable, as the `?` usually has a contextual type from its function, but can still happen there in closures. + +But with the design in this RFC, an alternative desugaring becomes available which takes advantage of how the holder type preserves the "result-ness" (or whatever-ness) of the original value. That might turn the block above into something like the following: +```rust +fn helper>(h: H) -> >::Output { Try::from_holder(h) } + +'block: { + foo(match Bubble::branch(x) { + ControlFlow::Continue(c) => c, + ControlFlow::Break(h) => break 'block helper(h), + }); + bar(match Bubble::branch(y) { + ControlFlow::Continue(c) => c, + ControlFlow::Break(h) => break 'block helper(h), + }); + Bubble::continue_with(z) +} +``` +(It's untested whether the inference engine is smart enough to pick the appropriate `C` with just that -- the `Output` associated type is constrained to have a `Continue` type matching the generic parameter, and that `Continue` type needs to match that of `z`, so it's possible. But hopefully this communicates the idea, even if an actual implementation might need to more specifically introduce type variables or something.) + +That way it could compile so long as the output types of the holders matched. For example, [these uses in rustc](https://github.com/rust-lang/rust/blob/7cf205610e1310897f43b35713a42459e8b40c64/compiler/rustc_codegen_ssa/src/back/linker.rs#L529-L573) would work without the extra annotation. + +Now, of course that wouldn't cover anything. It wouldn't work with anything needing error conversion, for example, but annotation is also unavoidable in those cases -- there's no reasonable way for the compiler to pick "the" type into which all the errors are convertible. + +So a future RFC could define a way (syntax, code inspection, heuristics, who knows) to pick which of the desugarings would be best. This RFC declines to even brainstorm possibilities for doing so. + +*Note that the `?` desugaring in nightly is already different depending whether it's inside a `try {}` (since it needs to block-break instead of `return`), so making it slightly more different shouldn't have excessive implementation cost.* + +## Possibilities for `yeet` + +As previously mentioned, this RFC neither defines nor proposes a `yeet` operator. However, like the previous design could support one with its `Try::from_error`, it's important that this design would be sufficient to support it. + +Because this "holder" design carries along the "result-ness" or "option-ness" or similar, it means there are two possibilities for a desugaring. + +- It could directly take the holder type, so `yeet e` would desugar directly to `Try::from_holder(e)`. +- It could put the argument into a special holder type, so `yeet e` would desugar to something like `Try::from_holder(Yeet(e))`. + +These have various implications -- like `yeet None`/`yeet`, `yeet Err(ErrorKind::NotFound)`/`yeet ErrorKind::NotFound.into()`, etc -- but thankfully this RFC doesn't need to discuss those. (And please don't do so in the comments either.) + + From 50244adc9cca3b3842533694191083f8525f0f1f Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 10 Jan 2021 11:34:05 -0800 Subject: [PATCH 02/12] Apply some fixes and feedback from zulip --- text/0000-try-trait-v2.md | 106 ++++++++++++++++++++++++++------------ 1 file changed, 73 insertions(+), 33 deletions(-) diff --git a/text/0000-try-trait-v2.md b/text/0000-try-trait-v2.md index f2f61e5094e..9349e96a082 100644 --- a/text/0000-try-trait-v2.md +++ b/text/0000-try-trait-v2.md @@ -67,17 +67,17 @@ let y: ControlFlow = it.try_for_each(|x| { ``` While one could also use `Result` to do this, it can be confusing to use `Err` for what one would mentally consider a _successful_ early exit. Using a different type without those extra associations can help avoid mental dissonance while reading the code. -You might also use it when exposing similar things yourself, just as a graph traversal or visitor, where you want the user to be able to choose to break early. +You might also use it when exposing similar things yourself, such as a graph traversal or visitor, where you want the user to be able to choose to break early. -## Definining your own `Result`-like type +## Defining your own `Result`-like type We've seen elsewhere in the book that `Result` is just an enum. Let's define our own to learn more about how `?` works. To start with, let's use this type: ```rust enum MyResult { - Awesome(T), - Terrible(U) + Terrific(T), + Unfortunate(U) } ``` @@ -92,29 +92,29 @@ Here's a full implementation: use std::ops::{ControlFlow, Bubble}; impl Bubble for MyResult { type Continue = T; - type Holder = as Bubble>::Holder; + type Holder = as Bubble>::Holder; fn branch(self) -> ControlFlow { match self { - MyResult::Awesome(v) => ControlFlow::Continue(v), - MyResult::Terrible(e) => ControlFlow::Break(Err(e)), + MyResult::Terrific(v) => ControlFlow::Continue(v), + MyResult::Unfortunate(e) => ControlFlow::Break(Err(e)), } } fn continue_with(v: T) -> Self { - MyResult::Awesome(v) + MyResult::Terrific(v) } } ``` Taking each of those associated items individually: - The `Continue` type is the type that comes out when applying the `?` operator. For us it's just one of our generic types. If there was only one value that represented success, though, it might just be `()`. -- The `Holder` type represents the other possible states. For now we'll just use `Result`'s holder type, but note that this depends only on `U`, not on `T` -- because anything `Awesome` will be in the `Continue` type, it'll never hold a `T`. -- The `branch` method tells the `?` operator whether or not we need to early-exit for a value. Here we've said that `?` should produce the value from the `Awesome` variant and short circuit for `Terrible` values. +- The `Holder` type represents the other possible states. For now we'll just use `Result`'s holder type, but will come back to it in a future section. +- The `branch` method tells the `?` operator whether or not we need to early-exit for a value. Here we've said that `?` should produce the value from the `Terrific` variant and short circuit for `Unfortunate` values. - One can also create an instance of our type from a value of the `Continue` type using the `continue_with` constructor. Because we used `Result`'s holder type, this is enough to use `?` on our type in a method that returns an appropriate `Result`: ```rust fn foo() -> Result<(), f32> { - let _: () = MyResult::Terrible(1.1)?; + let _: () = MyResult::Unfortunate(1.1)?; Ok(()) } ``` @@ -127,9 +127,9 @@ error[E0277]: the `?` operator can only be used in a function that returns `Resu --> C:\src\rust\src\test\ui\try-operator-custom-bubble-and-try.rs:29:17 | LL | / fn foo() -> MyResult<(), f32> { -LL | | let _: () = MyResult::Terrible(1.1)?; +LL | | let _: () = MyResult::Unfortunate(1.1)?; | | ^^^^^^^^^^^^^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `MyResult<(), f32>` -LL | | MyResult::Awesome(()) +LL | | MyResult::Terrific(()) LL | | } | |_- this function should return `Result` or `Option` to accept `?` | @@ -143,25 +143,25 @@ use std::ops::Try; impl Try for MyResult { fn from_holder(h: Self::Holder) -> Self { match h { - Err(e) => MyResult::Terrible(e), + Err(e) => MyResult::Unfortunate(e), Ok(v) => match v {}, } } } ``` -This is much simpler, with just the one associated function. Because the holder is always an error, we'll always produce a `Terrible` result. (The extra `match v {}` is because that's uninhabited, but [`exhaustive_patterns`](https://github.com/rust-lang/rust/issues/51085) is not yet stable, so we can't just omit the `Ok` arm.) +This is much simpler, with just the one associated function. Because the holder is always an error, we'll always produce a `Unfortunate` result. (The extra `match v {}` is because that's uninhabited, but [`exhaustive_patterns`](https://github.com/rust-lang/rust/issues/51085) is not yet stable, so we can't just omit the `Ok` arm.) With this we can now use `?` on both `MyResult`s and `Result`s in a function returning `MyResult`: ```rust fn foo() -> MyResult<(), f32> { - let _: () = MyResult::Terrible(1.1)?; - MyResult::Awesome(()) + let _: () = MyResult::Unfortunate(1.1)?; + MyResult::Terrific(()) } fn bar() -> MyResult<(), f32> { let _: () = Err(1.1)?; - MyResult::Awesome(()) + MyResult::Terrific(()) } ``` @@ -169,7 +169,9 @@ fn bar() -> MyResult<(), f32> { While interconversion isn't a problem for our custom result-like type, one might not always want it. For example, you might be making a type that short-circuits on something you think of as success, or just doesn't make sense as pass/fail so there isn't a meaningful "error" to provide. So let's see how we'd make a custom holder to handle that. -A "holder" type can always be associated back to its canonical `Try` (and `Bubble`) type. This allows generic code to keep the "result-ness" or "option-ness" of a type while changing the `Continue` type. So while we only need to store a `U`, we'll need some sort of wrapper around it to keep its "myresult-ness". +As we saw in the `Bubble::branch` implementation, the holder type preserves the values that weren't returned in the `Continue` type. Thus for us it'll depend only on `U`, never on `T`. + +Also, a holder type can always be associated back to its canonical `Try` (and `Bubble`) type. This allows generic code to keep the "result-ness" or "option-ness" of a type while changing the `Continue` type. So while we only need to store a `U`, we'll need some sort of wrapper around it to keep its "myresult-ness". Conveniently, though, we don't need to define a new type for that: we can use our enum, but with an uninhabited type on one side. As `!` isn't stable yet, we'll use `std::convert::Infallible` as a canonical uninhabited type. (You may have seen it before in `TryFrom`, with `u64: TryFrom` since that conversion cannot fail.) @@ -181,8 +183,8 @@ impl Bubble for MyResult { type Holder = MyResult; fn branch(self) -> ControlFlow { match self { - MyResult::Awesome(v) => ControlFlow::Continue(v), - MyResult::Terrible(e) => ControlFlow::Break(MyResult::Terrible(e)), + MyResult::Terrific(v) => ControlFlow::Continue(v), + MyResult::Unfortunate(e) => ControlFlow::Break(MyResult::Unfortunate(e)), } } ... no changes here ... @@ -194,8 +196,8 @@ As well as update our `Try` implementation for the new holder type: impl Try for MyResult { fn from_holder(h: Self::Holder) -> Self { match h { - MyResult::Terrible(e) => MyResult::Terrible(e), - MyResult::Awesome(v) => match v {}, + MyResult::Unfortunate(e) => MyResult::Unfortunate(e), + MyResult::Terrific(v) => match v {}, } } } @@ -228,12 +230,12 @@ With that we can still use `?` in both directions as in `foo` previously. And i ```rust let x = [1, 2].iter().try_find(|&&x| { if x < 0 { - MyResult::Terrible("uhoh") + MyResult::Unfortunate("uhoh") } else { - MyResult::Awesome(x % 2 == 0) + MyResult::Terrific(x % 2 == 0) } }); -assert!(matches!(x, MyResult::Awesome(Some(2)))); +assert!(matches!(x, MyResult::Terrific(Some(2)))); ``` As expected, the mixing in `bar` no longer compiles: @@ -246,8 +248,8 @@ help: the trait `Try>` is not implemented for `M `Result` allows mismatched error types so long as it can convert the source one into the type on the function. But if we try that with our current type, it won't work: ```rust fn qux() -> MyResult<(), i64> { - let _: () = MyResult::Terrible(3_u8)?; - MyResult::Awesome(()) + let _: () = MyResult::Unfortunate(3_u8)?; + MyResult::Terrific(()) } ``` @@ -256,8 +258,8 @@ That help message in the error from the previous section gives us a clue, howeve impl> Try> for MyResult { fn from_holder(h: MyResult) -> Self { match h { - MyResult::Terrible(e) => MyResult::Terrible(From::from(e)), - MyResult::Awesome(v) => match v {}, + MyResult::Unfortunate(e) => MyResult::Unfortunate(From::from(e)), + MyResult::Terrific(v) => match v {}, } } } @@ -287,10 +289,10 @@ For implementation-oriented RFCs (e.g. for compiler internals), this section sho ```rust #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ControlFlow { - /// Continue in the loop, using the given value for the next iteration - Continue(C), /// Exit the loop, yielding the given value Break(B), + /// Continue in the loop, using the given value for the next iteration + Continue(C), } ``` @@ -463,6 +465,36 @@ impl> ops::Try> for Poll>> { } ``` +### `ControlFlow` + +```rust +impl ops::Bubble for ControlFlow { + type Continue = C; + type Holder = ControlFlow; + fn continue_with(c: C) -> Self { + ControlFlow::Continue(c) + } + fn branch(self) -> ControlFlow { + match self { + ControlFlow::Continue(c) => ControlFlow::Continue(c), + ControlFlow::Break(b) => ControlFlow::Break(ControlFlow::Break(b)), + } + } +} + +impl ops::BreakHolder for ControlFlow { + type Output = ControlFlow; +} + +impl ops::Try for ControlFlow { + fn from_holder(x: Self::Holder) -> Self { + match x { + ControlFlow::Break(b) => ControlFlow::Break(b), + } + } +} +``` + ## Making the accidental `Option` interconversion continue to work This is done with an extra implementation: @@ -572,9 +604,17 @@ Why should we *not* do this? # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives +## Why `ControlFlow` pulls its weight + +The previous RFC discussed having such a type, but ended up deciding that defining a new type for the desugar wasn't worth it, and just used `Result`. + +This RFC does use a new type because one already [exists in nightly](https://doc.rust-lang.org/nightly/std/ops/enum.ControlFlow.html) under [the `control_flow_enum` feature gate](https://github.com/rust-lang/rust/issues/75744). +It's being used in [the library](https://github.com/rust-lang/rust/blob/fd34606ddf02d1e9364e459b373a6ad665c3d8a4/library/core/src/iter/traits/iterator.rs#L2239-L2252) and [the compiler](https://github.com/rust-lang/rust/blob/c609b2eaf323186a1167ec1a9ffa69a7d4a5b1b9/compiler/rustc_middle/src/ty/fold.rs#L184-L206), demonstrating that it's useful beyond just this desugaring, so the desugar might as well use it too for extra clarity. +There are also [ecosystem changes waiting on something like it](https://github.com/rust-itertools/itertools/issues/469#issuecomment-677729589), so it's not just a compiler-internal need. + ## Methods on `ControlFlow` -On nightly there are are a variety of methods available on `ControlFlow`. However, none of them are needed for the stabilization of the traits, so they left out of this RFC. They can be considered by libs at a later point. +On nightly there are are a [variety of methods](https://doc.rust-lang.org/nightly/std/ops/enum.ControlFlow.html#implementations) available on `ControlFlow`. However, none of them are needed for the stabilization of the traits, so they left out of this RFC. They can be considered by libs at a later point. There's a basic set of simple ones that could be included if desired, though: ```rust From b1e406699d67f44204b448319e1b109150e95c0c Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 10 Jan 2021 15:58:24 -0800 Subject: [PATCH 03/12] Add two missing returns in the desugarings Thanks Nokel81 for spotting this. --- text/0000-try-trait-v2.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0000-try-trait-v2.md b/text/0000-try-trait-v2.md index 9349e96a082..9313649c928 100644 --- a/text/0000-try-trait-v2.md +++ b/text/0000-try-trait-v2.md @@ -322,7 +322,7 @@ The previous desugaring of `x?` was ```rust match Try::into_result(x) { Ok(v) => v, - Err(e) => Try::from_error(From::from(e)), + Err(e) => return Try::from_error(From::from(e)), } ``` @@ -331,7 +331,7 @@ The new one is very similar: ```rust match Bubble::branch(x) { ControlFlow::Continue(v) => v, - ControlFlow::Break(h) => Try::from_holder(h), + ControlFlow::Break(h) => return Try::from_holder(h), } ``` From 05a1c41a015df77d670887762c8e410b6936cf7f Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 10 Jan 2021 18:50:04 -0800 Subject: [PATCH 04/12] Cross-link the other features mentioned in the summary --- text/0000-try-trait-v2.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/text/0000-try-trait-v2.md b/text/0000-try-trait-v2.md index 9313649c928..b03943e2876 100644 --- a/text/0000-try-trait-v2.md +++ b/text/0000-try-trait-v2.md @@ -15,7 +15,9 @@ while addressing the discovered shortcomings of the currently-implemented soluti as well as enabling new scenarios. *This is forward-looking to be compatible with other features, -like `try {}` blocks or `yeet e` expressions or `Iterator::try_find`, +like [`try {}`](https://doc.rust-lang.org/nightly/unstable-book/language-features/try-blocks.html) blocks +or [`yeet e`](https://twitter.com/josh_triplett/status/1248658754976927750) expressions +or [`Iterator::try_find`](https://github.com/rust-lang/rust/issues/63178), but the statuses of those features are **not** themselves impacted by this RFC.* # Motivation From aa62c5bb2d0f4d2c683250d7baaa36591babda9e Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 10 Jan 2021 20:11:35 -0800 Subject: [PATCH 05/12] Fix more oopses Thanks mbartlett21 & ehuss & tmccombs! --- text/0000-try-trait-v2.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/text/0000-try-trait-v2.md b/text/0000-try-trait-v2.md index b03943e2876..45cf47b31ee 100644 --- a/text/0000-try-trait-v2.md +++ b/text/0000-try-trait-v2.md @@ -29,7 +29,7 @@ However, new information has come in since the previous RFC, making people wish - An [experience report](https://github.com/rust-lang/rust/issues/42327#issuecomment-366840247) in the tracking issue mentioned that it's annoying to need to make a residual type. - The `try {}` conversations have wished for more source information to flow through `?` so that fewer annotations would be required. - Similarly, it's no longer clear that `From` should be part of the `?` desugaring for _all_ types. It's both more flexible -- making inference difficult -- and more restrictive -- especially without specialization -- than is always desired. -- Various library methods, such as `try_map` for arrays ([PR #79713](https://github.com/rust-lang/rust/pull/79713#issuecomment-739075171)), would like to be able to do HKT-like things to produce their result types. (For example, `Iterator::try_find` wants to be able to return a `Foo` from a predicate that returned a `Foo`.) +- Various library methods, such as `try_map` for arrays ([PR #79713](https://github.com/rust-lang/rust/pull/79713#issuecomment-739075171)), would like to be able to do HKT-like things to produce their result types. (For example, `Iterator::try_find` wants to be able to return a `Foo>` from a predicate that returned a `Foo`.) - Using the "error" terminology is a poor fit for other potential implementations of the trait. - It turned out that the current solution accidentally stabilized more interconversion than expected, so a more restricted form may be warranted. @@ -49,9 +49,9 @@ Why are we doing this? What use cases does it support? What is the expected outc This is a simple enum: ```rust -struct ControlFlow { - Break(B), - Continue(C), +enum ControlFlow { + Break(B), + Continue(C), } ``` @@ -215,8 +215,8 @@ LL | type Holder = MyResult; | ::: C:\src\rust\library\core\src\ops\try.rs:102:18 | -LL | type Holder: BreakHolder; - | --------------------- required by this bound in `std::ops::Bubble::Holder` +LL | type Holder: BreakHolder; + | --------------------------- required by this bound in `std::ops::Bubble::Holder` ``` But that's a simple one: @@ -309,7 +309,7 @@ trait Bubble { } trait BreakHolder { - type Output: Try; + type Output: Try; } trait Try::Holder>: Bubble { @@ -365,7 +365,7 @@ impl ops::BreakHolder for Result { } #[unstable(feature = "try_trait_v2", issue = "42327")] -impl> ops::Try2021> for Result { +impl> ops::Try> for Result { fn from_holder(x: Result) -> Self { match x { Err(e) => Err(From::from(e)), From 09d5e5ba2530575baf3c3586fce9739be9f901cd Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Tue, 12 Jan 2021 23:27:36 -0800 Subject: [PATCH 06/12] Add some new sections and examples to the Guide-level explanation --- text/0000-try-trait-v2.md | 172 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 167 insertions(+), 5 deletions(-) diff --git a/text/0000-try-trait-v2.md b/text/0000-try-trait-v2.md index 45cf47b31ee..488144d5eab 100644 --- a/text/0000-try-trait-v2.md +++ b/text/0000-try-trait-v2.md @@ -71,7 +71,9 @@ While one could also use `Result` to do this, it can be confusing to use `Err` f You might also use it when exposing similar things yourself, such as a graph traversal or visitor, where you want the user to be able to choose to break early. -## Defining your own `Result`-like type +## Using this with your own types + +### Defining your own `Result`-like type We've seen elsewhere in the book that `Result` is just an enum. Let's define our own to learn more about how `?` works. @@ -85,7 +87,7 @@ enum MyResult { That lets us do all the pattern matching things, but let's implement some more traits to support additional operators. -### Supporting `?` via `Bubble` +#### Supporting `?` via `Bubble` `Bubble` lets us define which values of our type let execution go on normally, and which should result in a short circuit. @@ -121,7 +123,7 @@ fn foo() -> Result<(), f32> { } ``` -### Consuming `?` via `Try` +#### Consuming `?` via `Try` If we change that function to return `MyResult`, however, we'll get an error: ```rust @@ -167,7 +169,7 @@ fn bar() -> MyResult<(), f32> { } ``` -### Avoiding interconversion with a custom `Holder` +#### Avoiding interconversion with a custom `Holder` While interconversion isn't a problem for our custom result-like type, one might not always want it. For example, you might be making a type that short-circuits on something you think of as success, or just doesn't make sense as pass/fail so there isn't a meaningful "error" to provide. So let's see how we'd make a custom holder to handle that. @@ -245,7 +247,7 @@ As expected, the mixing in `bar` no longer compiles: help: the trait `Try>` is not implemented for `MyResult<(), f32>` ``` -### Enabling `Result`-like error conversion +#### Enabling `Result`-like error conversion `Result` allows mismatched error types so long as it can convert the source one into the type on the function. But if we try that with our current type, it won't work: ```rust @@ -271,6 +273,166 @@ With that the `qux` example starts compiling successfully. (This can also be used to allow interconversion with holder types from other type constructors, but that's left as an exercise for the reader.) +### Using `?` with a non-generic type + +The type in the previous discussion was very flexible, so let's take the opportunity to look at the other end of the spectrum at a type with no generic parameters. + +We'll make a `ResultCode` type that works like is common in C: it's an integer internally, with success represented by zero and any non-zero value being a different error. + +The type itself can just a normal newtype, and let's give it a constant for the success value: +```rust +#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] +pub struct ResultCode(pub i32); +impl ResultCode { + pub const SUCCESS: Self = ResultCode(0); +} +``` +Users are expected to produce, operate on, and consume this type, so it gets the a usual list of traits. It doesn't need to maintain any invariants in it either, so the field might as well just be public -- no need for accessors. + +We decided earlier that zero is the only success value, so there's nothing interesting to provide out of `?` if the result code happens to be the success one. So we might as well use `()` as our `Continue` type. + +Now what should the holder look like? With no values of generic types, the holder won't need to be generic either. It needs to represent the non-zero values, so let's make it a newtype around `NonZeroI32`: +```rust +use std::num::NonZeroI32; +pub struct ResultCodeHolder(NonZeroI32); +``` +This type is really only there to be used transiently inside desugarings. There's no expectation that a user will ever need to touch one -- they'll be working with `ResultCode`s. So we don't need to offer any way to produce or consume these directly; it can just be opaque. + +*(Aside: it doesn't* have *to be completely opaque, however. In a real library it might want to be at least `Debug`+`Copy` for politeness, for example. Or it could be developed out into a "full" type with a `Display` which shows the description from C's `strerror`. But for the purpose of this RFC, there's no need for all that.)* + +With those types decided for the two sides of the control flow, we can go ahead and implement `Bubble`: +```rust +impl Bubble for ResultCode { + type Continue = (); + type Holder = ResultCodeHolder; + fn branch(self) -> ControlFlow { + match NonZeroI32::new(self.0) { + Some(e) => ControlFlow::Break(ResultCodeHolder(e)), + None => ControlFlow::Continue(()), + } + } + fn continue_with((): ()) -> Self { + Self::SUCCESS + } +} +``` + +Because `ResultCode` isn't generic, there's no way for `BreakHolder` to give out different types for different "keep going" payloads. Thus it's only defined for a `()` continue type: +```rust +impl BreakHolder<()> for ResultCodeHolder { + type Output = ResultCode; +} +``` +This means that we won't be able to use `ResultCode` with `Iterator::try_find` -- since there's no way for it to hold a `bool` or an `Option` -- but that's fine. Interestingly, it's still usable with `Iterator::try_for_each`, since that only requires `Continue = ()`. + +The implementation for `Try` is also simple: +```rust +impl Try for ResultCode { + fn from_holder(h: ResultCodeHolder) -> Self { + Self(h.0.into()) + } +} +``` + +And with that we have enough to try out our new type: +```rust +fn external_one() -> ResultCode; +fn external_two() -> ResultCode; +fn demo() -> ResultCode { + external_one()?; + external_two()?; + ResultCode::SUCCESS +} +``` + +*(Aside: As a nice bonus, the use of a `NonZero` type in the holder means that `::branch` [compiles down to a nop](https://rust.godbolt.org/z/GxeYax) on the current nightly. Thanks, enum layout optimizations!)* + +## Using these traits in generic code + +### When the generic parameter has the `Continue` type you need + +`Iterator::try_fold` has been stable to call (but not to implement) for a while now. To illustrate the flow through the traits in this RFC, lets implement our own version without using any of the sugar. + +As a reminder, an infallible version of a fold looks something like this: +```rust +fn simple_fold( + iter: impl Iterator, + mut accum: A, + mut f: impl FnMut(A, T) -> A, +) -> A { + for x in iter { + accum = f(accum, x); + } + accum +} +``` + +So instead of `f` returning just an `A`, we'll need it to return some other type that produces an `A` in the "don't short circuit" path. Conveniently, that's also the type we need to return from the function. + +Let's add a new generic parameter `R` for that type, and bound it to the continue type that we want: +```rust +fn simple_try_fold_1>( + iter: impl Iterator, + mut accum: A, + mut f: impl FnMut(A, T) -> R, +) -> R { + todo!() +} +``` + +Conveniently, `Bubble` is also the trait we need to get the updated accumulator from `f`'s return value and return the result if we manage to get through the entire iterator: +```rust +fn simple_try_fold_2>( + iter: impl Iterator, + mut accum: A, + mut f: impl FnMut(A, T) -> R, +) -> R { + for x in iter { + let cf = f(accum, x).branch(); + match cf { + ControlFlow::Continue(a) => accum = a, + ControlFlow::Break(_) => todo!(), + } + } + R::continue_with(accum) +} +``` + +Just `Bubble` isn't enough, though. We also need `Try` here in order to convert the holder inside the `Break` variant back into `R`. But because `Bubble` is a supertrait of `Try`, we can just change the bound rather than adding a second one: +```rust +pub fn simple_try_fold>( + iter: impl Iterator, + mut accum: A, + mut f: impl FnMut(A, T) -> R, +) -> R { + for x in iter { + let cf = f(accum, x).branch(); + match cf { + ControlFlow::Continue(a) => accum = a, + ControlFlow::Break(h) => return R::from_holder(h), + } + } + R::continue_with(accum) +} +``` + +### When you need to switch to a different `Continue` type + +You might have noticed that even the final `simple_try_fold` didn't need to mention `BreakHolder`. That's because it's only needed if you want to return a type from the same family, but with a different continue type. + +`Option::zip` stabilized a few releases ago, so here's a demonstration of a more-generalized version. To restrict the two inputs to the same family, it requires that the holder types match. Then it uses `BreakHolder` to get an output type from that same family. + +```rust +fn zip_demo>( + a: impl Bubble, + b: impl Bubble, +) -> >::Output { + Bubble::continue_with((a?, b?)) +} +``` + +Here `Try` doesn't need to be mentioned explicitly, as the `BreakHolder::Output` type is always bound by `Try`. + -That lets us do all the pattern matching things, but let's implement some more traits to support additional operators. -#### Supporting `?` via `Bubble` +## The `Try` trait -`Bubble` lets us define which values of our type let execution go on normally, and which should result in a short circuit. +The `ops::Try` trait describes a type's behavior when used with the `?` operator, like how the `ops::Add` trait describes its behavior when used with the `+` operator. -Here's a full implementation: -```rust -use std::ops::{ControlFlow, Bubble}; -impl Bubble for MyResult { - type Continue = T; - type Holder = as Bubble>::Holder; - fn branch(self) -> ControlFlow { - match self { - MyResult::Terrific(v) => ControlFlow::Continue(v), - MyResult::Unfortunate(e) => ControlFlow::Break(Err(e)), - } - } - fn continue_with(v: T) -> Self { - MyResult::Terrific(v) - } -} -``` +At its core, the `?` operator is about splitting a type into its two parts: -Taking each of those associated items individually: -- The `Continue` type is the type that comes out when applying the `?` operator. For us it's just one of our generic types. If there was only one value that represented success, though, it might just be `()`. -- The `Holder` type represents the other possible states. For now we'll just use `Result`'s holder type, but will come back to it in a future section. -- The `branch` method tells the `?` operator whether or not we need to early-exit for a value. Here we've said that `?` should produce the value from the `Terrific` variant and short circuit for `Unfortunate` values. -- One can also create an instance of our type from a value of the `Continue` type using the `continue_with` constructor. +- The *output* that will be returned from the `?` expression, with which the program will continue, and +- The *residual* that will be returned to the calling code, as an early exit from the normal flow. -Because we used `Result`'s holder type, this is enough to use `?` on our type in a method that returns an appropriate `Result`: -```rust -fn foo() -> Result<(), f32> { - let _: () = MyResult::Unfortunate(1.1)?; - Ok(()) -} -``` +(Oxford's definition for a residual is "a quantity remaining after other things have been subtracted or allowed for", thus the use here.) -#### Consuming `?` via `Try` +The `Try` trait also has facilities for rebuilding a type from either of its parts. This is needed to build the final return value from a function, both in `?` and in methods generic over multiple types implementing `Try`. -If we change that function to return `MyResult`, however, we'll get an error: -```rust -error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `Try`) - --> C:\src\rust\src\test\ui\try-operator-custom-bubble-and-try.rs:29:17 - | -LL | / fn foo() -> MyResult<(), f32> { -LL | | let _: () = MyResult::Unfortunate(1.1)?; - | | ^^^^^^^^^^^^^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `MyResult<(), f32>` -LL | | MyResult::Terrific(()) -LL | | } - | |_- this function should return `Result` or `Option` to accept `?` - | - = help: the trait `Try>` is not implemented for `MyResult<(), f32>` - = note: required by `from_holder` -``` +Here's a quick overview of a few standard types which implement `Try`, their corresponding output and residual types, and the functions which convert between them. +(Full details will come later; the goal for now is just to get the general idea.) -So let's implement that one: -```rust -use std::ops::Try; -impl Try for MyResult { - fn from_holder(h: Self::Holder) -> Self { - match h { - Err(e) => MyResult::Unfortunate(e), - Ok(v) => match v {}, - } - } -} +```text ++-------------+ +-------------------+ +-------------------+ +| Try::Output | | Try Type | | Try::Residual | ++-------------+ Try::branch is Continue +-------------------+ Try::branch is Break +-------------------+ +| T | <------------------------ | Result | ---------------------> | Result | +| T | | Option | | Option | +| C | ------------------------> | ControlFlow | <--------------------- | ControlFlow | ++-------------+ Try::from_output +-------------------+ Try::from_residual +-------------------+ ``` -This is much simpler, with just the one associated function. Because the holder is always an error, we'll always produce a `Unfortunate` result. (The extra `match v {}` is because that's uninhabited, but [`exhaustive_patterns`](https://github.com/rust-lang/rust/issues/51085) is not yet stable, so we can't just omit the `Ok` arm.) +If you've used `?`-on-`Result` before, that output type is likely unsurprising. Since it's given out directly from the operator, there's not much of a choice. -With this we can now use `?` on both `MyResult`s and `Result`s in a function returning `MyResult`: -```rust -fn foo() -> MyResult<(), f32> { - let _: () = MyResult::Unfortunate(1.1)?; - MyResult::Terrific(()) -} +The residual types, however, are somewhat more interesting. Code using `?` doesn't see them directly -- their usage is hidden inside the desugaring -- so there are more possibilities available. So why are we using these ones specifically? -fn bar() -> MyResult<(), f32> { - let _: () = Err(1.1)?; - MyResult::Terrific(()) -} -``` +Most importantly, this gives each family of types (`Result`s, `Option`s, `ControlFlow`s) their own *distinct* residual type. That avoids unrestricted *interconversion* between the different types, the ability to arbitrarily mix them in the same method. For example, it was mentioned earlier that just because a `ControlFlow::Break` is also an early exit, that doesn't mean that it should be allowed to consider it a `Result::Err` -- it might be a success, conceptually. So by giving `ControlFlow` and `Result<_, X>` different residual types, it becomes a compilation error to use the `?` operator on a `ControlFlow` in a method which returns a `Result`. (There are also ways to allow interconversion is specific situations where it's desirable.) -#### Avoiding interconversion with a custom `Holder` +> 🏗️ Note for those familiar with the previous RFC 🏗️ +> +> This is the most critical semantic difference. Structurally this definition of the trait is very similar to the previous -- there's still a method splitting the type into a discriminated union between two associated types, and constructors to rebuild it from them. But by keeping the "result-ness" or "option-ness" in the residual type, it gives extra control over interconversion that wasn't possible before. The changes other than this are comparatively minor, typically either rearrangements to work with that or renamings to change the vocabulary used in the trait. -While interconversion isn't a problem for our custom result-like type, one might not always want it. For example, you might be making a type that short-circuits on something you think of as success, or just doesn't make sense as pass/fail so there isn't a meaningful "error" to provide. So let's see how we'd make a custom holder to handle that. +Using `!` is then just a convenient yet efficient way to create those residual types. It's nice as a user, too, not to need to understand an additional type. Just the same "it can't be that one" pattern that's also used in `TryFrom`, where for example `i32::try_from(10_u8)` gives a `Result`, since it's a widening conversion which cannot fail. Note that there's nothing special going on with `!` here -- any uninhabited `enum` would work fine. -As we saw in the `Bubble::branch` implementation, the holder type preserves the values that weren't returned in the `Continue` type. Thus for us it'll depend only on `U`, never on `T`. -Also, a holder type can always be associated back to its canonical `Try` (and `Bubble`) type. This allows generic code to keep the "result-ness" or "option-ness" of a type while changing the `Continue` type. So while we only need to store a `U`, we'll need some sort of wrapper around it to keep its "myresult-ness". +## How error conversion works -Conveniently, though, we don't need to define a new type for that: we can use our enum, but with an uninhabited type on one side. As `!` isn't stable yet, we'll use `std::convert::Infallible` as a canonical uninhabited type. (You may have seen it before in `TryFrom`, with `u64: TryFrom` since that conversion cannot fail.) +One thing [The Book mentions](https://doc.rust-lang.org/stable/book/ch09-02-recoverable-errors-with-result.html#a-shortcut-for-propagating-errors-the--operator), +if you recall, is that error values in `?` have `From::from` called on them, to convert from one error type to another. -First we need to change the `Holder` type in our `Bubble` implementation, and change the body of `branch` accordingly: -```rust -use std::convert::Infallible; -impl Bubble for MyResult { - ... no changes here ... - type Holder = MyResult; - fn branch(self) -> ControlFlow { - match self { - MyResult::Terrific(v) => ControlFlow::Continue(v), - MyResult::Unfortunate(e) => ControlFlow::Break(MyResult::Unfortunate(e)), - } - } - ... no changes here ... -} -``` +The previous section actually lied to you slightly: there are *two* traits involved, not just one. The `from_residual` method is on `FromTryResidual`, which is generic so that the implementation on `Result` can add that extra conversion. Specifically, the trait looks like this: -As well as update our `Try` implementation for the new holder type: ```rust -impl Try for MyResult { - fn from_holder(h: Self::Holder) -> Self { - match h { - MyResult::Unfortunate(e) => MyResult::Unfortunate(e), - MyResult::Terrific(v) => match v {}, - } - } +trait FromTryResidual::Residual> { + fn from_residual(r: Residual) -> Self; } ``` -We're not quite done, though; the compiler will let us know that we have more work to do: -```rust -error[E0277]: the trait bound `MyResult: BreakHolder` is not satisfied - --> C:\src\rust\src\test\ui\try-operator-custom-v2.rs:17:5 - | -LL | type Holder = MyResult; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `BreakHolder` is not implemented for `MyResult` - | - ::: C:\src\rust\library\core\src\ops\try.rs:102:18 - | -LL | type Holder: BreakHolder; - | --------------------------- required by this bound in `std::ops::Bubble::Holder` -``` +And while we're showing code, here's the exact definition of the `Try` trait: -But that's a simple one: ```rust -impl BreakHolder for MyResult { - type Output = MyResult; +trait Try: FromTryResidual { + type Output; + type Residual; + fn branch(self) -> ControlFlow; + fn from_output(o: Self::Output) -> Self; } ``` -You can think of this trait as bringing back together the `T` and the `U` that `Bubble` had split into different associated types. +The fact that it's a super-trait like that is why I don't feel bad about the slight lie: Every `T: Try` *always* has a `from_residual` function from `T::Residual` to `T`. It's just that some types might offer more. -With that we can still use `?` in both directions as in `foo` previously. And it means that `Iterator::try_find` will give us back a `MyResult` if we return it in the predicate: +Here's how `Result` implements it to do error-conversions: ```rust -let x = [1, 2].iter().try_find(|&&x| { - if x < 0 { - MyResult::Unfortunate("uhoh") - } else { - MyResult::Terrific(x % 2 == 0) +impl> FromTryResidual> for Result { + fn from_residual(x: Result) -> Self { + match x { + Err(e) => Err(From::from(e)), + } } -}); -assert!(matches!(x, MyResult::Terrific(Some(2)))); -``` - -As expected, the mixing in `bar` no longer compiles: -``` -help: the trait `Try>` is not implemented for `MyResult<(), f32>` -``` - -#### Enabling `Result`-like error conversion - -`Result` allows mismatched error types so long as it can convert the source one into the type on the function. But if we try that with our current type, it won't work: -```rust -fn qux() -> MyResult<(), i64> { - let _: () = MyResult::Unfortunate(3_u8)?; - MyResult::Terrific(()) } ``` -That help message in the error from the previous section gives us a clue, however. Thus far we've been using the default generic parameter on `Try` which only allows an exact match to our declared `Bubble::Holder` type. But we can be more general if we want. Let's use `From`, like `Result` does: +But `Option` doesn't need to do anything exciting, so just has a simple implementation, taking advantage of the default parameter: + ```rust -impl> Try> for MyResult { - fn from_holder(h: MyResult) -> Self { - match h { - MyResult::Unfortunate(e) => MyResult::Unfortunate(From::from(e)), - MyResult::Terrific(v) => match v {}, +impl FromTryResidual for Option { + fn from_residual(x: Self::Residual) -> Self { + match x { + None => None, } } } ``` -With that the `qux` example starts compiling successfully. +In your own types, it's up to you to decide how much freedom is appropriate. You can even enable interconversion by defining implementations from the residual types of other families if you'd like. But just supporting your one residual type is ok too. -(This can also be used to allow interconversion with holder types from other type constructors, but that's left as an exercise for the reader.) +> 🏗️ Note for those familiar with the previous RFC 🏗️ +> +> This is another notable difference: The `From::from` is up to the trait implementation, not part of the desugaring. -### Using `?` with a non-generic type -The type in the previous discussion was very flexible, so let's take the opportunity to look at the other end of the spectrum at a type with no generic parameters. +## Implementing `Try` for a non-generic type -We'll make a `ResultCode` type that works like is common in C: it's an integer internally, with success represented by zero and any non-zero value being a different error. +The examples in the standard library are all generic, so serve as good examples of that, but non-generic implementations are also possible. -The type itself can just a normal newtype, and let's give it a constant for the success value: +Suppose we're working on migrating some C code to Rust, and it's still using the common "zero is success; non-zero is an error" pattern. Maybe we're using a simple type like this to stay ABI-compatible: ```rust -#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +#[repr(transparent)] pub struct ResultCode(pub i32); impl ResultCode { - pub const SUCCESS: Self = ResultCode(0); + const SUCCESS: Self = ResultCode(0); } ``` -Users are expected to produce, operate on, and consume this type, so it gets the a usual list of traits. It doesn't need to maintain any invariants in it either, so the field might as well just be public -- no need for accessors. -We decided earlier that zero is the only success value, so there's nothing interesting to provide out of `?` if the result code happens to be the success one. So we might as well use `()` as our `Continue` type. +We can implement `Try` for that type to simplify the code without changing the error model. -Now what should the holder look like? With no values of generic types, the holder won't need to be generic either. It needs to represent the non-zero values, so let's make it a newtype around `NonZeroI32`: +First, we'll need a residual type. We can make this a simple newtype, and conveniently there's a type with a niche for exactly the value that this can't hold. This is only used inside the desugaring, so we can leave it opaque -- nobody but us will need to create or inspect it. ```rust use std::num::NonZeroI32; -pub struct ResultCodeHolder(NonZeroI32); +pub struct ResultCodeResidual(NonZeroI32); ``` -This type is really only there to be used transiently inside desugarings. There's no expectation that a user will ever need to touch one -- they'll be working with `ResultCode`s. So we don't need to offer any way to produce or consume these directly; it can just be opaque. -*(Aside: it doesn't* have *to be completely opaque, however. In a real library it might want to be at least `Debug`+`Copy` for politeness, for example. Or it could be developed out into a "full" type with a `Display` which shows the description from C's `strerror`. But for the purpose of this RFC, there's no need for all that.)* - -With those types decided for the two sides of the control flow, we can go ahead and implement `Bubble`: +With that, it's straight-forward to implement the traits. `NonZeroI32`'s constructor even does exactly the check we need in `Try::branch`: ```rust -impl Bubble for ResultCode { - type Continue = (); - type Holder = ResultCodeHolder; - fn branch(self) -> ControlFlow { +impl Try for ResultCode { + type Output = (); + type Residual = ResultCodeResidual; + fn branch(self) -> ControlFlow { match NonZeroI32::new(self.0) { - Some(e) => ControlFlow::Break(ResultCodeHolder(e)), + Some(r) => ControlFlow::Break(ResultCodeResidual(r)), None => ControlFlow::Continue(()), } } - fn continue_with((): ()) -> Self { - Self::SUCCESS + fn from_output((): ()) -> Self { + ResultCode::SUCCESS } } -``` -Because `ResultCode` isn't generic, there's no way for `BreakHolder` to give out different types for different "keep going" payloads. Thus it's only defined for a `()` continue type: -```rust -impl BreakHolder<()> for ResultCodeHolder { - type Output = ResultCode; +impl FromTryResidual for ResultCode { + fn from_residual(r: ResultCodeResidual) -> Self { + ResultCode(r.0.into()) + } } ``` -This means that we won't be able to use `ResultCode` with `Iterator::try_find` -- since there's no way for it to hold a `bool` or an `Option` -- but that's fine. Interestingly, it's still usable with `Iterator::try_for_each`, since that only requires `Continue = ()`. -The implementation for `Try` is also simple: +Aside: As a nice bonus, the use of a `NonZero` type in the residual means that `::branch` [compiles down to a nop](https://rust.godbolt.org/z/GxeYax) on the current nightly. Thanks, enum layout optimizations! + +Now, this is all great for keeping the interface that the other unmigrated C code expects, and can even work in `no_std` if we want. But it might also be nice to give other *Rust* code that uses it the option to convert things into a `Result` with a more detailed error. + +For expository purposes, we'll use this error type: ```rust -impl Try for ResultCode { - fn from_holder(h: ResultCodeHolder) -> Self { - Self(h.0.into()) - } -} +#[derive(Debug, Clone)] +pub struct FancyError(String); ``` -And with that we have enough to try out our new type: +(A real one would probably be more complicated and have a better name, but this will work for what we need here -- it's bigger and needs non-core things to work.) + +We can allow `?` on a `ResultCode` in a method returning `Result` with an implementation like this: ```rust -fn external_one() -> ResultCode; -fn external_two() -> ResultCode; -fn demo() -> ResultCode { - external_one()?; - external_two()?; - ResultCode::SUCCESS +impl> FromTryResidual for Result { + fn from_residual(r: ResultCodeResidual) -> Self { + Err(FancyError(format!("Something fancy about {} at {:?}", r.0, std::time::SystemTime::now())).into()) + } } ``` -*(Aside: As a nice bonus, the use of a `NonZero` type in the holder means that `::branch` [compiles down to a nop](https://rust.godbolt.org/z/GxeYax) on the current nightly. Thanks, enum layout optimizations!)* +*The split between different error strategies in this section is inspired by [`windows-rs`](https://github.com/microsoft/windows-rs), which has both [`ErrorCode`](https://microsoft.github.io/windows-docs-rs/doc/bindings/windows/struct.ErrorCode.html) -- a simple newtype over `u32` -- and [`Error`](https://microsoft.github.io/windows-docs-rs/doc/bindings/windows/struct.Error.html) -- a richer type that can capture a stack trace, has an `Error` trait implementation, and can carry additional debugging information -- where the former can be converted into the latter.* -## Using these traits in generic code -### When the generic parameter has the `Continue` type you need +## Using these traits in generic code -`Iterator::try_fold` has been stable to call (but not to implement) for a while now. To illustrate the flow through the traits in this RFC, lets implement our own version without using any of the sugar. +`Iterator::try_fold` has been stable to call (but not to implement) for a while now. To illustrate the flow through the traits in this RFC, lets implement our own version. As a reminder, an infallible version of a fold looks something like this: ```rust @@ -369,9 +259,9 @@ fn simple_fold( So instead of `f` returning just an `A`, we'll need it to return some other type that produces an `A` in the "don't short circuit" path. Conveniently, that's also the type we need to return from the function. -Let's add a new generic parameter `R` for that type, and bound it to the continue type that we want: +Let's add a new generic parameter `R` for that type, and bound it to the output type that we want: ```rust -fn simple_try_fold_1>( +fn simple_try_fold_1>( iter: impl Iterator, mut accum: A, mut f: impl FnMut(A, T) -> R, @@ -380,9 +270,9 @@ fn simple_try_fold_1>( } ``` -Conveniently, `Bubble` is also the trait we need to get the updated accumulator from `f`'s return value and return the result if we manage to get through the entire iterator: +`Try` is also the trait we need to get the updated accumulator from `f`'s return value and return the result if we manage to get through the entire iterator: ```rust -fn simple_try_fold_2>( +fn simple_try_fold_2>( iter: impl Iterator, mut accum: A, mut f: impl FnMut(A, T) -> R, @@ -394,13 +284,13 @@ fn simple_try_fold_2>( ControlFlow::Break(_) => todo!(), } } - R::continue_with(accum) + R::from_output(accum) } ``` -Just `Bubble` isn't enough, though. We also need `Try` here in order to convert the holder inside the `Break` variant back into `R`. But because `Bubble` is a supertrait of `Try`, we can just change the bound rather than adding a second one: +We'll also need `FromTryResidual::from_residual` to turn the residual back into the original type. But because it's a supertrait of `Try`, we don't need to mention it in the bounds. All types which implement `Try` can always be recreated from their corresponding residual, so we'll just call it: ```rust -pub fn simple_try_fold>( +pub fn simple_try_fold_3>( iter: impl Iterator, mut accum: A, mut f: impl FnMut(A, T) -> R, @@ -409,29 +299,27 @@ pub fn simple_try_fold>( let cf = f(accum, x).branch(); match cf { ControlFlow::Continue(a) => accum = a, - ControlFlow::Break(h) => return R::from_holder(h), + ControlFlow::Break(r) => return R::from_residual(r), } } - R::continue_with(accum) + R::from_output(accum) } ``` -### When you need to switch to a different `Continue` type - -You might have noticed that even the final `simple_try_fold` didn't need to mention `BreakHolder`. That's because it's only needed if you want to return a type from the same family, but with a different continue type. - -`Option::zip` stabilized a few releases ago, so here's a demonstration of a more-generalized version. To restrict the two inputs to the same family, it requires that the holder types match. Then it uses `BreakHolder` to get an output type from that same family. - +But this "call `branch`, then `match` on it, and `return` if it was a `Break`" is exactly what happens inside the `?` operator. So rather than do all this manually, we can just use `?` instead: ```rust -fn zip_demo>( - a: impl Bubble, - b: impl Bubble, -) -> >::Output { - Bubble::continue_with((a?, b?)) +fn simple_try_fold>( + iter: impl Iterator, + mut accum: A, + mut f: impl FnMut(A, T) -> R, +) -> R { + for x in iter { + accum = f(accum, x)?; + } + R::from_output(accum) } ``` -Here `Try` doesn't need to be mentioned explicitly, as the `BreakHolder::Output` type is always bound by `Try`. +For example, this (admittedly contrived) loop +```rust +let mut sum = 0; +for x in iter { + if x % 2 == 0 { continue } + sum += x; + if sum > 100 { break } + continue +} +``` +can be written as +```rust +let mut sum = 0; +iter.try_for_each(|x| { + if x % 2 == 0 { return ControlFlow::Continue(()) } + sum += x; + if sum > 100 { return ControlFlow::Break(()) } + ControlFlow::Continue(()) +}); +``` +(Of course, one wouldn't normally use the `continue` keyword at the end of a `for` loop like that, but I've included it here to emphasize that even the `ControlFlow::Continue(())` as the final expression of the block it ends up working like the keyword would.) + +## Why `ControlFlow` has `C = ()` + +The type that eventually became `ControlFlow` was originally added way back in 2017 as [the internal-only type `LoopState`](https://github.com/rust-lang/rust/commit/b32267f2c1344d37c4aa30eccd5a9ab77642b3e6#diff-6f95fa6b66f447d11bb7507f832027237ee240310c159c74495a2363c82e76d7R357-R376) used to make some default implementations in `Iterator` easier to read. It had no type parameter defaults. + +[Issue #75744](https://github.com/rust-lang/rust/issues/75744) in 2020 started the process of exposing it, coming out of the [observation](https://github.com/rust-itertools/itertools/issues/469) that `Iterator::try_fold` isn't a great replacement for the deprecated-at-the-time `Itertools::fold_while` since using `Err` for a conceptual success makes code hard to read. + +The compiler actually had [its own version of the type](https://github.com/rust-lang/rust/blob/515c9fa505e18a65d7f61bc3e9eb833b79a68618/src/librustc_data_structures/graph/iterate/mod.rs#L91-L94) in `librustc_data_structures` at the time: +```rust +pub enum ControlFlow { + Break(T), + Continue, +} +``` + +The compiler was moved over to the newly-exposed type, and that inspired the creation of [MCP#374](https://github.com/rust-lang/compiler-team/issues/374), TypeVisitor: use ops::ControlFlow instead of bool. Experience from that lead to flipping the type arguments in [PR#76614](https://github.com/rust-lang/rust/pull/76614) -- which also helped the original use cases in `Iterator`, where things like default implementation of `find` also want `C = ()`. And these were so successful that it lead to [MCP#383](https://github.com/rust-lang/compiler-team/issues/383), TypeVisitor: do not hard-code a `ControlFlow<()>` result, having the visitors use `ControlFlow`. + +As an additional anecdote that `C = ()` is particularly common, [Hytak mentioned the following](https://discord.com/channels/530598289813536771/530603542138847242/807920021728264193) on Discord in response to seeing a draft of this RFC: + +> i didn't read your proposal in depth, but this reminds me of a recursive search function i experimented with a few days ago. It used a Result type as output, where Err(value) meant that it found the value and Ok(()) meant that it didn't find the value. That way i could use the `?` to exit early + +So when thinking about `ControlFlow`, it's often best to think of it not like `Result`, but like an `Option` which short-circuits the other variant. While it *can* flow a `Continue` value, that seems to be a fairly uncommon use in practice. + ## Was this considered last time? Interestingly, a [previous version](https://github.com/rust-lang/rfcs/blob/f89568b1fe5db4d01c4668e0d334d4a5abb023d8/text/0000-try-trait.md#using-an-associated-type-for-the-success-value) of RFC #1859 _did_ actually mention a two-trait solution, splitting the "associated type for ok" and "generic type for error" like is done here. It's no longer mentioned in the version that was merged. To speculate, it may have been unpopular due to a thought that an extra traits just for the associated type wasn't worth it. @@ -806,19 +704,31 @@ Current desires for the solution, however, have more requirements than were incl Also, ok-wrapping was decided [in #70941](https://github.com/rust-lang/rust/issues/70941), which needs such a constructor, making this ["much more appealing"](https://github.com/rust-lang/rust/issues/42327#issuecomment-379882998). -## Trait naming +## Why not make the output a generic type? + +It's helpful that type information can flow both ways through `?`. + +- In the forward direction, not needing a contextual type means that `println!("{}", x?)` works instead of needing a type annotation. (It's also just less confusing to have `?` on the same type always produce the same type.) +- In the reverse direction, it allows things like `let x: i32 = s.parse()?;` to infer the requested type from that annotation, rather than requiring it be specified again. + +Similar scenarios exist for `try`, though of course they're not yet stable: + +- `let y: anyhow::Result<_> = try { x };` doesn't need to repeat the type of `x`. +- `let x: i16 = { 4 };` works for infallible code, so for consistency it's good for `let x: anyhow::Result = try { 4 };` to also work (rather than default the literal to `i32` and fail). + +## Trait and associated type naming Bikeshed away! -## Why a "holder" type is better than an "error" type +## Why a "residual" type is better than an "error" type -Most importantly, for any type generic in its "continue type" it's easy to produce the holder type using an uninhabited type. That works for `Option` -- no `NoneError` residual type needed -- as well as for the `StrandFail` type from the experience report. And thanks to enum layout optimizations, there's no space overhead to doing this: `Option` is a ZST, and `Result` is no larger than `E` itself. So most of the time one will not need to define anything additional. +Most importantly, for any type generic in its "output type" it's easy to produce a residual type using an uninhabited type. That works for `Option` -- no `NoneError` residual type needed -- as well as for the `StrandFail` type from the experience report. And thanks to enum layout optimizations, there's no space overhead to doing this: `Option` is a ZST, and `Result` is no larger than `E` itself. So most of the time one will not need to define anything additional. -In those cases where a separate type *is* needed, it's still easier to make a holder type because they're transient and thus can be opaque: there's no point at which a user is expected to *do* anything with a holder type other than convert it back into a known `Try` type. This is different from the previous design, where less-restrictive interconversion meant that anything could be exposed via a `Result`. That has lead to requests, [such as for `NoneError` to implement `Error`](https://github.com/rust-lang/rust/issues/46871#issuecomment-618186642), that are perfectly understandable given that the instances are exposed in `Result`s. As holder types aren't ever exposed like that, it would be fine for them to implement nothing but `BreakHolder` (and probably `Debug`), making them cheap to define and maintain. +In those cases where a separate type *is* needed, it's still easier to make a residual type because they're transient and thus can be opaque: there's no point at which a user is expected to *do* anything with a residual type other than convert it back into a known `Try` type. This is different from the previous design, where less-restrictive interconversion meant that anything could be exposed via a `Result`. That has lead to requests, [such as for `NoneError` to implement `Error`](https://github.com/rust-lang/rust/issues/46871#issuecomment-618186642), that are perfectly understandable given that the instances are exposed in `Result`s. As residual types aren't ever exposed like that, it would be fine for them to implement nothing but `FromTryResidual` (and probably `Debug`), making them cheap to define and maintain. ## Use of `!` -This RFC uses `!` to be concise. It would work fine with `convert::Infallible` instead if `!` has not yet stabilized, though a few more match arms would be needed in the implementations. (For example, `Option::from_holder` would need `Some(c) => match c {}`.) +This RFC uses `!` to be concise. It would work fine with `convert::Infallible` instead if `!` has not yet stabilized, though a few more match arms would be needed in the implementations. (For example, `Option::from_residual` would need `Some(c) => match c {}`.) ## Moving away from the `Option`→`Result` interconversion @@ -828,43 +738,24 @@ For example, we could have a different, never-stable `Try`-like trait used in ol It's unclear that that's worth the effort, however, so this RFC is currently written to continue to support it going forward. Notably, removing it isn't enough to solve the annotation requirements, so the opportunity cost feels low. -## Bounds on `Bubble::Holder` - -The bound for this associated type could be tightened as follows: -```rust -type Holder: BreakHolder; -``` - -That forces every type to be in bijection with its holder; however it's not clear that such a restriction is valuable. - -The implementation for `Poll>`, for example, works well with reusing `Result`'s holder type. The type *wants* to support the interconversion anyway, so forcing it to define a newtype for the holder is extra busywork. It might not even be possible for types outside `core` due to coherence. - -The advantage of the bijection, though, is that methods like `Iterator::try_find` would always return the "same" thing conceptually. But in generic code that's mostly irrelevant, and even with known types it's not a big deal. The holder type will be the same, so `?` would still work to go back to the original type if needed. And of course the type can always define a distinct holder if they're worried about it. - -## `BreakHolder` vs using GATs - -Generic Associated Types (GATs) may not be stable at the time of writing this RFC, but it's natural to ask if we'd like to use them if available, and thus should wait for them. - -Types like `Result` and `Option` support any (sized) type as their continue type, so GATs would be reasonable for them. However, with other possible types that might not be the case. For example, imagine supporting `?` on `process::ExitStatus`. Its holder type would likely only be `BreakHolder<()>`, since it cannot hold a custom payload and for it "success is defined as a zero exit status" (per `ExitStatus::success`). So requiring that these types support an arbitrary generic continue type may be overly restrictive, and thus the trait approach -- allowing bounding to just the types needed -- seems reasonable. - -(Note that RFC #1598 only [included lifetime-GATs, not type-GATs](https://rust-lang.github.io/rfcs/1598-generic_associated_types.html#associated-type-constructors-of-type-arguments). So it's likely that it would also be a very long wait for type-GATs.) +## Why `FromTryResidual` is the supertrait -## Default `H` on `Try` +It's nicer for `try_fold` implementations to just mention the simpler `Try` name. It being the subtrait means that code needing only the basic scenario can just bound on `Try` and know that both `from_output` and `from_residual` are available. -The default here is provided to make the homogeneous case simple. Along with its supertrait, that greatly simplifies the bound on `Iterator::try_fold`, for example. Just requiring `R: Try` is enough to break apart and rebuild a value of that type (as was the case with the previous trait). +## Default `Residual` on `FromTryResidual` -It's also convenient when implementing the trait in cases that don't need conversion, as seen with `Option`. +The default here is provided to make the basic case simple. It means that when implementing the trait, the simple case (like in `Option`) doesn't need to think about it -- similar to how you can `impl Add for Foo` for the homogeneous case even though that trait also has a generic parameter. -## `Try::from_holder` vs `Holder::into_try` +## `FromTryResidual::from_residual` vs `Residual::into_try` -Either of these directions could be made to work. Indeed, an early experiment while drafting this had a method on `BreakHolder` that created the `Bubble` type (not just the associated type). However that was removed as unnecessary once `Try::from_holder` was added. +Either of these directions could be made to work. Indeed, an early experiment while drafting this had a method on a required trait for the residual that created the type implementing `Try` (not just the associated type). However that method was removed as unnecessary once `from_residual` was added, and then the whole trait was moved to future work in order to descope the RFC, as it proved unnecessary for the essential `?`/`try_fold` functionality. -A major advantage of the `Try::from_holder` direction is that it's more flexible with coherence when it comes to allowing other things to be converted into a new type being defined. That does come at the cost of higher restriction on allowing the new type to be converted into other things, but reusing a holder can also be used for that scenario. +A major advantage of the `FromTryResidual::from_residual` direction is that it's more flexible with coherence when it comes to allowing other things to be converted into a new type being defined. That does come at the cost of higher restriction on allowing the new type to be converted into other things, but reusing a residual can also be used for that scenario. -Converting a known holder into a generic `Bubble` type seems impossible (unless it's uninhabited), but consuming arbitrary holders could work -- imagine something like +Converting a known residual into a generic `Try` type seems impossible (unless it's uninhabited), but consuming arbitrary residuals could work -- imagine something like ```rust -impl> Try for LogAndIgnoreErrors { - fn from_holder(h: H) -> Self { +impl FromTryResidual for LogAndIgnoreErrors { + fn from_residual(h: H) -> Self { dbg!(h); Self } @@ -872,7 +763,7 @@ impl> Try for LogAndIgnoreErrors { ``` (Not that that's necessarily a good idea -- it's plausibly *too* generic. This RFC definitely isn't proposing it for the standard library.) -And, ignoring the coherence implications, a major difference between the two sides is that the target type is typically typed out visibly (in a return type) whereas the source type (going into the `?`) is often the result of some called function. So it's preferrable for any behaviour extensions to be on the type that can more easily be seen in the code. +And, ignoring the coherence implications, a major difference between the two sides is that the target type is typically typed out visibly (in a return type) whereas the source type (going into the `?`) is often the result of some called function. So it's preferable for any behaviour extensions to be on the type that can more easily be seen in the code. + # Future possibilities [future-possibilities]: #future-possibilities From 589d2e82d1d0f8d0fa27cc0aff5c007168d4dd3b Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Fri, 5 Mar 2021 20:14:47 -0800 Subject: [PATCH 10/12] A few more updates from feedback --- text/0000-try-trait-v2.md | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/text/0000-try-trait-v2.md b/text/0000-try-trait-v2.md index 7e7ef8b1940..88910cc17a7 100644 --- a/text/0000-try-trait-v2.md +++ b/text/0000-try-trait-v2.md @@ -74,6 +74,7 @@ impl TreeNode { ``` +Now, you *could* write the same thing with `Result<(), B>` instead. But that would require that the passed-in closure use `Err(value)` to early-exit the traversal, which can cause mental dissonance when that exit is because it successfully found the value for which it was looking. Using `ControlFlow::Break(value)` instead avoids that prejudice, the same way that `break val` in a `loop` doesn't inherently mean success nor failure. ## The `Try` trait @@ -105,7 +106,7 @@ If you've used `?`-on-`Result` before, that output type is likely unsurprising. The residual types, however, are somewhat more interesting. Code using `?` doesn't see them directly -- their usage is hidden inside the desugaring -- so there are more possibilities available. So why are we using these ones specifically? -Most importantly, this gives each family of types (`Result`s, `Option`s, `ControlFlow`s) their own *distinct* residual type. That avoids unrestricted *interconversion* between the different types, the ability to arbitrarily mix them in the same method. For example, it was mentioned earlier that just because a `ControlFlow::Break` is also an early exit, that doesn't mean that it should be allowed to consider it a `Result::Err` -- it might be a success, conceptually. So by giving `ControlFlow` and `Result<_, X>` different residual types, it becomes a compilation error to use the `?` operator on a `ControlFlow` in a method which returns a `Result`. (There are also ways to allow interconversion is specific situations where it's desirable.) +Most importantly, this gives each family of types (`Result`s, `Option`s, `ControlFlow`s) their own *distinct* residual type. That avoids unrestricted *interconversion* between the different types, the ability to arbitrarily mix them in the same method. For example, like in the traversal example earlier, just because a `ControlFlow::Break` is also an early exit, that doesn't mean that it should be allowed to consider it a `Result::Err` -- it might be a success, conceptually. So by giving `ControlFlow` and `Result<_, X>` different residual types, it becomes a compilation error to use the `?` operator on a `ControlFlow` in a method which returns a `Result`, and vice versa. (There are also ways to allow interconversion where it's desirable between a particular pair of types.) > 🏗️ Note for those familiar with the previous RFC 🏗️ > @@ -402,7 +403,7 @@ match Try::branch(x) { } ``` -It's just left conversion (such as `From::from`) up to the implementation instead of forcing it in the desugar. +The critical difference is that conversion (such as `From::from`) is left up to the implementation instead of forcing it in the desugar. ## Standard implementations @@ -737,11 +738,27 @@ But even for the error path, forcing `From` causes problems, notably because of As a bonus, moving conversion (if any) into the `FromResidual` implementation may actually speed up the compiler -- the simpler desugar means generating less HIR, and thus less work for everything thereafter (up to LLVM optimizations, at least). The `serde` crate has [their own macro](https://github.com/serde-rs/serde/blob/b0c99ed761d638f2ca2f0437522e6c35ad254d93/serde_derive/src/try.rs#L3-L6) for error propagation which omits `From`-conversion as they see a "significant improvement" from doing so. +## Why not merge `Try` and `FromResidual`? + +This RFC treats them as conceptually the same trait -- there are no types proposed here to implement `FromResidual<_>` which don't also implement `Try` -- so one might wonder why they're not merged into one `Try`. After all, that would seem to remove the duplication between the associated type and the generic type, as something like +```rust +trait Try { + type Output; + fn branch(self) -> ControlFlow; + fn from_residual(r: Residual) -> Self; + fn from_output(x: Self::Output) -> Self; +} +``` + +This, however, is technically too much freedom. Looking at the error propagation case, it would end up calling both `Try::branch` and `Try::from_residual`. With the implementation for `Result`, where those inference variables go through `From`, there's no way to pick what they should be, similar to how `.into().into()` doesn't compile. And even outside the desugaring, this would make `Try::from_output(x)` no longer work, since the compiler would (correctly) insist that the desired residual type be specified. + +And even for a human, it's not clear that this freedom is helpful. While any trait can be implemented weirdly, one good part of RFC #1859 that this one hopes to retain is that one doesn't need to know contextual information to understand what comes out of `?`. Whereas any design that puts `branch` on a generic trait would mean it'd be possible for `?` to return different things depending on that generic type parameter -- unless the associated type were split out into a separate trait, but that just reopens the "why are they different traits" conversation again, without solving the other issues. + ## Naming the `?`-related traits and associated types This RFC introduces the *residual* concept as it was helpful to have a name to talk about in the guide section. (A previous version proved unclear, perhaps in part due to it being difficult to discuss something without naming it.) But the `fn branch(self) -> ControlFlow` API is not necessarily obvious. -A different might be clearer for people. For example, there's some elegance to matching the variant names by using `fn branch(self) -> ControlFlow`. Or perhaps there are more descriptive names, like `KeepGoing`/`ShortCircuit`. +A different scheme might be clearer for people. For example, there's some elegance to matching the variant names by using `fn branch(self) -> ControlFlow`. Or perhaps there are more descriptive names, like `KeepGoing`/`ShortCircuit`. As a sketch, one of those alternatives might look something like this: ```rust @@ -758,7 +775,7 @@ trait FromBreak::Break> { However the "boring" `Output` name does have the advantage that one doesn't need to remember a special name, as it's the same as the other operator traits. (For precedent, it's `Add::Output` and `Div::Output` even if one could argue that `Add::Sum` or `Div::Quotient` would be more "correct", in a sense.) -> ℹ After discussion with T-libs, this is left as an unresolved question for the RFC, to be resolved in nightly. +> ℹ Per feedback from T-libs, this is left as an unresolved question for the RFC, to be resolved in nightly. ## Splitting up `Try` more @@ -826,7 +843,7 @@ trait Try = Branch + FromOutput + FromResidual<::Residual>; There are probably also useful intermediary designs here. Perhaps the `IgnoreAllErrors` example above suggests that *introduction* on its own is reasonable, but *elimination* should require that both be supported. That's also the direction that would make sense for `?` in infallible functions: it's absolutely undesirable for `()?????` to compile, but it might be fine for all return types to support something like `T: FromResidual` eventually. -> ℹ After discussion with T-libs, this is left as an unresolved question for the RFC, to be resolved in nightly. +> ℹ Per feedback from T-libs, this is left as an unresolved question for the RFC, to be resolved in nightly. ## Why a "residual" type is better than an "error" type @@ -930,6 +947,9 @@ Please also take into consideration that rust sometimes intentionally diverges f # Unresolved questions [unresolved-questions]: #unresolved-questions +Waiting on crater: +- [ ] Find out more about how widespread the accidental interconversions are, and change the RFC to propose whether to bless/lint/remove them, and how. + Questions from T-libs to be resolved in nightly: - [ ] What vocabulary should `Try` use in the associated types/traits? Output+residual, continue+break, or something else entirely? - [ ] Is it ok for the two traits to be tied together closely, as outlined here, or should they be split up further to allow types that can be only-created or only-destructured? From 71aef4088005b485d0723b66ce41c2d7c88b58b4 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Tue, 30 Mar 2021 09:50:51 -0700 Subject: [PATCH 11/12] Move the accidental interconversion support to an alternative, given crater results. --- text/0000-try-trait-v2.md | 132 +++++++++++++++++++++++++------------- 1 file changed, 87 insertions(+), 45 deletions(-) diff --git a/text/0000-try-trait-v2.md b/text/0000-try-trait-v2.md index 88910cc17a7..ffe5f75fc6e 100644 --- a/text/0000-try-trait-v2.md +++ b/text/0000-try-trait-v2.md @@ -10,9 +10,10 @@ Replace [RFC #1859, `try_trait`](https://rust-lang.github.io/rfcs/1859-try-trait with a new design for the currently-unstable [`Try` trait](https://doc.rust-lang.org/nightly/std/ops/trait.Try.html) and corresponding desugaring for the `?` operator. -The new design supports all the currently-stable conversions (including the accidental ones), -while addressing the discovered shortcomings of the currently-implemented solution, -as well as enabling new scenarios. +The new design includes support for all *intentional* interconversions. +It proposes removing the *accidental* interconversions, as a crater run +demonstrated that would be feasible, however includes an alternative system +that can support them as a low-support-cost edition mechanism if needed. *This is [forward-looking](#future-possibilities) to be compatible with other features, like [`try {}`](https://doc.rust-lang.org/nightly/unstable-book/language-features/try-blocks.html) blocks @@ -27,7 +28,7 @@ The motivations from the previous RFC still apply (supporting more types, and re However, new information has come in since the previous RFC, making people wish for a different approach. - Using the "error" terminology is a poor fit for other potential implementations of the trait. -- The ecosystem has started to see error types which are `From`-convertible from *any* type implementing `Debug`, which makes the previous RFC's method for controlling interconversions ineffective. +- The previous RFC's mechanism for controlling interconversions proved ineffective, with inference meaning that people did it unintentionally. - It's no longer clear that `From` should be part of the `?` desugaring for _all_ types. It's both more flexible -- making inference difficult -- and more restrictive -- especially without specialization -- than is always desired. - An [experience report](https://github.com/rust-lang/rust/issues/42327#issuecomment-366840247) in the tracking issue mentioned that it's annoying to need to make a residual type in common cases. @@ -554,34 +555,6 @@ impl ops::FromResidual for ControlFlow { } ``` -## Making the accidental `Option` interconversion continue to work - -This is done with an extra implementation: -```rust -mod sadness { - use super::*; - - /// This is a remnant of the old `NoneError` which is never going to be stabilized. - /// It's here as a snapshot of an oversight that allowed this to work in the past, - /// so we're stuck supporting it even though we'd really rather not. - #[derive(Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)] - pub struct PleaseCallTheOkOrMethodToUseQuestionMarkOnOptionsInAMethodThatReturnsResult; - - impl ops::FromResidual> for Result - where - E: From, - { - fn from_residual(x: Option) -> Self { - match x { - None => Err(From::from( - PleaseCallTheOkOrMethodToUseQuestionMarkOnOptionsInAMethodThatReturnsResult, - )), - } - } - } -} -``` - ## Use in `Iterator` The provided implementation of `try_fold` is already just using `?` and `try{}`, so doesn't change. The only difference is the name of the associated type in the bound: @@ -855,14 +828,6 @@ In those cases where a separate type *is* needed, it's still easier to make a re This RFC uses `!` to be concise. It would work fine with `convert::Infallible` instead if `!` has not yet stabilized, though a few more match arms would be needed in the implementations. (For example, `Option::from_residual` would need `Some(c) => match c {}`.) -## Moving away from the `Option`→`Result` interconversion - -We could consider doing an edition switch to make this no longer allowed. - -For example, we could have a different, never-stable `Try`-like trait used in old editions for the `?` desugaring. It could then have a blanket impl, plus the extra interconversion one. - -It's unclear that that's worth the effort, however, so this RFC is currently written to continue to support it going forward. Notably, removing it isn't enough to solve the annotation requirements, so the opportunity cost feels low. - ## Why `FromResidual` is the supertrait It's nicer for `try_fold` implementations to just mention the simpler `Try` name. It being the subtrait means that code needing only the basic scenario can just bound on `Try` and know that both `from_output` and `from_residual` are available. @@ -890,6 +855,85 @@ impl FromResidual for LogAndIgnoreErrors { And, ignoring the coherence implications, a major difference between the two sides is that the target type is typically typed out visibly (in a return type) whereas the source type (going into the `?`) is often the result of some called function. So it's preferable for any behaviour extensions to be on the type that can more easily be seen in the code. +## Can we just remove the accidental interconversions? + +This depends on how we choose to read the rules around breaking changes. + +A [crater run on a prototype implementation](https://github.com/rust-lang/rust/pull/82322#issuecomment-792299734) found that some people are doing this. PRs have been sent to the places that broke, and generally it was agreed that removing the mixing improved the code: + +> Definitely a good change. + +> Thanks for spotting that, that was indeed a confusing mix + +However another instance is in an abandoned project where the repository has been archived, so will not be fixed. And of course if it happened 3 times, there might be more instances in the wild. + +The interesting pattern boils down to this: + +```rust +.map(|v| Ok(something_returning_option(v)?)) +``` + +That means it's using `?` on an `Option`, but the closure ends up returning `Result<_, NoneError>` without needing to name the type as trait resolution discovers that it's the only possibility. It seems reasonable that this could happen accidentally while refactoring. That does mean, however, that the breakage could also be considered "allowed" as an inference change, and hypothetically additional implementations could make it ambiguous in the future. (It's like the normal `AsRef` breakage, and fits the pattern of "there's a way it could be written that works before and after", though in this case the disambiguated form requires naming an unstable type.) + +This RFC thus proposes removing the accidental interconversions. + +### Compatibility with accidental interconversions (if needed) + +If something happens that turns out they need to be supported, the following approach can work. + +This would take a multi-step approach: +- Add a new never-stable `FromResidualLegacy` trait +- Have a blanket implementation so that users interact only with `FromResidual` +- Add implementations for the accidental interconversions +- Use `FromResidualLegacy` in the desugaring, [perhaps only for old editions](https://github.com/scottmcm/rust/commit/do-or-do-not-edition) + +This keeps them from being visible in the trait system on stable, as `FromResidual` (the only form that would ever stabilize, or even be mentionable) would not include them. + +```rust +mod sadness { + use super::*; + + /// This includes all of the [`ops::FromResidual`] conversions, but + /// also adds the two interconversions that work in 2015 & 2018. + /// It will never be stable. + pub trait FromResidualLegacy { + fn from_residual_legacy(r: R) -> Self; + } + + impl, R> FromResidualLegacy for T { + fn from_residual_legacy(r: R) -> Self { + >::from_residual(r) + } + } + + /// This is a remnant of the old `NoneError` which is never going to be stabilized. + /// It's here as a snapshot of an oversight that allowed this to work in the past, + /// so we're stuck supporting it even though we'd really rather not. + /// This will never be stabilized; use [`Option::ok_or`] to mix `Option` and `Result`. + #[derive(Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)] + pub struct LegacyNoneError; + + impl ops::FromResidual> for Result + where + E: From, + { + fn from_residual(x: Option) -> Self { + match x { + None => Err(From::from(LegacyNoneError)), + } + } + } + + + #[unstable(feature = "try_trait_v2", issue = "42327")] + impl FromResidualLegacy> for Option + { + fn from_residual_legacy(_: Result) -> Self { + None + } + } +} +```