Handle return type properly

[richardpringle]

Motivation

Closes #6306

Solution

Explicitly state the return type of the last statement such that the compiler gives the proper error if the types do not match.

I'm looking for some initial feedback. As I'm writing this description, I'm realizing that I could probably wrap the entire body instead of just the last statement.

The error now looks like this:

error[E0308]: mismatched types
 --> src/main.rs:2:20
  |
2 | async fn main() -> Result<(), ()> {
  |          ----      ^^^^^^^^^^^^^^ expected `Result<(), ()>`, found `()`
  |          |
  |          implicitly returns `()` as its body has no tail or `return` expression
3 |     Ok(())?;
4 |     Ok(());
  |           - help: remove this semicolon to return this value
  |
  = note:   expected enum `Result<(), ()>`
          found unit type `()`

For more information about this error, try `rustc --explain E0308`.
error: could not compile `a-thing` (bin "a-thing") due to 1 previous error

[richardpringle]

🤔 can already see it doesn't work for !, any ideas?
nvm

[richardpringle]

Will have to rename this

[richardpringle]

I'm using this enum to find a possible never type, but it might be tough to make this exhaustive in a forward-compatible way. There's panic!, std::process::exit, loop, and core::panicking::panic_explicit() that I know of, but there may be others.

That doesn't include any user-defined functions that return !.

I was getting compiler warnings if I output something like:

let main_body: Result<(), Error> = loop {
    some_fn().await?;
 };

(because the type is obviously wrong)

So I'm not sure that this approach really makes sense...

[richardpringle]

This is definitely super wasteful to parse the entire syntax tree until hitting a loop.
Visit has 182 methods and I could add skip! to all of them if *self == Found but that's a lof of nothing code just to fix an error message

[richardpringle]

I think I can actually remove this branch

[richardpringle]

I can fix the let...else after someone take a look at this (CI / minrust failure)

[taiki-e]

Thanks for the PR. This seems to be an approach I said in #3039 (comment)? If so you have to handle at least never type and impl traits. See also #5619 (comment).

[taiki-e]

Why does this need to search the input.stmts? Wouldn't it work by just checking the return value?

[richardpringle]

When you say the "return value," do you mean the last statement? I guess any statement that contained a loop that doesn't exit would have to be the last statement otherwise any statements that follow would be dead code.

So that would be an improvement, but that doesn't change the fact that using visit could potentially be expensive (with very little gained).

[taiki-e]

I think you can use matches!(&**ret_type, syn::Type::Never(..)) instead of ==.

[taiki-e]

Both extra-traits and visit are not very good from a view of compile-time...

[richardpringle]

Haha, yup.

[richardpringle]

Do you think this is doable without these though? I'm asking more about visit, there are workarounds for the PartialEq implementation that I'm using from extra-traits.

[Darksonn]

As discussed on #6306 (comment), I will close this since the path forward is unclear. Still, thank you for taking the time to submit a PR.

[richardpringle]

Hey, thanks for taking the time to review @Darksonn!

All good, I'm sure someone will figure it out at some point.