Add the () → Result<(), _> coercion rule, for removing Ok(()) everywhere.

[kennytm]

• Rendered

Alternative to #2107, considering that is heavily downvoted.

I also have a backup RFC for automatic Ok(()) insertion as an AST pass, but I'm not convinced that adding a coercion rule is actually worse than the AST trickery, so I'm pushing for coercion instead.

cc rust-lang/rust-roadmap-2017#17.

[scottmcm]

Thanks for posting this, @kennytm. There do seem to be irreconcilable differences in #2107, and we discussed at RustConf closing that one for now, revisiting it later as async develops (as that also "wraps").

I am of course sad to not get something that works ∀ T, but I agree that Ok(()) is so bad that we need to find something to do here. And I like that this reiterates always-wrapping catch.

I definitely agree that the coercion is better than the magic AST pass. Having it not be something special will let it grow better as coercions in general develop—imagine being able to pass a fn(T) to something wanting fn(T)->Result<(), E>, for example.

My favourite part of this RFC is the "this isn't an exception to the transitivity rules, but it won't be transitive in practice because none of the other coercions are relevant" table. Things becoming weird in combination was the thing that scared me off the coercion path in general, and this makes me not worried.

I'm intrigued by EmptyTailExpr. Ignoring probably-horrible inference back-compat implementation issues, would having { A; } => { A; EmptyTailExpr::default() } open up any interesting opportunities?

[glaebhoerl]

On its own I think I'd be okay with this, and independently of anything else I would definitely favor making catch behave as originally specified (which is not tremendously surprising given that I did the specifying). However, I think I might prefer a broader solution like throw/throws, and even if it's not strictly-speaking forwards-incompatible, I'm concerned that the two might coexist awkwardly, and that adopting this would cut off other avenues for evolution from a practical perspective. In other words, I'm concerned that while this may be an improvement relative to the current situation, it might get us stuck in an otherwise-suboptimal local optimum.

[strega-nil]

I really dislike this - it's only fixing a minor weirdness in some functions, rather than the nice general solution presented in #2107

[est31]

This RFC is better in some ways and worse in others. I don't really like the fact its implemented as coercion, rather than maing "END OF BLOCK" mean () or Ok(()) depending on situation.

For the good stuff, I like that its limited to () which means that you still need to wrap substantial stuff in Ok(). I guess I have a disagreement here with @ubsan.

Also I like that the RFC is very surgical. It doesn't elevate the entire Try trait to some special new class of types where automatic wrapping is happening.

Now for the stuff where its worse.

Code like this would now compile:

fn foo_0() -> Result<(), E> {
    println!("Hello!");
    ()
}

and

    let r: Result<(), E> = ();

and

fn foo() -> Result<(), E> {
    let mut v = Vec::new();
    v.push(42i32)
}

All three not really nice, because they are unintended consequences that make code less readable and behaviour less understandable. Even less nice, the coercion allows you to attach ? to every function call that returns () (and in any number you want!!!):

fn foo() -> Result<(), E> {
    let mut v = Vec::new();
    v.push(0i32);
    v.push(42)?;
    v.push(200)?????;
}

All of the problems I've outlined above are fixed by the backup RFC with the EmptyTailExpr trait, which is why I prefer it.

[kennytm]

@glaebhoerl In 2015-next ("1.417") / 2018 ("2.0") where throws is introduced, we could deprecate this coercion rule (emit a warning in 2015-next whenever it is used, removed in 2018 or 2021). There's no reason to keep the coercion once the design of throws is done.

Though, the only problem that coercion + throws can cause is double wrapping, which is a type that practically won't appear, so I think it should be fine for them to coexist.

fn foo() -> Result<(), E1> throws E2 {
}
// returns Ok(Ok(()))

[kennytm]

@est31 Yes those are known issues. But when I write the alternatives section of the EmptyTailExpr RFC (yes I wrote the backup first), I find that these drawbacks are not convincing enough to dismiss coercion is the primary solution, given that coercion is so much simpler. The only problem which practically appears is

fn foo() -> Result<(), E> {
    let mut v = Vec::new();
    v.push(42i32)  // <-- no ';'
}

but this has the same behavior as

fn foo() -> Result<(), E> {
    let mut v = Vec::new();
    v.push(42i32);  // <-- with ';'
}

which can also be considered as an advantage. There is also some decision problem like, should EmptyTailExpr be inserted in these cases?

fn a() -> Result<(), E> {
    if cond() { return Err(E::Bad) }
    // ^ yes I've seen code omitting the ';'
}
fn b() -> Result<(), E> {
    if cond() { self.value += read()? }
}

If we include assignments and return to void expressions, the next question is why don't we include () as well, and that pushes us to "why not just use coercion"...

