Killing `Ok(())` without ok-wrapping

Coming from Haskell, my expectation of a _ expression would be that it'd be causing a compilation error (or it would be a todo!() equivalent) that gives you the inferred type of what's expected in that position as part of the error message (or as part of a warning in case of the todo!()-like behavior).

2 Likes

Which in particular?

The two places I can think of are in types to say "figure it out", which is sort of similar; and in place of bindings to discard a value: which is sort of the inverse of summoning a value from nothing, matching the opposite value direction.

The theme is "I don't care about this", I guess?

Theoretically, some other free symbol might be usable, but I somewhat doubt there's anything more worth burning on this, and at least default is common enough in usage having a very short spelling might be nice. (Eg Foo { bar, .._ } to default the other fields)

I'm not married to it.

Slightly surprisingly, Result doesn't implement Default, though...

I was referring to using _ as a shorthand for Default::default().

Using "anonymous destructuring" eg let _ { foo, ... } = <EXPR>; resembles the current usage enough that it wouldn't be that surprising I think.

And yeah _ can in current usage pretty much be interpreted that way eg the catch-all branch of a match, or let _ = <EXPR>; etc.

What would be a sane default though? Depending on the use case I think either variant could be reasonable.

Sure, which is why it's only slightly surprising, but I would argue we expect Default to be the "neutral" state, and to me Ok is clearly more neutral than Err, which on the common usage will short-circuit the function, instead of "doing nothing" like Ok.

Not saying it's a fault or problem other than that surprise, except in that it means returning default doesn't really help this issue! :grin:

1 Like

I think we should not make Ok(()) implicit, regardless of how it's accomplished.

Specifically, I think that implicit Ok(()) slightly worsens the language in-and-of-itself, even without considering the complexity cost for Rust learners and proc macro authors (of course, those costs deserve consideration as well!)

Here's my example:

fn notify_one_server(&self) -> Result<(), Error> {
  for server in self.list_servers()? {
    if server.notify().is_ok() {
      return Ok(())
    }
  }

  // Should say Err(NoServersAvailable), but whoops-a-daisy,
  // I seem to have forgotten to think about what code should go here!
  // In current Rust, a compile error will remind me to fix this.
  // With the proposed feature, it will silently misbehave.
}

The biggest time-and-effort cost for programmers is undetected mistakes. A trailing Ok(()) may usually look extraneous, but it has a semantic meaning, and protects Rust programmers from making mistakes like the above. I don't know if the above is "good code", but it's code that a normal programmer might write, with a mistake a normal programmer might make.

The only times that explicit things should be made implicit are when either:

  1. it would be extremely rare to make a serious mistake as a result of the implicitness, or
  2. the implicit version is so dramatically more concise that it has a significant impact on what code can be written at all, thus justifying the risk of mistakes.

Trailing Ok(()) is neither of those situations.

24 Likes

I personally feel like implicit Ok(()) feels more okay in function-return-positions than in arbitrary blocks.

I feel like

fn foo() -> Result<()> {
    do_something()?;
    something_else();
}

or

fn bar() -> Result<()> {}

significantly less weird than something like

let x: Result<()> = {};

I assume that this feeling of mine is related to why ok-wrapping in try blocks feels reasonable; combined with the fact that ordinary functions are already a lot like try {} blocks (in that they both support ? operators). Arguably, the difference to ok-wrapping from try blocks is that in the latter case, the wrapping applies to non-() values, too; functions cannot become the same in this regard (without additional opt-in syntax like try fn), because an explicit return value already has the meaning that itā€™s returned directly.


To flesh out what that would mean in detail: I think itā€™s important, that trailing blocks are supported, too, so

fn foo() -> Result<()> {
    do_something()?;
    if {
        something_else();
    } else {
        bar()?;
    }
}

would work, too. As should using argument-less return; statements / expressions.

Iā€™m not sure if something like this would be good to accept, too.

fn foo() -> Result<()> {
    do_something()?;
    if {
        something_else(); // <- semicolon, so implicit `Ok(())` return value is used
    } else {
        bar() // <- type Result, value is returned
    }
}

Arguably, the thing is (mostly) equivalent to using bar()?; like in the previous code example anyways, so it isnā€™t even all that useful to support it.

So if the heterogeneous kind of if expression like above is disallowed, then the rule could be: Implicit Ok(()) return will be inserted at the end of a function body, if

  • the function has no final/return expression, or
  • the function has a final/return expression, but thatā€™s an expression-with-block of () type, i.e. precisely the kind of expression that would allow a subsequent statement without a semicolon in-between
    • the rationale for this then is that the function does in this case conceptionally[1] still have no return expression, but the last statement is a statement that doesnā€™t require a semicolon.
  • Furthermore, argument-less return expressions are allowed to return Ok(()).
  • The same rules apply to closures whose body is a block, and to async blocks.
    • Closures whose body is an expression-with-block of () type also seem reasonable.

