try_trait_v2: A new design for the ? desugaring by scottmcm · Pull Request #3058 · rust-lang/rfcs (original) (raw)

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.