[est31]

should EmptyTailExpr be inserted in these cases?

In the first case, its not really needed (wrapping will work any way), and in the second case you can just put a ; :).

For the first case, it will most likely desugar the implicit else to:

fn a() -> Result<(), E> {
    if cond() {
        return Err(E::Bad)
    }
    else {} // implicit else desugaring
}

Maybe it desugars to else { () } but as of current Rust the two are equivalent. We only need to remove the () here if it makes any trouble.

In any case, this would desugar with the RFC to:

fn a() -> Result<(), E> {
    if cond() {
        return Err(E::Bad)
    } else {
        EmptyTailExpr::empty_tail_expr() // empty tail expr desugaring
    }
}

As of current Rust, any block with return or a call to a function that has ! as return type evals itself to !. It has to in order for fn foo()->u32 {return 42;} to work. E.g. this works: let _: ! = { return };. And ! can desugar to an expression of any type. So you have an if with ! on one branch, and the EmptyTailExpr on the other. It will give you the desired Result.

For the second case, the expression gets desugared to a call to AddAssign::add_assign which has () as return type.

My more general concern with coercions of any kind is that they are weakening the type system and as such are really sharp tools, which I would like to avoid unless there is no other alternative.

[oli-obk]

All three not really nice, because they are unintended consequences that make code less readable and behaviour less understandable

I'm not worried about these constructs, since they are really trivial to detect in as a lint (e.g. in clippy).

[nox]

I am strongly against this RFC. Removing Ok(()) hurts readability and is more magic for beginners to understand.

[glaebhoerl]

In 2015-next ("1.417") / 2018 ("2.0") where throws is introduced, we could deprecate this coercion rule (emit a warning in 2015-next whenever it is used, removed in 2018 or 2021). There's no reason to keep the coercion once the design of throws is done.

We could do this. I don't think we'd actually have the appetite to cause this much churn in user code over a relatively minor issue though.

[oli-obk]

I am strongly against this RFC. Removing Ok(()) hurts readability and is more magic for beginners to understand.

I have encountered the opposite. Students have asked me why they get a mismatched type error when they explicitly used a Result<(), E> to signify that successful completion of a function doesn't result in a value. Their next attempt was to return Ok() or Ok, and the error messages they got then sure didn't help at all:

generics noise

  |     ^^ expected enum `std::result::Result`, found fn item
  |
  = note: expected type `std::result::Result<(), i32>`
             found type `fn(_) -> std::result::Result<_, _> {std::result::Result<_, _>::Ok}`

and "what function???"

error[E0061]: this function takes 1 parameter but 0 parameters were supplied
 --> src/main.rs:2:5
  |
2 |     Ok()
  |     ^^^^ expected 1 parameter

[EDIT] Suggestion for Ok() implemented in rust-lang/rust#44059

I could get behind not doing what this RFC suggests, if the errors were improved enormously instead. So they need suggestions and other helpful things, instead of just stating what's wrong.

The RFC basically just makes the mechanical changes, that you'd to after encountering the error, implicit.

[mark-i-m]

I've gotten very wary of implicit coercion because of JavaScript (yes, I know that's an extreme). I have always loved the explicitness of Rust, and that's why I have favored the other RFC more.

Here is an example of something I'm afraid of

let x = {
    // Do some stuff
    ...
    if blah {
        Ok(())
    } else {
        Err("oh no");
   }
};

if let Ok(_) = x {
    println!("this always prints");
}

Oops... In this case, the type system actually would give false confidence. Scary.

Also, more generally I would like the solution to be specific to exiting a function or block.

[mark-i-m]

Admittedly, I think this would be caught by an "unused value" lint. But it is still scary.

[RalfJung]

Besides the points I raised in #2107 (comment), one thing I dislike about this RFC is that it feels very un-targeted. Making this a coercion means it could fire anywhere, and I don't think we actually want e.g. something like the following to compile:

fn foo(_x: Result<(), ...>) {}

fn bar() {
  foo(());
}

This is so... arbitrary. Why not also coerce () to Option<()> while we are at it, or to the empty array?

The table arguing that transitivity is not a problem also is very discouraging IMHO. Transitivity is not a problem now, but this is fragile and has to be re-evaluated with every new coercion that's added. The fact that we don't want transitivity here shows that coercions are the wrong tool. This is a hack.

[oli-obk]

The arguments I've seen against doing this remind me of the issues with if (foo); bar(); in C. Stray semicolons can quickly invalidate code, and are in some cased only caught by clippy or not at all in complex cases. The fact that there's even the discussion about it makes me very hesitant about fixing the problems with coercions. We should under no circumstances repeat mistakes from other languages which are prone to show up in beginner code and have occasionally resulted in security issues in software in practice. Especially for beginners, having lints like dead code doesn't help, because they are already overwhelmed by the amount of compiler messages

