Rust: Diagnostic for comparison error in if-let has wrong span

[crumblingstatue]

fn call(arg: String) {}

fn foo() {
    let opt: Option<String>;
    let string = String::new();
    if let Some(s) = opt {
        s == &string;
    }
    let ill_just_pass_this_string_here: String;
    call(ill_just_pass_this_string_here);
}

can't compare `std::string::String` with `&std::string::String`
  --> src/lib.rs:10:10
   |
10 |     call(ill_just_pass_this_string_here);
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `std::string::String == &std::string::String`

This is reproducible with at least rustc 1.39 up to latest nightly.

[hellow554]

Excuse me? I stared this for way too long to figure out what's going on.

fn call(_: String) {}

fn foo() {
    if let Some(s) = Option::<String>::None {
        s == &String::new();
    }
    let local_var: String;
    call(local_var);
}

This only works with a variable which has a type specified (in this case local_var). If you leave out String, the correct span will be displayed.

Also slightly different code will produce a correct span.

[jumbatm]

If you fix up the comparison error

fn call(_: String) {}

fn main() {
    if let Some(s) = Option::<String>::None {
        s == String::new();
    }
    let local_var: String;
    call(local_var);
}

a possibly-uninitialised error is issued for the call to local_var, on the incorrect span which we've been observing.

If you go back to hellow's example and this time initialise local_var, but leave the comparison incorrect:

fn call(_: String) {}

fn main() {
    if let Some(s) = Option::<String>::None {
        s == &String::new();
    }
    let local_var: String = String::new();
    call(local_var);
}

you'll get the correct span.

It appears to me that the comparison error is being issued with the maybe-uninitialised variable error's span. I'll do a bit more digging and see if I can find if or why this is the case.

[jumbatm]

Nice - It looks like this has already been fixed :+1:

[estebank]

Closing based on the last comment.

[hellow554]

@estebank do we have a ui test?

[estebank]

@hellow554 I know I saw a PR go by that fixed this and other similar issues, and that one included tests for at least one case.