Add homogeneous_try_blocks RFC

[scottmcm]

Tweak the behaviour of ? inside try{} blocks to not depend on context, in order to work better with methods and need type annotations less often.

The stable behaviour of ? when not in a try{} block is untouched.

Rendered

[Nadrieril]

Big +1 on "the common case should just work". We could just have method for people who want the type-changing case, like foo().bikeshed_change_try_type()?, right?

[joshtriplett]

@scottmcm Can you clarify why this is a T-libs-api RFC in addition to a T-lang one? The implementation of this might involve changes to the traits underlying Try, but it seems like the key purpose of this RFC is "how should try and ? interact", rather than the implementation of that.

[Nadrieril]

Should we wait a few weeks before nominating for T-lang? A few recent RFCs felt rushed to me because they were FCPed very quickly. I'd like to see more comments from the community before, especially when this is pushed by a lang member

[Lokathor]

Nominating only makes it show up on the generated meeting agendas for T-lang triage, but given the pace of those meetings in recent years it doesn't really mean that this RFC will necessarily be discussed at the next meeting, or even in the next 3 meetings. And the RFC theoretically could be discussed at the meeting even without it being nominated because people can just bring up topics they feel need to be discussed. There's basically no reason to not nominate this if 2 lang members already take this proposal seriously.

[dev-ardi]

Suggested change

	reader.lines().try_for_each(|line| -> io::Result<()> { // <-- return type
	reader.lines().try_for_each(|line| -> Result<_, io::Error> { // <-- return type

We only need to fill in what it doesn't know, I think this does a better job at that?

[scottmcm]

I think I'd still spell that as io::Result<_>, but sure, could remove the () from here.

[dev-ardi]

Could it be possible to request this as a fallback for the closure (and async blocks) case too?
Adding the try {...}? block as a workaround is not that much different from specifying the type of the Err, it's still an annoying papercut.

[scottmcm]

@Nadrieril Good point! The majority case there is just .map_err(Into::into) -- I've added a section. We could probably do something similar for residual-to-residual conversion too, though it's unclear how commonly needed that would be.

@dev-ardi Unfortunately "fallback" in the type system is extremely complicated, especially as things get nested. If you have a try block inside another try block, for example, deciding which one falls back is non-obvious. See also multiple years of discussion about wishing we could have "fallback to usize" for indexing, for example, in order to allow indexing by more types without changing how existing code infers.

My thought here is that the try marker is particularly tolerable for the closure case, because adding the try{} also allows removing the Ok() from the end. So in the unit case, using try actually means fewer characters than a fallback rule.

[scottmcm]

@joshtriplett I originally just put lang, but then thought "well but I did put traits in here", so added libs-api too in case.

You're absolutely right that my intent here is about the interaction, rather than the detail. So if libs-api wishes to leave this to lang, that's fine by me. I just didn't want to assume that.

[dev-ardi]

What would be the downside of adding this desugaring (make_try_type()) to everything? That should solve the issue with closures and async bocks too, right?

[RalfJung]

Given that the motivation section beings with closures, this feels like an unexpected segue. I was expecting it the RFC would now explain how the very first example, the closure, can be accepted. It would be good to explain somewhere why that is not an option, and we have to insert a try block (with changed desugaring) instead.

[Kobzol]

I don't know if the compiler is the ideal place to look for evaluating if this case is common or not, as it has very structured code that focuses on a single domain. The examples show essentially only situations where the try block is applied only on options or only on calls that return io::Result. I think that the compiler mostly uses similar types for errors, rather than wrappers like Box<dyn Error> (AFAIK). But in my experience, Rust code in the wild tends to be much more messy, especially in applications.

In particular, I think there is one use-case that should IMO by supported out-of-the-box by try blocks, but I'm not sure how well it would work with homogeneous try blocks, and that is integration with anyhow::Error (as its usage is super common in applications, at least in my experience). For example, a sequence of fallible operations that I encounter all the time in Rust applications (so in very applied messy code that deals with a lot of very different fallible things at once) is trying to fetch something from an API (io::error), then deserializing it (serde::Error) and then e.g. checking some domain invariant, or storing the thing inside a DB (which both would produce a different kind of an error). These cases are then all unified into anyhow::Error. This works perfectly with ? in functions today, but would be quite annoying to do with homogeneous try blocks. Arguably, just spamming ? might not be ideal, but that can be improved with .context/.with_context. It would be quite verbose and boilerplate-y if we had to write foo()[.context("...")].map_err(Into::into)? after every such operation. map_err is great for carefully written library-like code, not so much for high-level application-like code that just wants to get stuff done with the least amount of cruft.

That being said, annotating the try blocks such as try $ anyhow::Error seems like a reasonable solution to fix this, and it seems symmetric with functions (functions require a return type -> heterogeneous ?. try blocks without return type -> homogeneous ?. try blocks with a return type -> heterogeneous ?).

[scottmcm]

These cases are then all unified into anyhow::Error.

Hmm, one interesting possibility here is just using .context() -- that always returns an anyhow::Result<_>. Thus if you have a bunch of disparate error types in your try block, it becomes homogeneous so long as you always write .context("whatever")? instead of just ?.

You wouldn't need the .map_err(Into::into) because the context call has already normalized the error type.

That being said, annotating the try blocks such as try $ anyhow::Error seems like a reasonable solution to fix this

In case you hadn't seen this, that's when I mention in https://github.com/scottmcm/rfcs/blob/if-at-first-you-dont-succeed/text/3721-homogeneous-try-blocks.md#annotated-heterogeneous-try-blocks as potential future work.

[Kobzol]

Ah, good point, I didn't realize that context returns anyhow::Error, but it makes sense of course.

That being said, even though using context is the "proper thing to do" in many (but not all) situations, I do see (and write) code that does indeed just spam ?.

In fact, for me, the try operator is a way to avoid using context everywhere! Usually when I want to use try, it's because I essentially want to group a few errors together, and treat them as one opaque error (to which I then attach the context), but I don't care about the specific errors within the try block so much. So something like try { a?; b?; }.context(...). If the try block could figure out from that context call that I want anyhow::Error, that would also be nice (I saw the discussion about something like this in the RFC).

Yeah, I saw the future possibility with the return type annotation, that's why I mentioned it :)

[afetisov]

@scottmcm I think it's a very nice property of this RFC: it also solves the issue of error type inference for closures and async blocks in a backwards-compatible and syntactically light way. It's kind of obvious, but could you add it as an explicit supporting use case in the RFC? My first thought was that the default error type mechanism could be extended to those cases, but this RFC obviates such additions.

[tmccombs]

Should this be included in the downsides?

[purplesyringa]

I'm a bit concerned about this change. Applications and libraries often use crates like thiserror to automatically group errors. For example, I often write something like

#[derive(Error)]
enum MyError {
    #[error("Failed to parse config: {0}")]
    InvalidConfig(#[from] serde::Error),
    #[error("Failed to connect to server: {0}")]
    ServerConnectionFailed(#[from] io::Error),
    ...
}

which I then use as

fn example() -> Result<(), MyError> {
    let config = parse_config()?; // ? promotes serde::Error to MyError
    let server = connect_to_server(server.url)?; // ? promotes io::Error to MyError
    // ...
}

With this change, this approach would stop working in try blocks.