Add homogeneous_try_blocks RFC#3721
Conversation
c5f49c0 to
e37d4f3
Compare
|
Big +1 on "the common case should just work". We could just have method for people who want the type-changing case, like |
|
@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 |
|
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 |
|
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. |
| use std::io::{self, BufRead}; | ||
| pub fn concat_lines(reader: impl BufRead) -> io::Result<String> { | ||
| let mut out = String::new(); | ||
| reader.lines().try_for_each(|line| -> io::Result<()> { // <-- return type |
There was a problem hiding this comment.
| 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?
There was a problem hiding this comment.
I think I'd still spell that as io::Result<_>, but sure, could remove the () from here.
|
Could it be possible to request this as a fallback for the closure (and async blocks) case too? |
|
@Nadrieril Good point! The majority case there is just @dev-ardi Unfortunately "fallback" in the type system is extremely complicated, especially as things get nested. If you have a My thought here is that the |
|
@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. |
|
What would be the downside of adding this desugaring ( |
| the normal case is definitely that it just stays a `None`. | ||
|
|
||
| This RFC proposes using the unannotated `try { ... }` block as the marker to | ||
| request a slightly-different `?` desugaring that stays in the same family. |
There was a problem hiding this comment.
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.
|
|
||
| ## This case really is common | ||
|
|
||
| The rust compiler uses `try` blocks in a bunch of places already. Last I checked, they were *all* homogeneous. |
There was a problem hiding this comment.
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 ?).
There was a problem hiding this comment.
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::Errorseems 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.
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
For folks using anyhow, needing to annotate the return type of a block isn't a huge deal because the library has an alias, anyhow::Ok. In the projects I've worked in using anyhow, it's a huge lifesaver for closures that return anyhow::Result<T>.
I know that there's probably consensus on implicit Ok-wrapping in try blocks, but explicitly spelling it out and having an Ok alias like anyhow would be the sweet path for me and the projects I work on. Basically:
try {
let saves = foo()?.bar()?;
self.cache_savegames(saves)?;
anyhow::Ok(())
}.context("failed to load savegame list")?;We also have a lot of code that uses anyhow with heterogeneous error types. Oftentimes our .context call happens outside the big block of ? spam, like when a function has a bunch of small bits of I/O.
There was a problem hiding this comment.
I know that there's probably consensus on implicit Ok-wrapping in try blocks
Yes, that was decided in rust-lang/rust#70941
That said, I've made a PR (rust-lang/rust#149489) to experimentally allow
try bikeshed anyhow::Result<_> {
let saves = foo()?.bar()?;
self.cache_savegames(saves)?;
}.context("failed to load savegame list")?;so that people can try that out in nightly as well and see how it goes.
(Notably with the type embedded in the syntax, so you don't need the let temp: anyhow::Result<_> = try { … }; temp.context(…)? dance.)
|
@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. |
| [a bunch of `io` operations](https://github.com/rust-lang/rust/blob/d6f3a4ecb48ead838638e902f2fa4e5f3059779b/compiler/rustc_borrowck/src/nll.rs#L355-L367) which all use `io::Result`. Additionally, `try` blocks work with | ||
| `?`-on-`Option` as well, where error-conversion is never needed, since there is only `None`. | ||
|
|
||
| It will fail to compile, however, if not everything shares the same error type. |
There was a problem hiding this comment.
Should this be included in the downsides?
|
I'm a bit concerned about this change. Applications and libraries often use crates 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 |
|
I think that changing behavior of I think we need a more consistent solution which will work for both closures and For example: fn foo() -> Result<(), E1> { ... }
fn bar() -> Result<(), E1> { ... }
fn baz() -> Result<(), E2> { ... }
fn zoo<E: Error>() -> Result<(), E> { ... }
// Works. Evaluates to `Result<u32, E1>`
let val = try {
foo()?;
bar()?;
42u32
};
// This also works. Because we "bubble" `Result`, this block evaluates to `Result<(), E1>`
let val = try {
foo()?;
bar()?;
};
// Same for closures. The closure returns `Result<u32, E1>`.
let f = || {
foo()?;
bar()?;
Ok(42u32)
};
// Compilation error. Encountered two different "break" types (`E1` and `E2`),
// additional type annotations are needed
let val = try {
foo()?;
baz()?;
42u32
};
// Compilation error. Generics also require explicit annotations.
// Same for mixing `Option` and `Result` in one block.
let val = try {
foo()?;
zoo()?;
42u32
};In the case of nested |
I admit I have no idea how rustc handles this, but I believe this is complicated due to type inference. Generics might be parametric in the return type, so there's no "type" you can "collect" before settling on the type, at which point it's too late to reconsider the choice. I think what you want is a fallback in type inference: try to union all return types (as in HM; I'm not sure if that's how rustc handles this), and only consider |
|
Hi team, This RFC has been open for an almost a year, and it addresses a genuinely painful ergonomics issue with Importantly, this RFC directly solves one of the explicit stabilization blockers listed in the
This isn't just a nice-to-have ergonomic improvement – it's solving a documented requirement for I’d really appreciate if someone from the team could share any insights on whether there are specific blockers preventing this RFC from moving forward (e.g., design concerns, implementation complexity, etc.) Thanks so much for your time and all the hard work on this! |
|
I'm moving this from lang-radar to lang-nominated since our nomination queue is finally close to empty and this RFC should be a priority. (This should wait for a meeting where @scottmcm is available to discuss.) |
|
Thanks @scottmcm for this RFC. I think this is a good approach and I'm very happy to resolve this open question for Another point was made in the lang meeting: If we later find a better way than homogeneous try blocks (say, by making inference more powerful), we can easily migrate to that way over an edition. All that edition migration would need is an explicit form that states the type, and we want such a form anyway for heterogeneous try blocks. @rfcbot reviewed |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
| ``` | ||
|
|
||
|
|
||
| # Drawbacks |
There was a problem hiding this comment.
A context-dependent change of behavior is surprising enough in general... but it is even more so in a language with macros.
How is the user supposed to know what is the meaning of ? when invoked in the context of a macro:
fn my_function(x: Result<i32, X>, y: Result<i32, Y>) -> Result<i32, R> {
some_macro! {
Ok(x? + y?)
}
}If some_macro doesn't encapsulate the passed code in a try block, this code will compile as long as X and Y are convertible to R.
If some_macro does encapsulate the passed code in a try block, this code no longer compiles as the return type of x? and y? is not homogeneous.
That's... very much not obvious, and it will require very good diagnostics to explain to the user that inference failed because of a try block they can't see...
There was a problem hiding this comment.
A context-dependent change of behavior is surprising enough in general... but it is even more so in a language with macros.
how is it different from e.g. a break changes the behavior depending on whether some_macro contains a loop
and tbh this "change of behavior" already happens if some_macro wraps the code in an async block
There was a problem hiding this comment.
Yeah, this is just already how macros work because the context for ? is non-hygienic. If that macro wraps it in (|| -> Result<_, Q> { ... body here ... })() (IIFE-style) then you have a similar problem since it's a different context.
EDIT later: If one wanted to "fix" that, then it would be a matter of using the hygiene of the ? to match to a context, which is at least hypothetically possible, like how block-break-value break 'foo x is hygienic on the label name and won't pick up a 'foo from inside the macro definition. Whether that's always better is non-obvious, though, and would certainly a bunch of work -- and as kennytm mentions, would probably also end up wanting to affect things like continue as well, since those are also non-hygienic.
|
@rfcbot reviewed Agreed this is the right move overall. Thanks to @scottmcm for a carefully-written RFC, as usual. As we discussed in a recent lang meeting, probably we'll want to see heterogeneous |
| # Future possibilities | ||
| [future-possibilities]: #future-possibilities | ||
|
|
||
| ## Annotated heterogeneous `try` blocks |
There was a problem hiding this comment.
Today, there are many ways to hint the type when inference fails:
- Annotate the variable created,
let var: Result<_, E> = try { ... }; - Use the variable created in an unambiguous context,
let var = try { ... }; do_it(var);. - Add the type somewhere in the expression,
let var = try ☃️ Result<_, E> { ... };.
This section proposes turning off the special behavior of ? based on a special annotation of the try block, ie (3); there seems to be a number of missing scenarios.
Incomplete annotation
let var: Vec<_> = try { foo(x?, y?).collect() }?;
let var = try ☃️ Result<Vec<_>, _> { foo(x?, y?).collect() }?;As defined, the first syntax works -- hinting the collection to collect to, without turning off homogenization -- while the second syntax doesn't -- it doesn't hint the error type to unify to.
Annotated variable
let var: anyhow::Result<Vec<_>> = try { foo(x?, y?).collect() };
let var = try ☃️ anyhow::Result<Vec<_>> = try { foo(x?, y?).collect() };This is the opposite scenario, the first syntax doesn't work -- homogenization is still on, despite the desired error type being clear -- while the second does -- homogenization is off.
Sometimes the variable should be annotated, sometimes the try should be annotated. This is messy :/
Unambiguous context
handle(try { foo(x?, y?).collect() })?;The syntax doesn't work, as homogenization still occurs even though the method forces the error type.
Of course, there's no way at the syntactic level to know whether the type is known or not.
It would be better if homogenization could be turned off from day 1 -- perhaps with try☃️ _ -- but it still seems overall messy.
| is not worth the complexity and that adding type annotations instead is fine. | ||
|
|
||
|
|
||
| # Rationale and alternatives |
There was a problem hiding this comment.
One missing alternative would be a (non-existing) universal syntax for inline type hinting.
There are already today expressions which require type hinting, a common example being collect calls and into calls, and how to do so is a tad haphazard:
let var: C = x.collect();orlet var = x.collect::<C>();.let var: C = x.into();orlet var = <X as Into<C>>::into(c);.
A universal syntax for inline typing hinting would be more uniform than the current case-by-case solutions: teach once, use everywhere.
For example:
iterator.collect().as::<BTreeSet<_>>().into_iter().x.into().as::<Foo>().call(...).try { slice.get(i)? + slice.get(j)? }.as::<Option<_>>().unwrap_or(-1).
It is not the tersest option, but I do believe it should be added to the potential set of alternatives.
Notably, I do note that if such a universal syntax for inline type hinting -- whatever its form -- was added in the future, then the costs/benefits analysis of this RFC would change significantly. Thus, it seems to me this RFC should demonstrate that it would still be beneficial enough in the presence of such universal syntax to warrant inclusion, despite its warts (such as how awkward deactivating homogenization is).
There was a problem hiding this comment.
This feature is called "type ascription", you should be able to find a lot of discussion about it.
There was a problem hiding this comment.
Thus, it seems to me this RFC should demonstrate that it would still be beneficial enough in the presence of such universal syntax to warrant inclusion
This is what things like https://github.com/scottmcm/rfcs/blob/if-at-first-you-dont-succeed/text/3721-homogeneous-try-blocks.md#this-case-really-is-common are getting at.
To me, the core piece of this RFC is that this simple case should do the thing that people will obviously expect it to do. try { x? + 1 } should be x.map(|x| x + 1), not anything else. It should never require that you specify "yes, I still want it to be a Result/Option/ControlFlow, same as it was before", because of course that's what you want. And I'm perfectly willing to accept "and if you don't want homogeneity like that, you need some kind of marker instead" because those situations are fundamentally not the "simple" ones.
Notably it doesn't matter how short the annotation is, I still think it's too much to require every time for something like try { a.get(i)? + a.get(j)? }.unwrap_or(0).
Of course, if years down the line the solver gets some new smarts that means it can just "prefer" homogeneous in some way, that'd be cool too, so it could just "do the right thing". But I don't think we need to wait for that -- we could always used the edition of the try token to decide whether things should be using the particular behaviours defined here vs the "now that we have a smarter solver do this other thing".
There was a problem hiding this comment.
This is what things like https://github.com/scottmcm/rfcs/blob/if-at-first-you-dont-succeed/text/3721-homogeneous-try-blocks.md#this-case-really-is-common are getting at.
And I agree with the idea, I just think it should be spelled out explicitly in an Alternatives section dedicated to a universal type ascription mechanism.
That is, the section should mentioned that in the presence of type ascription, which solves the "generic" problem type inference without introducing extra variables, it is still valuable to make the common case ergonomic, as per the part you mentioned.
There was a problem hiding this comment.
To me, the core piece of this RFC is that this simple case should do the thing that people will obviously expect it to do.
try { x? + 1 }should bex.map(|x| x + 1), not anything else. It should never require that you specify "yes, I still want it to be a Result/Option/ControlFlow, same as it was before", because of course that's what you want. And I'm perfectly willing to accept "and if you don't want homogeneity like that, you need some kind of marker instead" because those situations are fundamentally not the "simple" ones.
I agree with homogeneity in Option -> Option, Result -> Result, ControlFlow -> ControlFlow.
However, I think that while it works really well for Option, there will be still type inference issues for Result & ControlFlow caused by the enforced residual homogeneity.
As a simple example, let's take:
fn foo() -> Result<i32, FooError>;
fn bar() -> Result<i32, BarError>;
try { foo()? + bar()? }.unwrap_or(0)As per the Playground, we get:
error[E0308]: mismatched types
--> src/main.rs:20:48
|
20 | ControlFlow::Break(r) => break $lt make_try_type(r),
| ^^^^^^^^^^^^^^^^ expected `Result<_, FooError>`, found `Result<_, BarError>`
...
39 | try { yeet!('a foo()) + yeet!('a bar()) }
| --------------- in this macro invocation
|
= note: expected enum `Result<_, FooError>`
found enum `Result<_, BarError>`
= note: this error originates in the macro `yeet` (in Nightly builds, run with -Z macro-backtrace for more info)
help: use the `?` operator to extract the `Result<_, BarError>` value, propagating a `Result::Err` value to the caller
|
20 | ControlFlow::Break(r) => break $lt make_try_type(r)?,
| +Just because one wants Result -> Result or ControlFlow -> ControlFlow does not mean that one wants E -> E or B -> B, which the proposed desugaring enforces.
Differing error types requiring unification is a common usecase too, and the proposed desugaring hinders it.
There was a problem hiding this comment.
Differing error types are heterogeneous in all the ways that matter. Doing try bikeshed Result<_, _> { x? }? even when x: Result<_, _> has all the same problems: the From::from calls in how Result does ? mean that it's essentially the same as .into().into(), which also doesn't work.
So yea, I agree that different error types requiring unification is a common use case, but it's fundamentally not the homogeneous use case.
There was a problem hiding this comment.
Differing error types are heterogeneous in all the ways that matter.
Yes they are, this is why I consider this RFC -- as is -- a mistake.
Without this RFC, the following is at least possible:
let result: Result<_, Box<dyn Error>> = try {
foo()?; // raises FooError
bar()? // raises BarError
};
let result = result?;(And so is handle(try { foo()?; bar()? }) if handle "pins" the error type in some way)
With this RFC, however, this perfectly fine code no longer compiles, despite the target error type being fixed by the caller. This is a regression in ergonomics.
So yea, I agree that different error types requiring unification is a common use case, but it's fundamentally not the homogeneous use case.
There are two levels of homogeneity: homogeneous type family, and homogeneous type.
I think we both agree that an homogeneous type family is a perfectly fine restriction to impose in general.
I argue that an homogeneous type is NOT a fine restriction to have. Making the use of try worse for Result to make it better for Option when they each account for half the usecases seems like a poor trade-off.
It seems possible to me to alter make_try_type to pin the family without fully pinning the type (for Result and ControlFlow), thereby improving the ergonomics for Option without regressing them for Result.
There was a problem hiding this comment.
I wanted to acknowledge this thread before merging, since I don't think any of the factual things you're saying are incorrect. It's absolutely true that this RFC is choosing to make certain things "impossible" with try blocks in order to make other things less noisy. We had a short discussion about that in the lang meeting today. The plan is that that doesn't need to block this RFC, though, with the intent to fast-follow this RFC with another one so that
let result = try as Result<_, Box<dyn Error>> {
foo()?; // raises FooError
bar()? // raises BarError
};or
handle(try _ { foo()?; bar()? })or something will be available for those scenarios. We don't want to say "those aren't valuable", just "they're not this RFC".
Co-authored-by: Daniel Scherzer <daniel.e.scherzer@gmail.com> Co-authored-by: scottmcm <scottmcm@users.noreply.github.com>
|
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. |
|
Huzzah! The @rust-lang/lang team has decided to accept this RFC. To track further discussion, subscribe to the tracking issue here: rust-lang/rust#154391 If you were interested in this RFC, you might also be interested in following the experimental tracking issue for heterogeneous try blocks: rust-lang/rust#149488 It has two sub-issues under active discussion to determine the direction of that experiment. |
View all comments
Tweak the behaviour of
?insidetry{}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 atry{}block is untouched.Rendered