Error message "mismatched types" has wrong types

[akazantsev]

UPDATE: If you are interested in fixing this bug, there are mentoring instructions in this comment below.

---

fn main() {
    let x = plus_one(5);
    println!("X = {}", x);
}

fn plus_one(x: i32) -> i32 {
    x + 1;
}

  |
6 |   fn plus_one(x: i32) -> i32 {
  |  ____________________________^ starting here...
7 | |     x + 1;
8 | | }
  | |_^ ...ending here: expected (), found i32
  |
  = note: expected type `()`
             found type `i32`

I think that types in "expected type () found type i32" needs to be exchanged

$ rustc --version
rustc 1.18.0-nightly (452bf0852 2017-04-19)

[estebank]

I believe this was introduced by #40224, specifically this line. Swapping the items in that tuple reflects the expected output:

error[E0308]: mismatched types
  --> file.rs:11:28
   |
11 |   fn plus_one(x: i32) -> i32 {
   |  ____________________________^
12 | |     x + 1;
13 | | }
   | |_^ expected (), found i32
   |
   = note: expected type `i32`
              found type `()`

CC @nikomatsakis

[nikomatsakis]

Hmm, yes. Well, you certainly can't change just that one line, because it is correct in most other cases. The flawed assumption here is that the "forced unit" case means that () should always be the "expected" type. I guess we'll have to thread a parameter through. This seems like a good mentoring issue, so I'm going to write up some instructions:

@estebank correctly identified the problem here. The coercion code includes a function called coerce_force_unit() covering cases where the rust syntax itself supplies a type of (); examples include return; (no argument expression; only legal if the return type is unit); {foo;} (no tail expression); the type of a while loop; or break; (no argument expression). I incorrectly assumed we would always want to label the () type in such cases as the "expected type", but this example provides a counterexample.

To account for this, I think we need to add a parameter to coerce_force_unit(), defined here. I would called it something like label_unit_as_expected: bool. This will then have to be passed to coerce_inner(), which can have a new argument like label_expression_as_expected: bool. coerce_inner() is also called from coerce(); in that case, I think we can just pass false for this argument.

Naturally, we will need to change the callers to coerce_forced_unit to add the new argument. Most of them can use true (and thus keep the current behavior), but this one call is where we type-check blocks. In that case, we can pass false, I think. It'd probably be good to add a comment citing this issue, something like:

// #41425 -- label the implicit `()` as being the "found type" here, rather than the "expected type".

Meanwhile, when an error occurs, we will not check if expression.is_none(), but rather we'll check for label_expression_as_expected being true. This comment and assertion can be removed since they no longer seem appropriate.

After that, we just want to add a ui test for this case to prevent it from regressing again. I'd call it something like src/test/ui/coercion-missing-tail-expected-type.rs and include a comment referencing this issue number.

Tagged as E-mentor. If you're interested, please leave a comment to let others know you are working on it! Feel free to ask questions here or to find me on IRC (nmatsakis) or Gitter (@nikomatsakis).

[alexeyzab]

Hi there! I'd like to try implementing this.

[nikomatsakis]

@alexeyzab great! let me know how it goes =)

[alexeyzab]

@nikomatsakis I've added the changes you mentioned, seems to work correctly. Let me know if I should change anything in my PR. Thanks for explaining everything! :)

[shioju]

Looks like this issue has been resolved. Can someone close this please, so that people looking for issues to work on can skip this. Thanks.