Reviewing these proposed rules, I see that type inference might be hard though, given that the function return type can be used in type inference for the return expression. Maybe it could make sense to be slightly more restrictive and require an expression-with-block that is clearly known to be of type (), otherwise type inference would probably need to happen twice, or need some new priority/defaulting mechanisms, or something like that.

An expression-with-block might be clearly known to be of type () e.g. by syntactical criteria alone, such as checking whether all contained blocks have not return value. For example

if ā€¦ { statement; /* no return value */ } else { statement; }

or

match ā€¦ { ā€¦ => { statement; } }

but not something like

match ā€¦ { ā€¦ => function_call() }

(even though function_call() might return ()).

To be precise, even with such a syntactical criterion, the expression might still be actually of type !. But then, type inference starting with the assumption that itā€™s () could still not fail nonetheless. And something like

match ā€¦ { ā€¦ => function_call(), ā€¦ => { statement; } }

would not count since if statement was !, then function_call() and thus the whole match could have any type.

Such syntactical criterion could also be a first approximation, and allowing more cases in the future would be possible, since undetected cases of type-() return values in type-Result functions would simply result in a type mismatch error. (The error message diagnostics then still do a heuristic / best-effort analysis to suggest appropriate semicolons, either after the whole expression, or inside of some contained block thatā€™s missing it, whenever such semicolons appear to be likely to help.)


Closures and async blocks have the additional difficulty that they might not have a known return type. A conservative approach would simply not allow anything new unless in cases where the return type is already known to be Result ā€œearly enoughā€ by type inference. It should work well enough so that macros such as tokio::main or async_trait can be defined in such a way that the implicit return values still work for code using those macros. (Iā€™m not saying that their current definitions would need to work. E.g. tokio::main seems to be using let let body = async {}; which is providing fairly little type information, though from the subsequent usage, there is some type information; Iā€™m not sure if that type information would be ā€œearly enoughā€ though, but changing the macro to put some more explicit type annotation migth still be possible). And async_trait currently is using something like async { let _ret = { ā€¦block contentsā€¦ }; _ret } which doesnā€™t have the block in return position, but I donā€™t know why they do it this way and I think it could be changed, too.


  1. ignoring the differences in drop order ā†©ļøŽ

2 Likes

This strongly resonates with me:

I like the readability of a fn() -> Result<(), E> where all the exit cases are visible by explicit Err(e), expr? or Ok(()), particularly when there is a long match involved as the return expression.

EDIT: Clarifying this feeling is for fns only. Since try fn is an opt-in, you can choose the auto-wrapping (?) as you wish.

5 Likes

I agree with almost every particular, but we do at least get a warning on the something_else() line if it needs it.

This post has convinced me that we should be very careful with implicit Ok(()) and try fn.

Having the rightwards drift of a try block is actually a decent visual aid to keep eyes alert for the implicit ok:

fn foo() -> Result<()> {
    try {
        do_something()?;
        something_else();
    }
}

