try_trait_v2: A new design for the ? desugaring

[scottmcm]

Replace RFC #1859, try_trait, with a new design for the currently-unstable Try trait and desugaring for the ? operator.

🖼️ Rendered 🖼️
📖 RFC Book 📖
📝 Tracking Issue 📝

Update 2021-02-07: This was almost completely rewritten. If you're looking to read the discussion, you might want to jump to #3058 (comment) for the comments that are about the current version of the text.

I've also made a proof-of-concept implementation: https://github.com/scottmcm/rust/pull/2/files

Thanks to everyone who discussed this problem space in the original RFC thread, in the try_trait tracking issue, in zulip topics, in IRLO threads, on Discord, and elsewhere. There are far too many to mention individually.

This RFC has at least three major bikesheds. To focus on the motivations, problem space, and proposed semantics of this change initially, please hold off on feedback about item names until 2021-01-18. I reserve the right to hide bikeshedding posts made before then. Note also that this RFC intentionally makes no decisions about try blocks or yeet expressions; they're only mentioned in the explicitly non-normative "future possibilities" section. So please refrain from making any comments about their desirability.

[joshtriplett]

This looks great.

Nominating for the next design meeting.

[dureuill]

Wow! I'm unsure if I'm qualified to comment on a RFC, but for the record, I believe this covers everything discussed here and then some.

This looks great!

[Diggsey]

This seems like a good design, but I have a few concerns:

• The ? operator is already known as the Try operator, so in: let a: T = ...; a? the trait that T must implement should be the Try trait. Given that the RFC proposes using the Bubble trait for this, it should also propose to change the name of the operator (and consider the fallout that will cause).

• Holder means very little to me. Payload seems slightly better, but the design doesn't make it easy to choose good names. (just saw the note about bikeshedding, the prior point is not bikeshedding imo)

• The design as a whole seems confusing if you're not deeply familiar with the limitations of the Rust type system. For example, the Holder type will generally be defined with a ! parameter and exists mostly to workaround the lack of HKTs. It also serves two separate purposes:

  1. Carry the error value.
  2. Specify the "class" of preferred return types, ie. Option<!> is used as a proxy for Option<*>.

  To explain further, you could split the Holder type into two:

  trait Bubble {
      type Continue;
      type Break;
      type TypeClass: TypeClass<Self::Continue, Self::Break>;
      fn continue_with(c: Self::Continue) -> Self;
      fn branch(self) -> ControlFlow<Self::Break, Self::Continue>;
  }
  
  trait TypeClass<C, B> {
      type Output;
  }
  
  struct OptionClass;
  
  impl<T> TypeClass<T, ()> for OptionClass {
      type Output = Option<T>;
  }

  With this solution you don't have to rely on the ! type being optimised correctly.

  I'm not sure if this is better, but it's worth acknowledging that Holder is being overloaded here to achieve two things, and it could cause problems if those two ever result in conflicting requirements. For example, a type which is not generic over the "Continue" type cannot follow this pattern when defining its Holder.

• Don't RFCs normally include a "how do we document this" section?

• How confident are we that this is the simplest solution that achieves the stated goals?

[scottmcm]

Thanks for the reply! Splitting out the TypeClass marker like that is interesting.

Don't RFCs normally include a "how do we document this" section?

That section is no longer in the template: https://github.com/rust-lang/rfcs/blob/master/0000-template.md

I'm not sure if this is better, but it's worth acknowledging that Holder is being overloaded here to achieve two things, and it could cause problems if those two ever result in conflicting requirements. For example, a type which is not generic over the "Continue" type cannot follow this pattern when defining its Holder.

Yeah, I discuss holder types for types that aren't generic in Continue a bit in https://github.com/scottmcm/rfcs/blob/do-or-do-not/text/0000-try-trait-v2.md#why-a-holder-type-is-better-than-an-error-type

