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

Labeled line for "here the type of var is inferred to be" can be wrong #106590

Closed
compiler-errors opened this issue Jan 8, 2023 · 6 comments
Closed
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@compiler-errors
Copy link
Member

Given the following code: playground

struct Wrap<T, S>(T, S);

fn main() {
    let s = Wrap(None, None);
    s.0 = Some(1);
    s.1 = Some(2);
    let z: Wrap<Option<i32>, Option<&str>> = s;
}

The current output is:

error[E0308]: mismatched types
 --> src/main.rs:7:46
  |
5 |     s.0 = Some(1);
  |     - here the type of `s` is inferred to be `Wrap<Option<{integer}>, Option<_>>`
6 |     s.1 = Some(2);
7 |     let z: Wrap<Option<i32>, Option<&str>> = s;
  |            -------------------------------   ^ expected `&str`, found integer
  |            |
  |            expected due to this
  |
  = note: expected struct `Wrap<Option<i32>, Option<&str>>`
             found struct `Wrap<Option<{integer}>, Option<{integer}>>`

Ideally the output should look like:

error[E0308]: mismatched types
 --> src/main.rs:7:46
  |
5 |     s.0 = Some(1);
6 |     s.1 = Some(2);
  |     - here the type of `s` is inferred to be `Wrap<Option<{integer}>, Option<{integer}>>`
7 |     let z: Wrap<Option<i32>, Option<&str>> = s;
  |            -------------------------------   ^ expected `&str`, found integer
  |            |
  |            expected due to this
  |
  = note: expected struct `Wrap<Option<i32>, Option<&str>>`
             found struct `Wrap<Option<{integer}>, Option<{integer}>>`

The problem here is that while s.0 = .. is the first expression to constrain Wrap<_, _>, it's not the expression which constrains Wrap<_, _> in such a way that causes the type error, so pointing to it is possibly misleading -- possibly even more confusing if the lines are further separated.

@compiler-errors compiler-errors added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 8, 2023
@compiler-errors
Copy link
Member Author

Similarly confusing output:

use std::collections::HashMap;

