Tracking Issue for try_trait_v2, A new design for the ? desugaring (RFC#3058)

[scottmcm]

This is a tracking issue for the RFC "try_trait_v2: A new design for the ? desugaring" (rust-lang/rfcs#3058).
The feature gate for the issue is #![feature(try_trait_v2)].

This obviates rust-lang/rfcs#1859, tracked in #42327.

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

• Implement the RFC
  • Add the new traits and impls
  • Update the desugar in ast lowering
  • Fixup all the tests
  • Add nice error messages in inference
  • Improve perf with enough MIR optimizations
  • Delete the old way after a bootstrap update Remove the TryV2 alias #88223
• Not strictly needed, but a mir-opt to simplify the matches would really help: LLVM/MIR optimizations to simplify the new RFC3058 ? desugaring #85133
• Add more detailed documentation about how to implement and use the traits
• Decide whether to block return types that are FromResidual but not Try
• Fix rustdoc to show the default type parameter on FromResidual better (Issue rustdoc removes Try from <Self as Try>::Residual in std::ops::FromResidual #85454)
• Before stabilizing, ensure that all uses of Infallible are either fine that way or have been replaced by !
• Stabilizing this will allow people to implement Iterator::try_fold
  • As part of stabilizing, document implementing try_fold for iterators (perhaps reopen Document implementing try_fold for iterators for internal iteration #62606)
  • Ensure that the default implementations of other things have the desired long-term DAG, since changing them is essentially impossible later. (Specifically, it would be nice to have fold be implemented in terms of try_fold, so that both don't need to be overridden.)
• Adjust documentation (see instructions on rustc-dev-guide)
• Stabilization PR (see instructions on rustc-dev-guide)

Unresolved Questions

From RFC:

• 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 experience in nightly:

• Should there be a trait requirement on residuals of any kind? It's currently possible to accidentally be FromResidual from a type that's never actually produced as a residual (Fix the types in one of the FromResidual implementations rwf2/Rocket#1645). But that would add more friction for cases not using the Foo<!> pattern, so may not be worth it.
  • Given the trait in Make array::{try_from_fn, try_map} and Iterator::try_find generic over Try #91286, that might look like changing the associated type Residual; to type Residual: Residual<Self::Output>;.

Implementation history

• Basic traits and impls added, Add the try_trait_v2 library basics #84092
• Removing try_trait from stdarch, Remove #![feature(try_trait)] from a test stdarch#1142
• Implementing the desugaring, Implement the new desugaring from try_trait_v2 #84767

[artemii235]

We have a problem in our project related to the new question mark desugaring. We use the track_caller feature in From::from implementation of the error types to collect stack traces with generics and auto and negative impl traits magic implemented by @sergeyboyko0791 (https://github.com/KomodoPlatform/atomicDEX-API/blob/mm2.1/mm2src/common/mm_error/mm_error.rs).

After updating to the latest nightly toolchain this stack trace collection started to work differently. I've created a small project for the demo: https://github.com/artemii235/questionmark_track_caller_try_trait_v2

cargo +nightly-2021-05-17 run outputs Location { file: "src/main.rs", line: 18, col: 23 } as we expect.

cargo +nightly-2021-07-18 run outputs Location { file: "/rustc/c7331d65bdbab1187f5a9b8f5b918248678ebdb9/library/core/src/result.rs", line: 1897, col: 27 } - the from_residual implementation that is now used for ? desugaring.

Is there a way to make the track caller work the same way as it was before? Maybe we can use some workaround in our code?

Thanks in advance for any help!

[cuviper]

That's interesting -- maybe Result::from_residual could also have #[track_caller]? But that may bloat a lot of callers in cases that won't ever use the data.

[thomaseizinger]

From the description:

A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.

@artemii235 Do you mind opening a separate issue?

[artemii235]

Do you mind opening a separate issue?

No objections at all 🙂 I've just created it #87401.

[crlf0710]

May i suggest changing branch method's name to something else? When searching for methods, it's a little not obvious to see Option::branch or Result::branch is not the method one should usually call...

[huntiep]

How do I use ? with Option -> Result now? Before it was only necessary to implement From<NoneError> for my error type.

[tmccombs]

Use .ok_or(MyError)?

[RagibHasin]

Why the implementation of FromResidual for Result uses trait From in stead of Into. According to the documentation of trait Into and From, we should

Prefer using Into over From when specifying trait bounds on a generic function to ensure that types that only implement Into can be used as well.

Clarification is welcome as an error type implementing only Into trait arises with associated error type on traits and associated types cannot bind on From for lack of GATs.

[steffahn]

@RagibHasin

see #31436 (comment)
and #31436 (comment)
(and the following discusion, respectively)

[BGR360]

Hi, I'm keen to see this stabilized. Is there any work that can be contributed to push this forward? It would be my first Rust contribution, but I have a little experience working on compiler code (little bit of LLVM and KLEE in college).

[scottmcm]

@BGR360 Unfortunately the main blockers here are unknowns, not concrete-work-needing-to-be-done, so it's difficult to push forward. It's hard to ever confirm for sure that people don't need the trait split into parts, for example.

Have you perhaps been trying it out on nightly? It's be great to get experience reports -- good or bad -- about how things went. (For example, #42327 (comment) was a big help in moving to this design from the previous one.) If it was good, how did you use it? If it was bad, what went wrong? In either case, was there anything it kept you from doing which you would have liked to, even if you didn't need it?

[BGR360]

Experience Report

@scottmcm I have tried #[feature(try_trait_v2)] in its current form. I'll give an experience report:

Overall my experience with this feature is positive. It may end up being critical for my professional work in Rust.

My use case is very similar to @artemii235: #84277 (comment). At my work, we need a way to capture the sequence of code locations that an error propagates through after it is created. We aren't able to simply use std::backtrace to capture the backtrace at the time of creation, because errors can propagate between multiple threads as they bubble up to their final consumer. The way we do this in our C code is to manually wrap every returned error value in a special forward_error macro which appends the current __file__, __line__, and __func__ to the error's backtrace.

We would love to be able to do this in our Rust code using just the ? operator, no macros or boilerplate required. So I experimented with implementing my own replacement for std::result::Result (call it MyResult). I implemented std::ops::Try on MyResult in a very similar manner to std::result::Result, but I annotated FromResidual::from_residual with #[track_caller] so that I could append the location of the ? invocation to the error's backtrace. The experiment was successful and relatively straightforward.

To get this to work, I made express use of the fact that you can implement multiple different FromResidual on a type (I think that might be what you're referring to when you say "splitting the trait into parts"?). I have one FromResidual to coerce from std::result::Result to my MyResult, and another one to coerce from MyResult to MyResult.

I'd be happy to give more specifics on how I achieved my use case either here or on Zulip, just let me know :)

Pros:

• Allows me to implement multiple FromResidual for my Try type. This was critical for my use case.

Cons:

• Documentation is a little weak, but I was able to learn by example by reading the source code for std::result::Result.
• It'd be great to be able to achieve my use case without having to rewrite Result. See my other comment below.

[kevincox]

Experience report

I was using try_trait on an app of mine and upgraded to try_trait_v2 because the build started failing on the latest nightly. My use case was a bit weird as I am using the same type of Ok and Err variants as it is a generic Value type for a programming language. However the try operator is still incredibly helpful in the implementation.

Pros:

• The conversion was localized.

Cons:

• More code to get it to work.
• Many more new concepts than try_trait. For example I now need to use:
  • ControlFlow which is fairly straight forward (although I don't know why the arguments are backwards compared to Result.
  • Residual which I still barely understand and the name is incredibly perplexing. "Residue" is something left over but it isn't clear what is being left over in this case.
• The docs are not very helpful. I had to guess the impl<E: Into<Val>> std::ops::FromResidual<Result<std::convert::Infallible, E>> for Val incantation from the error messages and it still isn't completely clear to me how this type comes to be.

Overall this v2 is a clear downgrade for this particular use case however the end result isn't too bad. If this is making other use cases possible it is likely worth it with better names and docs.

The full change: https://gitlab.com/kevincox/ecl/-/commit/a1f348633afd2c8dd269f95820f95f008b461c9e

[BGR360]

So I experimented with implementing my own replacement for std::result::Result (call it MyResult).

This is actually a little bit unfortunate, in retrospect. It would be much better if I could just make use of std::result::Result as it already exists. That would require two things that are missing:

• <std::result::Result as FromResidual>::from_residual would need to have #[track_caller]
• I would need to be able to intercept invocations of From<T>::from() -> T so I can push to the stack even when the ? operator does not coerce the result to a different error type.

To illustrate, here's how things work in my experiment:

pub struct ErrorStack<E> {
    stack: ..,
    inner: E,
}

impl<E> ErrorStack<E> {
    /// Construst new ErrorStack with the caller location on top.
    #[track_caller]
    fn new(e: E) -> Self { ... }

    /// Push location of caller to self.stack
    #[track_caller]
    fn push_caller(&mut self) { ... }

    /// Return a new ErrorStack with the wrapped error converted to F
    fn convert_inner<F: From<E>>(f: F) -> ErrorStack<F> { ... }
}

pub enum MyResult<T, E> {
    Ok(T),
    Err(ErrorStack<E>),
}

pub use MyResult::Ok;
pub use MyResult::Err;

impl<T, E> Try for MyResult<T, E> {
    type Output = T;
    type Residual = MyResult<Infallible, E>;

    /* equivalent to std::result::Result's Try impl */
}

/// Pushes an entry to the stack when one [`MyResult`] is coerced to another using the `?` operator.
impl<T, E, F: From<E>> FromResidual<MyResult<Infallible, E>> for MyResult<T, F> {
    #[inline]
    #[track_caller]
    fn from_residual(residual: MyResult<Infallible, E>) -> Self {
        match residual {
            // seems like this match arm shouldn't be needed, but idk the compiler complained
            Ok(_) => unreachable!(),
            Err(mut e) => {
                e.push_caller();
                Err(e.convert_inner())
            }
        }
    }
}

/// Starts a new stack when a [`std::result::Result`] is coerced to a [`Result`] using `?`.
impl<T, E> FromResidual<std::result::Result<Infallible, E>> for Result<T, E> {
    #[inline]
    #[track_caller]
    fn from_residual(residual: std::result::Result<Infallible, E>) -> Self {
        match residual {
            // seems like this match arm shouldn't be needed, but idk the compiler complained
            std::result::Result::Ok(_) => unreachable!(),
            std::result::Result::Err(e) => Err(StackError::new(e)),
        }
    }
}

If std::result::Result had #[track_caller] on its FromResidual::from_residual, then I could avoid everything above by just pushing to the stack inside an impl From:

impl<E, F: From<E>> From<ErrorStack<E>> for ErrorStack<F> {
    #[track_caller]
    fn from(mut e: ErrorStack<E>) -> Self {
        e.push_caller();
        e.convert_inner()
    }
}

However, this does not work because it conflicts with the blanket From<T> for T implementation.

I could limit my From to types E, F such that E != F, but I need functions to show up in my error trace even if the residual from ? does not change types. For example:

fn foo() -> MyResult<(), io::Error> {
    fs::File::open("foo.txt")?;
}

fn bar() -> MyResult<(), io::Error> {
    // I need bar to show up in error traces, so I wrap with Ok(..?).
    // Without my custom MyResult, I am unable to intercept this invocation of the `?` operator, because
    // the return type is the same as that of `foo`.
    Ok(foo()?)
}

[luksan]

Experience report

I used try_traits_v2 to implement FromResidual for Result<Infallible, impl Into> and Option for a custom Iterator<Item=Result<Value,EvalError>>. I use this in an AST walker where each node can return zero or more Values, or an error. Before I implemented FromResidual I had to use Result<iterator, EvalError> as return type in the visitor methods in order to use "?", which caused a lot of Ok-wrapping and having to handle the fact that an error could be both the outer result or inside the iterator. Also, returning an empty iterator was very explicit.

After FromResidual was implemented I could change the return type on the visitors "iterator" and still use Err(EvalError)? to return errors inside an iterator, or return an empty iterator with None?. All the Ok-wrapping went away.

It was quite straightforward to figure out what was needed. The only stumbling blocks was that the default for R in FromResidual made the RustRover autocomplete the impl skeleton incorrectly for my usage, which caused a few minutes of head-scratching. The other thing that took a few tries was to see that the Ok type on the Result impl must be Infallible, but in that case the type errors from rustc were quite helpful.

All in all, great feature. Works for me. I don't mind the "Residual" name, even though it's not intuitive for me. Just another concept to learn. The default for R might cause more problems than it solves, though.

[paulyoung]

I recently tried this out by implementing an Either type (isomorphic to Result)

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=4301a1db0befb2c3b68b5a94abb28c32

I still don't understand why the type of L (equivalent to Ok in Result) can't be inferred. I needed to provide type arguments when I expected not too.

It's possible that I got something wrong or had incorrect expectations but wanted to share my experience in case this wasn't intended.

Thanks!

[WaffleLapkin]

@paulyoung as you've written, L is the equivalent of Err. Try::Output is the type of x?. Second of all, you don't need all the types, just the ones that can't be inferred.

In let foo = Either::<String, _>::Right("foo".to_string())?; and let foo_bar_baz = Either::Right(format!("{foo_bar} baz"))? the left type can't be inferred because the compiler can't solve ?T with the only bound being String: From<?T> (there are many types for which this is true) (remember that L is the Err-like here). Note that you specify the From bound in the FromResidual impl (if you remove the F and From it will work):

impl<L, R, F: From<L>> FromResidual<Either<L, Infallible>> for Either<F, R> {

With let foo_bar = Either::<_, String>::Left(format!("{foo} bar"))? the right is otherwise unbounded also (Left is Err-like, which means it is just returned and then the R could be anything which implements Display).

[ianks]

At risk of sounding overly ambitious, let's… ship this?

The overall sentiment has been positive. People feel it's solving real problems and filling an important gap.

Two quick decisions could unblock us:

1. FromResidual: Keep the flexibility or simplify?
2. Stick with Output/Residual or change? If someone is passionate about this one, please speak up but let’s not shed too much

Thoughts on pushing this across the finish line?

[Amanieu]

@scottmcm This was discussed a few weeks ago in a libs-api meeting in which you participated. We decided that this feature probably needs a dedicate design meeting. Could you prepare for the design meeting a document breaking down the open questions, what could be partially stabilized, etc. We would then discuss this during in the first hour of the next libs-api meeting (or the next one after you have had a chance to compile the document).

[apodolsk]

Is there a bad interaction between FromResidual and the orphan rule? The following violates the orphan rule:

impl<E> FromResidual<SomeOrRet<!, E>> for E {
    fn from_residual(_residual: SomeOrRet<!, E>) -> Self {
        unsafe { _residual.0.err().unwrap_unchecked() }
    }
}

pub trait AsSomeOrRet<T> {
    fn or_ret<E>(e : E) -> SomeOrRet<T, E>;
}

impl<T> AsSomeOrRet<T> for Option<T> {
    fn or_ret<E>(e : E) -> SomeOrRet<T, E> {
        SomeOrRet(Err(e))
    }
}

I have a function which wants to return partial output if it's forced to stop partway through. In this particular case, it's a data structure search function which wants to record the major steps of a failing search to inform a potential future insert. I want to write option_val.or_ret(progress_state)? to do this.

I'm successfully using these same traits to implement Option -> () short circuiting, so I could use a ()-typed try block and perform the return manually, outside of the block. But it seems I'm forced to write let _: () = try, which feels bad.

I haven't read the discussions about these traits carefully, and am too much of a beginner to have a concrete suggestion, a confident analysis, or even confidence that what I want is reasonable and the answer isn't "better try block ergonomics". But it seems suspicious to me that I'm hitting orphan rule issues given that I'm using a local type to define an Option -> E conversion.

[oshaboy]

Should the Try trait also provide unwrap unwrap_unchecked, unwrap_or and map? That way you can use them in generic code without having to re-implement them.

[bjoernager]

Should the Try trait also provide unwrap unwrap_unchecked, unwrap_or and map? That way you can use them in generic code without having to re-implement them.

I think that this thought does show some merit. Currently, this can be achieved using the return value from Try::branch, but this approach is quite cumbersome.

Implementing this does not, obviously, cover secondary variants, such as those covered by e.g. Result::unwrap_err. But I don't think these necessarily have to be covered generically. After all, the current Try trait generally puts the emphesis on Output, whereas the details of Residual are more left up to the implementor (at least that is how it is done in practice).

[programmerjake]

imo if you have unwrap you should also have expect since expect is commonly used to provide documentation for why it shouldn't end up panicking.

[bjoernager]

Result (at least) also has unwrap_or_else and unwrap_or_default.

[oshaboy]

Ok so I understand implementing Try for Floats is a bad idea. But what about implementing Try for ExitCode and ExitStatus?

[bjoernager]

I believe ExitCode is not meant to be handled by user code – only the operating system. The type already doesn't expose much functionality.

It seems the same also goes for ExitStatus.

[tmccombs]

user code can handle ExitStatus and ExitCode when determining whether a child process has succeeded.

And ExitCode implements the Termination trait, so can be used as a return value for main.

I could see wanting to use ? to propagate exit codes from child processes up to the main function to return the same status code to the parent process.

[bjoernager]

And ExitCode implements the Termination trait, so can be used as a return value for main.

Yes, partly because Termination::report already returns some ExitCode value.

I could see wanting to use ? to propagate exit codes from child processes up to the main function to return the same status code to the parent process.

ExitCode does not define a portable success/failure pattern other than the SUCCESS and FAILURE constants, but the set of distinct values may still be larger than these two internally. This would make it non-trivial for user code to predict Try::branch, thus rendering the function a platform-dependent, black box. But maybe this isn't a problem at all (I don't personally know). Although IIRC, the raw values can also have differing behaviour depending on the shell being used.

[programmerjake]

I expect ExitCode to be a subset of the values of ExitStatus, and ExitStatus has success, so it should be possible to define Try on both ExitStatus and ExitCode using the same logic.