[DanielJin21]

I would like to posit that while this RFC does improve code writability, it would make error handling even harder to explain to new users coming to Rust. As such, my view very much echoes that of @nox.

I am also in agreement with @oli-obk that current error messages regarding error handling are suboptimal. However, I would argue that with such a change proposed by this RFC, there will be a just as significant number of new users confused about why their function works in the first place. Note that implicit coercion as a concept is explained much later in the book compared to error handling.

In any case, I would like to see the potential drawbacks noted by @est31, @mark-i-m, and @RalfJung to be added to the RFC.

[scottmcm]

I'm not convinced that Result::unwrap_err(()) is any less "nonsense" than ().unwrap_err().

And ? desugars to non-method form, so removing receiver coercion doesn't impact that.

[leonardo-m]

Generally I don't like to add special cases to the language.

[egilburg]

As a smaller step, does anyone have an opinion of allowing Ok() to be equivalent to Ok(())? This could be generic for anything, not just Ok. Similar to how functions can just omit return value instead of always specifying -> (), an "omitted" argument in an option could be equivalent to the unit.

[mark-i-m]

@egilburg I actually thought of something like that too, but it still feels like special magic for one use case.

Perhaps we can make it more general by saying "For enum variants with exactly one parameter ONLY: Variant will desugar to Variant(Default::default())"... I would be less opposed to that, even though it still feels a bit like a special case. That would enable stuff like this:

fn foo() -> Result<(), String> {
    if error {
        Err("Oh no")
    } else {
        Ok
    }
}

[mark-i-m]

Or we could just change conventions: Use Option instead of Result if there is no return value... Then you can just return None. However, this seems less intuitive than Ok, and would probably cause some amount of code churn, too...

[egilburg]

@mark-i-m Your example requires less typing, but it seems to conflate option definition with invocation, similar to how in Rust (unlike languages like Ruby) you cannot invoke a zero-argument function without parentheses. My example proposed similar to what were you saying except the parentheses, e.g., "Variant() will desugar to Variant(Default::default())"

[mark-i-m]

@egilburg Yes, I tried to avoid that. Notice, that I said for "enum variants ... ONLY", not function calls. I think something like this would be very bad for function calls because it obscures what that function call is doing. This doesn't seem to be as much of a problem with enums, though, IMHO.

Also, note that you can already create enums without parens if they have no args (e.g. None)...

[scottmcm]

I think Variant(Default::default()) is too extreme, since then you can use Ok() instead of, say, Ok(Vec::new()). But the thrust is interesting if restricted to "marker ZST"s: PhantomData jumps to mind. For a type like struct RefAndBit<'a, T>(usize, PhantomData<&'a T>);, the second argument on construction is useless, so it might be interesting to allow constructing it (privately) as just RefAndBit(ptr as usize).

[dns2utf8]

I am very against this RFC because, as stated before, it introduces magic sometimes.
Plus it reduced typesafety, as stated before.

I agree Ok(()) is not very readable. For that reason I use Ok( () ) in my code.
We could teach rustfmt to format it like that to make it more readable.

However, as stated in the API guidlines in the section about Error-Types:

... define a meaningful error type specific to your crate.

The same is true for return types.

Having userdefined functions return Result<(), E> pushes developers to programming with globally shared state.
That leads to the usual hardly testable and very closely coupled code.
Keeping it visible also keeps the language consistent.

[hadronized]

I agree Ok(()) is not very readable. For that reason I use Ok( () ) in my code.
We could teach rustfmt to format it like that to make it more readable.

I think we could still ask the OP to change the RFC and just present a syntactic sugar Ok () for that problem.

However, what I don’t like is that we try to patch the language by adding exceptions and syntactic sugar for a few types – this and all similar RFCs.

Still, Ok () or Ok() seems sane to me, as it’s often the case in other constructs of the language that “emptiness” is akin to unit (()), like in the following:

match a {
  Foo => do_something("foo"),
  Bar => {}
  Zoo => ()
}

Both the two latest branches yields the same () expression. Hence, why not adding that to Ok(x) where x = (), and end up with Ok() – or Ok ()?

[hadronized]

Or, maybe even easier:

fn ok<E>() -> Result<(), E> {
  Ok(())
}

And use it with ok().

[kennytm]

@phaazon Given that the reason both this and RFC 2107 are postponed to give way to the impl-Period, I don't think any RFC in this area will be accepted before 2018.