I think what pushed me to using the same type for both things is that a "residual" type (to use Niko's word) of some sort is needed for just about any type -- Result<T, E> could potentially just put an E in the Break, but just about everything else needs to pick something other than one of its generic parameters. Various conversations have talked about just using the Self type to avoid that, but I'm not a fan of that approach because it means handling a should-be-impossible variant in from_error/from_holder methods.

Suppose we wanted ? on Ordering (as it's a nice simple example, though we may well not choose to actually have it in std). Taking the definition that was proposed in a PR, Ordering::Equal should result in Continue(_), and Ordering::{Less, Greater} should result in Break(_). But what type should that payload be in the Break? I'm not a fan of having it be Ordering, as previously mentioned. The laziest one would be bool, since we only need two states. But then we'd end up with some form of Ordering: WhateverTheBikeshed<bool>, which seems unclear in what the semantics would be. I'd rather a newtype like struct OrderingHolder(bool); that doesn't expose its contents to the world or an additional enum like enum Unequal { Less = -1, Greater = 1 }. But once that exists, I don't know that also making a struct OrderingClass; as well is providing much value.

Even for Result, I don't know that I'd want a separate struct ResultClass;. I'd be awfully tempted to just do type TypeClass = Result<(), ()>; or something, even though that's not a ZST, because it's just a type-level marker so why not? (Or maybe Result<!, !>? Is it expected that one could have values of these types?)

[mbartlett21]

This would be useful to have something like NonCoercingResult or something

[PurpleMyst]

Maybe I'm not qualified to speak on this, but I find this RFC really confusing. Perhaps it's because I'm a non-native english speaker, but I fail to follow its whole reasoning about the Holder type in the Guide-level explanation. I'm just not sure I understand how it can be used in a way that's not just another form of Result's Holder.

[tmccombs]

I am I native english speaker, and I do have to agree with @castroed47 a little. I did find the description for Holder in the guide level explanation a little confusing, and had to read through it multiple times. I also didn't understand the necessity of the BreakHolder trait at all after reading the Guide Level explanation. And didn't really get the point until reading the implementation of try_find. At the very least I think the purpose of that trait, and how it would be used should be better explained in the guide.

[clarfonthey]

I'm also a native english speaker and I also had a really hard time trying (heh) to understand the way all of these traits were set up. In hindsight my explanation is super rambly but maybe it'll help clarify what I think are the biggest issues with this setup. Most of them aren't about the spirit of the implementation (which, after thinking about it, I am fine with), but rather the actual implementation itself and the naming.

Like, I don't actually have a big problem with the general idea of this, and especially I feel like the ControlFlow enum is a very good idea. However, the way the traits are set up almost feels a bit embarrassed about the implementation and they don't really have enough good conceptual cohesion to hold their own weight. As someone implementing the traits for their own types, I should have a strong grasp of what each of the traits is and what all the methods and associated types do, and right now, I really don't have that. It solves the problem of relying on Result at the cost of making the control flow harder to understand.

I know you didn't mention any bikeshedding and I don't think this should be considered as a suggestion of names, but I do want to point out how the flow is very confusing with the existing methods:

1. Bubble::branch is called when we enter the try flow to make a control flow decision.
2. Try::from_holder is called when we exit the flow due to a break.
3. Bubble::continue_with is called when we exit the flow due to a continue.

The names do not have any consistency at all, and don't represent at all what's happening during the flow. Although the implementation mentions control flow decisions, it doesn't at all mention what flow these decisions are controlling. The term "bubble" implies that they're part of a "bubble" flow but we've already decided to call the flow a try flow, so it seems to imply that we're talking about an entirely different flow even though we're not.

Now, let's look at the types:

1. Branching gives us a ControlFlow<B, C>. Seems okay so far.
2. Except we have a "holder" and a continue. Given the control flow struct, this should be called "break" and continue. It doesn't matter if the break is wrapped in some other type; it is a break.
3. Inside the flow, we use continues to continue the flow. That's fine.
4. Once we exit the flow due to a break, after going through multiple traits, we end up back at the original type that managed the control flow. That also seems fine.
5. Once we exit the flow after our final continue, we pass a continue to continue_with, which returns the type that managed the control flow. That also seems fine.
6. The entire flow, ultimately, would make sense as having a T -> T signature, right? Wrong. We have T -> <T::Holder as ops::BreakHolder<S>>::Output. That's because continues can vary throughout the entire flow, and we need some way of demonstrating this difference when we get to the end. To do this, we just generally assume that our "holder" is generic over multiple continues, and that each of these continues ultimately brings us back to a similar type with a different continue. (Note, the T -> T thing is obviously not the case when you consider the purpose of the original flow, but I'm choosing to just interpret it based upon the methods for now.)

Basically… the entire flow is designed to sidestep the fact that we don't have GATs. The whole purpose of the holder is to have a single type that represents what can happen in the control flow, and then get the associated type associated with the right continue to finish things off. After thinking about it, going away from GATs actually is a good idea (your ExitStatus example convinced me) and it is better to just have specific implementations on other traits narrow down what types of breaks and continues we can have, but really, we should at least arrange those other traits properly.

So… here's my general description of what we're trying to do with this implementation. I'm going to use Result as the example but it will apply in either case.

1. It has an "enter flow" method that returns ControlFlow<B, C>. Where B represents the "break" type and C represents the "continue" type.
2. It has an "exit with break" method that takes in B and returns Result<T, E>.
3. It has an "exit with continue" method that takes in C and returns Result<T, E>.

Now, there are a few caveats:

1. During the whole flow, T may change into S, and the flow should return S instead.
2. During the whole flow, E may change into F, but the flow should still return E.

Note that in both cases, we have a conversion, but the direction of the conversions is switched.

The way that this implementation solves the second problem is with a BreakHolder -- encapsulate the concept of Result<!, E> and allow us to convert Result<!, F> into Result<!, E>. However, we also use BreakHolder to encapsulate the first bit, which I think is a mistake: because Result<!, E> can be turned into both Result<T, E> or Result<S, E>, we can also add an associated type that will let us "convert" it back into a different continue.

Really, what we want is our "try" trait to have "exit with break" have a signature like B<F> -> Result<T, E> and "exit with continue" have a signature like C<S> -> Result<S, E>. I think that having BreakHolder be the one that makes the decision about continues and Try be the one that makes the decision about breaks to be extremely unintuitive. I'm not convinced we can't make it work more symmetrically.

[clarfonthey]

(Not sure if what I said counts as bikeshedding; I mention names as a way of discussing issues with structure but tried not to strictly discuss how the trait should specifically be named.)

[PurpleMyst]

I think @clarfonthey's comment really clarifies thing a lot, at least for me. If I'm understanding this correctly, Bubble turns our type into ops::ControlFlow, and, in the case of a Break, our Holder type holds information about just the Break. The BreakHolder trait has then the job to put type information about the Continue case back into our Holder type, and the Try trait for a given type allows it to represent the end of a control flow, no matter if it ends with a continue or with a break, while a Bubble type can only represent an ongoing control flow and must be converted into a Try type at the end. Am I correct, close, totally off..?

[QuineDot]

Constructive criticism after my second or third pass.

The section "Avoiding interconversion with a custom Holder" is very hard to follow. Especially if you come back to it and aren't continuing from the previous section. This is a shame, because for me at least, it was a key section for understanding how everything ties together. Some specific points:

• The interconversion in question is never defined. The only mention of interconversion before this section is that it was some sort of oops in the current implementation. After the first paragraph of this section, there's 2-3 pages of "how to make a custom Holder", and then the last sentence is "As expected, the mixing in bar no longer compiles". Ahh, that must have been the point.
  • bar was introduced in the last section, not this section; I had to hunt it down
  • Naming it interconverting_example instead of bar would help
• Early on is a sentence, "Thus for us it'll depend only on U, never on T". This is the first mention of T and U in this section.
  • In retrospect I see they are short for Terrific and Unfortunate, but they also happen to be the most common non-descriptive generic trait parameter names in the ecosystem. Longer names would help.
• There's a sentence near the end, "With that we can still use ? in both directions as in foo previously."
  • I'm still not sure what this means, really. What directions? You have two versions of foo in the prior sections... are we talking about both? If not, which one?

The point of continue_with could be made more directly. It shows up in the unstable try_find example and the future possibilities for try{}. From the try {} example (another unstable feature), I now recognize it was implicit in the try_fold example as well. And after writing that out, I now actually wonder... is it even needed for this RFC?

Relatedly, changing the Continue type is mentioned in passing during the Guide-level explanation, but isn't really explored. I guess this is the discussion that needs to happen around scope, as you mention in the unresolved questions. Probably what an alternative without BreakHolder would look like should be sketched out, along with a rationale of why BreakHolder should be included. There seems to be a lot of machinery here motivated by unstable features and future possibilities.

ControlFlow is unstable, though it recently had a MCP. I'll definitely go read up on it myself, but it feels like that should be stabilized or at least formalized first? I don't yet have a feel for how far reaching that is, but wonder if it's another iceberg like scenario, with a small visible surface area here pulling a much larger unstable and future possibility mass behind it.

I'm still assimilating the technical aspects, but those are my initial thoughts on (mostly) the presentation. Hopefully they're useful. I'm looking forward to the possibility of custom Try types and early successful returns.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

[scottmcm]

Huzzah! The rust language and library teams have decided to accept this RFC.

To track further Try discussion, subscribe to the tracking issue here: rust-lang/rust#84277

For ControlFlow, subscribe to the control_flow_enum tracking issue here: rust-lang/rust#75744

[m-ou-se]

edit:
Apparently the only useful feature of try_trait was in fact removed, thanks.

@tvallotton impl FromResidual<Option<Infallible>> for Error works fine.

People are working hard on improving the language. I don't know why you felt the need to add a sarcastic "thanks" in an edit of your comment, or why you think this is 'the only useful feature of try_trait', but snarky comments like that are not welcome here. I've hidden your comment.