Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error messages for missing return values #25228

Closed
yggie opened this issue May 8, 2015 · 6 comments
Closed

Better error messages for missing return values #25228

yggie opened this issue May 8, 2015 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-diagnostics Working group: Diagnostics

Comments

@yggie
Copy link

yggie commented May 8, 2015

I recently ran into an error when writing a Rust program, it was a simple mistake but the compiler warnings were very misleading. I started out with a program much like:

fn foo(bar: usize) -> usize {
    if bar % 5 == 0 {
        return 1;
    }
}

fn main() {
    println!("Hello, {}!", foo(1))
}

It is obvious here that the function foo does not return values for all control paths. The compiler on the other hand, returns something like this:

<anon>:2:5: 4:6 error: mismatched types:
 expected `usize`,
    found `()`
(expected usize,
    found ()) [E0308]
<anon>:2     if bar % 5 == 0 {
<anon>:3         return 1;
<anon>:4     }

Not being a veteran in deciphering the rust compiler messages, it took me a good deal of time to figure out what the real problem was. I think the issue here is that the compiler thinks that the return value is the result of the if expression, which is (). Adding a semicolon after the brace gives a much better error message:

<anon>:1:1: 5:2 error: not all control paths return a value [E0269]
<anon>:1 fn foo(bar: usize) -> usize {
<anon>:2     if bar % 5 == 0 {
<anon>:3         return 1;
<anon>:4     };
<anon>:5 }

In my opinion, this message is much more helpful. I don’t think this scenario is uncommon, and for those like myself who are unfamiliar with the language, it would be a real pain to have to deal with these misleading compiler messages on top of the learning the language. Hopefully this is a quick fix, and it can get in before the official 1.0 release.

PS: I have tested this with for loops and a similar problem occurs.

@steveklabnik
Copy link
Member

It is obvious here that the function foo does not return values for all control paths.

It does, actually. if is an expression, and if you don't have an else, always evaluates to (). That's why you get this error, as you've noticed.

I personally think this is too much of a special case for an error, but someone else might feel differently.

@steveklabnik steveklabnik added the A-diagnostics Area: Messages for errors, warnings, and lints label May 10, 2015
@ghost
Copy link

ghost commented May 19, 2015

In addition to what @steveklabnik said, I should add we already have a special diagnostic for if expressions with no else clause:

fn main() {
    if true {
        0
    } else if false {
        1
    };
}
<anon>:4:12: 6:6 error: if may be missing an else clause:
 expected `()`,
    found `_`
(expected (),
    found integral variable) [E0308]
<anon>:4     } else if false {
<anon>:5         1
<anon>:6     };
error: aborting due to previous error
playpen: application terminated with error code 101

But it doesn't kick in if you drop the trailing ;. That's probably what we should fix here…

@steveklabnik
Copy link
Member

Triage: no change

@steveklabnik steveklabnik added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2017
@Mark-Simulacrum
Copy link
Member

With a single if, we now state that it might be missing an else clause. However, other control-flow statements don't have any help regarding a missing return statement, as shown by the second error below. I think the best thing we can do here is probably to add a note that not all control flow paths return usize (since that's the return type annotated on the function definition), and possibly point a span at the first "exit point" if possible.

error[E0317]: if may be missing an else clause
 --> test.rs:2:5
  |
2 | /     if bar % 5 == 0 {
3 | |         return 1;
4 | |     }
  | |_____^ expected (), found usize
  |
  = note: expected type `()`
             found type `usize`

error: aborting due to previous error(s)
error[E0308]: mismatched types
 --> test.rs:2:5
  |
2 | /     for x in &[0, 10] {
3 | |         return 1;
4 | |     }
  | |_____^ expected usize, found ()
  |
  = note: expected type `usize`
             found type `()`

error: aborting due to previous error(s)

@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 22, 2017
@estebank estebank added E-needs-mentor WG-diagnostics Working group: Diagnostics labels Nov 29, 2017
@estebank
Copy link
Contributor

Current output for the last case presented:

error[E0308]: mismatched types
 --> src/main.rs:2:5
  |
1 |   fn foo(bar: usize) -> usize {
  |                         ----- expected `usize` because of return type
2 | /     for i in 0..1 {
3 | |         return 1;
4 | |     }
  | |_____^ expected usize, found ()
  |
  = note: expected type `usize`
             found type `()`

@estebank
Copy link
Contributor

estebank commented Mar 7, 2019

#58981 will provide the similar output for the original case as we already do for the prior comment:

error[E0317]: if may be missing an else clause
  --> $DIR/if-without-else-as-fn-expr.rs:2:5
   |
LL |   fn foo(bar: usize) -> usize {
   |                         ----- found `usize` because of this return type
LL | /     if bar % 5 == 0 {
LL | |         return 3;
LL | |     }
   | |_____^ expected (), found usize
   |
   = note: expected type `()`
              found type `usize`
   = note: `if` expressions without `else` must evaluate to `()`

bors added a commit that referenced this issue Mar 21, 2019
Point at coercion reason for `if` expressions without else clause if caused by return type

```
error[E0317]: if may be missing an else clause
  --> $DIR/if-without-else-as-fn-expr.rs:2:5
   |
LL |   fn foo(bar: usize) -> usize {
   |                         ----- found `usize` because of this return type
LL | /     if bar % 5 == 0 {
LL | |         return 3;
LL | |     }
   | |_____^ expected (), found usize
   |
   = note: expected type `()`
              found type `usize`
   = note: `if` expressions without `else` must evaluate to `()`
```

Fix #25228.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-diagnostics Working group: Diagnostics
Projects
None yet
Development

No branches or pull requests

4 participants