(Also I'm strongly against using Ok()/Ok/OK/:ok:/:ok_hand: as a substitute for Ok(()), especially after rust-lang/rust#44059 lands, so please find someone else to write that RFC.)

[zackw]

I do abstractly want to not have to write Ok(()) at the end of fallible functions called for their side effects, but regarding this RFC I'd like to second all of the maintenance / debuggability concerns raised above. I find this example terrifying:

the following will unexpectedly always return Ok(()):

fn typo() -> Result<(), E> {
   if cond {
        Ok(())
    } else {
        Err(E::OhNo);  // <- typo
    }
}

In a complicated function, that has all the hallmarks of being the kind of one-character bug that takes days of debugging to find: no diagnostic at compile time, silently swallows error condition at runtime, and the first umpteen times you read the problem code you don't notice the semicolon.

This could be addressed with lints — in the compiler, not in clippy! — but I would want to see the lints specified in the RFC, with thought given to false positives and negatives. It may be necessary to write a function with signature fn (...) -> Result<(), _> that always returns Ok(()) in order to satisfy some external interface, for instance.

I think someone said upthread that the EmptyTailExpr alternative does not have this problem, and I might even see why (without a coercion, the arms of the if expression don't have the same result type?) but I'd like to hear someone confirm that.

[kennytm]

@zackw I think EmptyTailExpr still have the same problem. #2107 and throws both won't have this problem, since you are not supposed to return an Err at all, you must throw it.

[ghost]

@zackw

I don't find that code terrifying - it doesn't even compile:

mismatched types: expected (), found enum `std::result::Result`

If you remove the first branch (if cond { Err(E::OhNo); }), it still won't compile:

type annotations needed: cannot infer type for `T`

Now if you add type annotations (Err::<(), E>(E::OhNo)), you still get a warning:

unused `std::result::Result` which must be used

Do you perhaps have a more convincing example?

[zackw]

@stjepang I think you misunderstand: that example doesn't compile right now, but it would with the changes proposed in this RFC, and that's what I'm objecting to. (It is quoted verbatim from the RFC, see under "drawbacks.")

[stevenblenkinsop]

@zackw

It wouldn't compile with the proposed changes because (1) the compiler couldn't infer the type of Err(E::OhNo) (the Ok type is unconstrained), and (2) there's already a lint disallowing unused Result values.

[zackw]

@stevenblenkinsop I see, but that doesn't give me enough confidence that, with the proposed changes, something like this can never sneak by silently. The RFC author thought that this example would compile under their proposed changes, after all. I'm asking them to make a case, in the text of the RFC, that this kind of typo cannot ever sneak by the compiler with the existing set of lints, or else propose new lints to fill the gaps.

[dns2utf8]

What is bugging me the most is that it makes the code unclear.
When I read my own code after 6 months and I use "clever tricks" it takes me way longer to understand what I was thinking back then.
So checking my own code for constraints I aim for maximum clarity.

Speaking of code I do work on professionally, I have to assume the next person reading the code has no time at hand and since the project already throws some warnings one more will not make a difference.
I think @zackw is right and the code would compile: Demo with stable code

[est31]

@dns2utf8 I'd like to point out that your linked example fires the "unused result which must be used" lint.

[scottmcm]

the compiler couldn't infer the type of Err(E::OhNo) (the Ok type is unconstrained)

There've been discussions about having unconstrained type parameters in enum variant constructors default to !. That would let println!("{:?}", Ok(3)) compile, the same way the 3 defaults to i32.

[mark-i-m]

the compiler couldn't infer the type of Err(E::OhNo) (the Ok type is unconstrained)

Also, that's a fantastically unintuitive way to say "you have an extra semicolon, and are discarding an error"...

[burdges]

I find that example terrifying too @zackw

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[dns2utf8]

@est31 true and as I stated, while developing code there are usually many warnings floating around.
So the scenario @zackw drew is in my opinion very realistic even if it drops an error.
Searching for such a bug will be very hard.

Second I like rust because there are no exceptions for certain types in the language.
This simplicity means in this case writing 6 more Bytes and have a consistent set of rules that is explainable to beginners.
Plus the code stays readable, which is especially handy when working on a project with four or more languages in it.

[canndrew]

Why don't we just allow the entire function body to be a catch block?

fn foo(x: bool) -> Result<(), u32> catch {
    if x {
        Err(123)?;
    }
}

Edit: Oh, I just learned that catch statements still require you to use Ok inside them. That's... weird.

[scottmcm]

@canndrew some discussion of things more along those lines in #2107 (comment) & follow-ups.

[rfcbot]

The final comment period is now complete.

[aturon]

This RFC is being closed as postponed. While the lang team is very interested in pursuing improvements to Result-oriented programming along these lines, more design work and experimentation is going to be needed, so we're punting until after the impl period.

Thanks @kennytm!