This feels much nicer and consistent with existing language features than Ok-wrapping or try blocks would be. (Notably, it doesn't require special syntax for a single idiom.)

However, it is also kinda confusing that something that is not a destructor gets to run automatically at the end of a block (and, relatedly, it brings up issues with regards to the ordering of calls to this new trait method and drop().)

Overall I still think Ok(()) is not a problem that needs to be solved.

3 Likes

This is a very real example, too. An ongoing project of mine requires writing typed wrappers around database transactions, and the exact same problem occurs in commit()/rollback() functions that need to perform further post-processing or cleanup in addition to calling the underlying DB's commit/rollback mechanism.

If the post-processing or cleanup step fails, I want to signal that to the caller by returning an error, even though I may still want to attempt (and hopefully succeed) rolling back. With an implicit Ok at the end, I could forget returning the cleanup error to-be-propagated entirely, merely because the underlying rollback/commit succeeded (so no ?-bubbling took place).

This could completely compromise data integrity between the DB and the client, since the client would then mistakenly think that the commit/rollback succeeded and all is well, whereas from the PoV of the database, it is not the case.

1 Like

That's a very long list of complicated, confusing and error-prone rules to eliminate something as trivial as Ok(()). And the net result is a language which is less consistent, less readable, and has more magic to confuse newcomers.

2 Likes

How in the world this is more consistent with the language compared to try fn? To me the proposal looks like an ad hock hack for eliminating Ok(()), which is simply the most glaring example of a more general problem.

I do not understand why people find syntax like this inconsistent with the language:

try fn foo() -> Result<u32, Error> {
    let res = do_stuff()?;
    if res.check1() {
        // `yeet` here is a hypothetical return-like keyword
        // usable only inside `try fn`s and `try` blocks
        yeet Error::Check;
    }
    if res.check2() {
        return 24;
    }
    42
}

try fn bar() -> Result<Result<u32, ()>, Error> {
    do_stuff1()?;
    do_stuff2();
    Ok(42)
}

try fn baz(a: u32) -> Option<u32> {
    // ..
    42
}

In my opinion it integrates very naturally with the old Try trait.

Also note that we already have "special syntax" for the idiom in the form of ?, simply because of how common this idiom is.

You could implement it using a proc macro which would call separate functions on Ok and Error return variants. Something like this:

// `commit` and `rollback` accept `&T` and `&E` respectively
#[wrap(on_ok = commit, on_err = rollback)]
fn do_stuff(args: A) -> Result<T, E> {
    // body
}
// it gets desugared into

fn foo(args: A) -> Result<T, E> {
    let res = {
        // body
    };
    match &res {
        Ok(v) => commit(v),
        Err(e) => rollback(e),
    }
    res
}

I might be misreading things here, but are folks here talking past one another? I see some appear to talk about "implicit Ok(())" and others appearing to talk about "implicit Ok-wrapping." They are very different things.

This particular topic has also been discussed quite a bit in the past, and there is a lot of nuance to this. It would help to be very clear about what specifically you're discussing. It isn't even limited to those two things either. You might only want to do them in certain contexts (like "only in try blocks", instead of "any place where a Result it's expected").

5 Likes

In my understanding, "implicit Ok-wrapping" is a more general solution which implies "implicit Ok(())", so they are closely related to each other. In other words, introducing the former would resolve most of motivation for the latter.

The problem with try fn imo is that it is basically line noise. unlike async func error handling is ubiquitous and marking functions with the try keyword wouldn't add any real value to code.

IMO the real issue here is that we have a partial syntax solution (the ? operator) for error handling and extending it with try functions as in your examples would be major churn for little to no gain, also taking into account Rust's stability and backwards compat. guaranties.

At the very least I'd want more consistency with the async fn design by changing the return type to the inner type and sugar to remove the need to define Error types to make the churn worthwhile.

E.g:

fn foo() -> u32 
    yeets Error1, Error2 // compiler would generate the enum for me
{
    let res = do_stuff()?;
    if res.check1() {
        // `yeet` here is a hypothetical return-like keyword
        // usable only inside `try fn`s and `try` blocks
        yeet Error1;
    }
    if res.check2() {
        return 24;
    }
    42
}

where yeet is the placeholder for the error case as you have mentioned and yeets is a placeholder for an error specification.

1 Like

(I understand that we are starting to rehash the numerous previous discussions, so I will try to be brief)

It's the same value as with the ? operator. It reduces noise inside a function body by removing Ok and Err wraps which are often redundant for humans at the cost of 3 letters in a function signature. Eliminating Ok(()) is a nice bonus. Also it allows much easier migration of previously non-failing functions to failing ones, you simply slap try, add new yeet returns and ?s if needed, and that's it. While today you have to haunt all returns and wrap results with Ok.

Why would it result in churn? The old fns will continue to work without any changes, similarly to how you can use fns which return impl Future instead of async fn. Also if maintainers will decide to migrate their codebase to try fns, I think it can be done mostly automatically.

I think you want something like this: Pre-RFC: type-level sets Your example would look like this:

try fn foo() -> Result<u32, Error1 | Error2> { .. }

Making types imlicit in signatures only adds confusion and reduces flexibility. Even with async fns it causes issues around implicitly derived trait bounds for return type. For example, how would your proposal work with Option or other custom types implementing the Try trait?

UPD: I guess one can argue that try fn is inconsistent in the sense that it should not belong to the function signature, because unlike unsafe fn and async fn it only change how body of the function is compiled, not it's signature (i.e. try fn foo() -> Resut<T, E> { .. } is fully equivalent to fn foo() -> Resut<T, E> { .. }). So for some it could make more sense to use fn foo() -> Resut<T, E> try { .. } as a shorcut for fn foo() -> Resut<T, E> { try { .. } }.

I'll try to brief as well, but let me quickly clarify a few points. Mainly, I'm not against try blocks, in fact I think they are great! There's just no sufficient justification IMO to expand this to the function signature when we could simply allow the function body to be any kind of block (I feel like this is a good idea regardless).

fn foo() -> Result<i32, Error> 
try { // or async or ...
    let res = do_stuff()?;
    if res.check1() {
         yeet Error::Check;
    }
    42
} 

So yeah, your update is exactly how I feel.

Yep.

To be fair, that was more of a tongue in cheek suggestion - I'm not putting forth an RFC here. Ideally, I want a consistent & orthogonal syntax (unlike C++) and I would have liked to not have async fn or const fn either. Unfortunately, that ship has already sailed.

Out of curiosity, would you have preferred, eg, fn foo() async {}? I feel like they wouldn't work, an async body is slightly different semantics since it doesn't already include the arguments (probably the same with const for the same reason)