fn main() {
    let mut x = HashMap::<u32, _>::new();
    x.insert(1i32, Default::default());
}
error[[E0308]](https://doc.rust-lang.org/nightly/error-index.html#E0308): mismatched types
 --> src/main.rs:5:14
  |
5 |     x.insert(1i32, Default::default());
  |       ------ ^^^^  ------------------ this is of type `_`, which causes `x` to be inferred as `HashMap<u32, _>`
  |       |      |
  |       |      expected `u32`, found `i32`
  |       |      this is of type `i32`, which causes `x` to be inferred as `HashMap<u32, _>`
  |       arguments to this method are incorrect
  |
note: associated function defined here
 --> /rustc/ee0412d1ef81efcfabe7f66cd21476ca85d618b1/library/std/src/collections/hash/map.rs:1103:12
help: change the type of the numeric literal from `i32` to `u32`
  |
5 |     x.insert(1u32, Default::default());
  |               ~~~

Specifically:

this is of type i32, which causes x to be inferred as HashMap<u32, _>

@compiler-errors compiler-errors added the D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. label Jan 8, 2023
@estebank
Copy link
Contributor

estebank commented Jan 8, 2023

For the second case it can be guarded against by breaking on an earlier iteration of the suggestion logic. For the original we'll need to add logic to handle field access explicitly.

@compiler-errors
Copy link
Member Author

compiler-errors commented Jan 8, 2023

I don't think the original issue is fixed by adding logic to handle field accesses explicitly.

There are a number of other ways of using an expression that can constrain it partially, for example:

#[derive(Copy, Clone)]
struct Wrap<T, S>(T, S);

fn main() {
    let s = Wrap(None, None);
    constrain::<i32, _>(s);
    constrain::<_, i32>(s);
    let z: Wrap<Option<i32>, Option<&str>> = s;
}

fn constrain<T, S>(_: Wrap<Option<T>, Option<S>>) {}

Results in:

error[[E0308]](https://doc.rust-lang.org/nightly/error-index.html#E0308): mismatched types
 --> src/main.rs:8:46
  |
6 |     constrain::<i32, _>(s);
  |                         - here the type of `s` is inferred to be `Wrap<Option<i32>, Option<_>>`
7 |     constrain::<_, i32>(s);
8 |     let z: Wrap<Option<i32>, Option<&str>> = s;
  |            -------------------------------   ^ expected `&str`, found `i32`
  |            |
  |            expected due to this
  |
  = note: expected struct `Wrap<_, Option<&str>>`
             found struct `Wrap<_, Option<i32>>`

For more information about this error, try `rustc --explain E0308`.

The issue here is that the heuristic is just "did the type of the expression change at all" -- not "did the type of the expression change in a way to that causes the type error down the road".

@estebank
Copy link
Contributor

estebank commented Jan 8, 2023

The issue here is that the heuristic is just "did the type of the expression change at all" -- not "did the type of the expression change in a way to that causes the type error down the road".

You're right, and that might be solved by evaluating that the current ty can still coerce to the original 🤔

@compiler-errors
Copy link
Member Author

compiler-errors commented Jan 8, 2023

by evaluating that the current ty can still coerce to the original

I actually implemented a change to do something similar, which fixes the examples (1) and (3) above, but the code is a bit too tangled up with the special casing for method calls and argument mismatches and it breaks the point-at-inference-3.rs UI test. :/

@estebank
Copy link
Contributor

estebank commented Jan 8, 2023

Shame. We could rework the logic for methods to be more consistent with the others, which might help :-/

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 23, 2023
…ush, r=estebank

Fix escaping inference var ICE in `point_at_expr_source_of_inferred_type`

Fixes rust-lang#107158

`point_at_expr_source_of_inferred_type` uses `lookup_probe` to adjust the self type of a method receiver -- but that method returns inference variables from inside a probe. That means that the ty vars are no longer valid, so we can't use any infcx methods on them.

Also, pass some extra span info to hack a quick solution to bad labels, resulting in this diagnostic improvement:

```rust
fn example2() {
    let mut x = vec![1];
    x.push("");
}
```

```diff
  error[E0308]: mismatched types
   --> src/main.rs:5:12
    |
  5 |     x.push("");
    |       ---- ^^
    |       |    |
    |       |    expected integer, found `&str`
-   |       |    this is of type `&'static str`, which causes `x` to be inferred as `Vec<{integer}>`
    |       arguments to this method are incorrect
```
(since that "which causes `x` to be inferred as `Vec<{integer}>` part is wrong)

r? `@estebank`

(we really should make this code better in general, cc rust-lang#106590, but that's a bit bigger issue that needs some more thinking about)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 25, 2023
…ush, r=estebank

Fix escaping inference var ICE in `point_at_expr_source_of_inferred_type`

Fixes rust-lang#107158

`point_at_expr_source_of_inferred_type` uses `lookup_probe` to adjust the self type of a method receiver -- but that method returns inference variables from inside a probe. That means that the ty vars are no longer valid, so we can't use any infcx methods on them.

Also, pass some extra span info to hack a quick solution to bad labels, resulting in this diagnostic improvement:

```rust
fn example2() {
    let mut x = vec![1];
    x.push("");
}
```

```diff
  error[E0308]: mismatched types
   --> src/main.rs:5:12
    |
  5 |     x.push("");
    |       ---- ^^
    |       |    |
    |       |    expected integer, found `&str`
-   |       |    this is of type `&'static str`, which causes `x` to be inferred as `Vec<{integer}>`
    |       arguments to this method are incorrect
```
(since that "which causes `x` to be inferred as `Vec<{integer}>` part is wrong)

r? ``@estebank``

(we really should make this code better in general, cc rust-lang#106590, but that's a bit bigger issue that needs some more thinking about)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 26, 2023
…ush, r=estebank

Fix escaping inference var ICE in `point_at_expr_source_of_inferred_type`

Fixes rust-lang#107158

`point_at_expr_source_of_inferred_type` uses `lookup_probe` to adjust the self type of a method receiver -- but that method returns inference variables from inside a probe. That means that the ty vars are no longer valid, so we can't use any infcx methods on them.

Also, pass some extra span info to hack a quick solution to bad labels, resulting in this diagnostic improvement:

```rust
fn example2() {
    let mut x = vec![1];
    x.push("");
}
```

```diff
  error[E0308]: mismatched types
   --> src/main.rs:5:12
    |
  5 |     x.push("");
    |       ---- ^^
    |       |    |
    |       |    expected integer, found `&str`
-   |       |    this is of type `&'static str`, which causes `x` to be inferred as `Vec<{integer}>`
    |       arguments to this method are incorrect
```
(since that "which causes `x` to be inferred as `Vec<{integer}>` part is wrong)

r? `@estebank`

(we really should make this code better in general, cc rust-lang#106590, but that's a bit bigger issue that needs some more thinking about)
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 D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants