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

Incorrect help text when if and else differ by reference #82361

Closed
isaacg1 opened this issue Feb 21, 2021 · 1 comment · Fixed by #82364
Closed

Incorrect help text when if and else differ by reference #82361

isaacg1 opened this issue Feb 21, 2021 · 1 comment · Fixed by #82364
Assignees
Labels
A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@isaacg1
Copy link

isaacg1 commented Feb 21, 2021

I tried this code:

fn main() {
    if true {
        1
    } else {
        &1
    };
}  

I expected to see this happen: I expected that the help text would give syntactically correct code, or that there would be no help text.

Instead, this happened:

help: consider dereferencing the borrow
  |
4 |     } else *{
5 |         &1
6 |     };
  |

The provided code snippet is not valid - it tries to deference the else-block, which is impossible. Better would be to put the * inside the else-block.

Meta

rustc --version --verbose:

rustc 1.52.0-nightly (83b30a639 2021-02-20)
binary: rustc
commit-hash: 83b30a639d5abd1270ade35d9bd92271f5a5ba18
commit-date: 2021-02-20
host: x86_64-unknown-linux-gnu
release: 1.52.0-nightly
LLVM version: 11.0.1

Also occurs on stable, 1.50.0.

@isaacg1 isaacg1 added the C-bug Category: This is a bug. label Feb 21, 2021
@osa1
Copy link
Contributor

osa1 commented Feb 21, 2021

@rustbot claim

I have a fix for this bug + an improvement. Gotta got for now but will submit the PR later today.

@jonas-schievink jonas-schievink added A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels Feb 21, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 24, 2021
Improve error msgs when found type is deref of expected

This improves help messages in two cases:

- When expected type is `T` and found type is `&T`, we now look through blocks
  and suggest dereferencing the expression of the block, rather than the whole
  block.

- In the above case, if the expression is an `&`, we not suggest removing the
  `&` instead of adding `*`.

Both of these are demonstrated in the regression test. Before this patch the
first error in the test would be:

    error[E0308]: `if` and `else` have incompatible types
     --> test.rs:8:9
      |
    5 | /     if true {
    6 | |         a
      | |         - expected because of this
    7 | |     } else {
    8 | |         b
      | |         ^ expected `usize`, found `&usize`
    9 | |     };
      | |_____- `if` and `else` have incompatible types
      |
    help: consider dereferencing the borrow
      |
    7 |     } else *{
    8 |         b
    9 |     };
      |

Now:

    error[E0308]: `if` and `else` have incompatible types
     --> test.rs:8:9
      |
    5 | /     if true {
    6 | |         a
      | |         - expected because of this
    7 | |     } else {
    8 | |         b
      | |         ^
      | |         |
      | |         expected `usize`, found `&usize`
      | |         help: consider dereferencing the borrow: `*b`
    9 | |     };
      | |_____- `if` and `else` have incompatible types

The second error:

    error[E0308]: `if` and `else` have incompatible types
      --> test.rs:14:9
       |
    11 | /     if true {
    12 | |         1
       | |         - expected because of this
    13 | |     } else {
    14 | |         &1
       | |         ^^ expected integer, found `&{integer}`
    15 | |     };
       | |_____- `if` and `else` have incompatible types
       |
    help: consider dereferencing the borrow
       |
    13 |     } else *{
    14 |         &1
    15 |     };
       |

now:

    error[E0308]: `if` and `else` have incompatible types
      --> test.rs:14:9
       |
    11 | /     if true {
    12 | |         1
       | |         - expected because of this
    13 | |     } else {
    14 | |         &1
       | |         ^-
       | |         ||
       | |         |help: consider removing the `&`: `1`
       | |         expected integer, found `&{integer}`
    15 | |     };
       | |_____- `if` and `else` have incompatible types

Fixes rust-lang#82361

---

r? `@estebank`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 24, 2021
Improve error msgs when found type is deref of expected

This improves help messages in two cases:

- When expected type is `T` and found type is `&T`, we now look through blocks
  and suggest dereferencing the expression of the block, rather than the whole
  block.

- In the above case, if the expression is an `&`, we not suggest removing the
  `&` instead of adding `*`.

Both of these are demonstrated in the regression test. Before this patch the
first error in the test would be:

    error[E0308]: `if` and `else` have incompatible types
     --> test.rs:8:9
      |
    5 | /     if true {
    6 | |         a
      | |         - expected because of this
    7 | |     } else {
    8 | |         b
      | |         ^ expected `usize`, found `&usize`
    9 | |     };
      | |_____- `if` and `else` have incompatible types
      |
    help: consider dereferencing the borrow
      |
    7 |     } else *{
    8 |         b
    9 |     };
      |

Now:

    error[E0308]: `if` and `else` have incompatible types
     --> test.rs:8:9
      |
    5 | /     if true {
    6 | |         a
      | |         - expected because of this
    7 | |     } else {
    8 | |         b
      | |         ^
      | |         |
      | |         expected `usize`, found `&usize`
      | |         help: consider dereferencing the borrow: `*b`
    9 | |     };
      | |_____- `if` and `else` have incompatible types

The second error:

    error[E0308]: `if` and `else` have incompatible types
      --> test.rs:14:9
       |
    11 | /     if true {
    12 | |         1
       | |         - expected because of this
    13 | |     } else {
    14 | |         &1
       | |         ^^ expected integer, found `&{integer}`
    15 | |     };
       | |_____- `if` and `else` have incompatible types
       |
    help: consider dereferencing the borrow
       |
    13 |     } else *{
    14 |         &1
    15 |     };
       |

now:

    error[E0308]: `if` and `else` have incompatible types
      --> test.rs:14:9
       |
    11 | /     if true {
    12 | |         1
       | |         - expected because of this
    13 | |     } else {
    14 | |         &1
       | |         ^-
       | |         ||
       | |         |help: consider removing the `&`: `1`
       | |         expected integer, found `&{integer}`
    15 | |     };
       | |_____- `if` and `else` have incompatible types

Fixes rust-lang#82361

---

r? ``@estebank``
@estebank estebank added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 24, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 24, 2021
Improve error msgs when found type is deref of expected

This improves help messages in two cases:

- When expected type is `T` and found type is `&T`, we now look through blocks
  and suggest dereferencing the expression of the block, rather than the whole
  block.

- In the above case, if the expression is an `&`, we not suggest removing the
  `&` instead of adding `*`.

Both of these are demonstrated in the regression test. Before this patch the
first error in the test would be:

    error[E0308]: `if` and `else` have incompatible types
     --> test.rs:8:9
      |
    5 | /     if true {
    6 | |         a
      | |         - expected because of this
    7 | |     } else {
    8 | |         b
      | |         ^ expected `usize`, found `&usize`
    9 | |     };
      | |_____- `if` and `else` have incompatible types
      |
    help: consider dereferencing the borrow
      |
    7 |     } else *{
    8 |         b
    9 |     };
      |

Now:

    error[E0308]: `if` and `else` have incompatible types
     --> test.rs:8:9
      |
    5 | /     if true {
    6 | |         a
      | |         - expected because of this
    7 | |     } else {
    8 | |         b
      | |         ^
      | |         |
      | |         expected `usize`, found `&usize`
      | |         help: consider dereferencing the borrow: `*b`
    9 | |     };
      | |_____- `if` and `else` have incompatible types

The second error:

    error[E0308]: `if` and `else` have incompatible types
      --> test.rs:14:9
       |
    11 | /     if true {
    12 | |         1
       | |         - expected because of this
    13 | |     } else {
    14 | |         &1
       | |         ^^ expected integer, found `&{integer}`
    15 | |     };
       | |_____- `if` and `else` have incompatible types
       |
    help: consider dereferencing the borrow
       |
    13 |     } else *{
    14 |         &1
    15 |     };
       |

now:

    error[E0308]: `if` and `else` have incompatible types
      --> test.rs:14:9
       |
    11 | /     if true {
    12 | |         1
       | |         - expected because of this
    13 | |     } else {
    14 | |         &1
       | |         ^-
       | |         ||
       | |         |help: consider removing the `&`: `1`
       | |         expected integer, found `&{integer}`
    15 | |     };
       | |_____- `if` and `else` have incompatible types

Fixes rust-lang#82361

---

r? ```@estebank```
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 25, 2021
Improve error msgs when found type is deref of expected

This improves help messages in two cases:

- When expected type is `T` and found type is `&T`, we now look through blocks
  and suggest dereferencing the expression of the block, rather than the whole
  block.

- In the above case, if the expression is an `&`, we not suggest removing the
  `&` instead of adding `*`.

Both of these are demonstrated in the regression test. Before this patch the
first error in the test would be:

    error[E0308]: `if` and `else` have incompatible types
     --> test.rs:8:9
      |
    5 | /     if true {
    6 | |         a
      | |         - expected because of this
    7 | |     } else {
    8 | |         b
      | |         ^ expected `usize`, found `&usize`
    9 | |     };
      | |_____- `if` and `else` have incompatible types
      |
    help: consider dereferencing the borrow
      |
    7 |     } else *{
    8 |         b
    9 |     };
      |

Now:

    error[E0308]: `if` and `else` have incompatible types
     --> test.rs:8:9
      |
    5 | /     if true {
    6 | |         a
      | |         - expected because of this
    7 | |     } else {
    8 | |         b
      | |         ^
      | |         |
      | |         expected `usize`, found `&usize`
      | |         help: consider dereferencing the borrow: `*b`
    9 | |     };
      | |_____- `if` and `else` have incompatible types

The second error:

    error[E0308]: `if` and `else` have incompatible types
      --> test.rs:14:9
       |
    11 | /     if true {
    12 | |         1
       | |         - expected because of this
    13 | |     } else {
    14 | |         &1
       | |         ^^ expected integer, found `&{integer}`
    15 | |     };
       | |_____- `if` and `else` have incompatible types
       |
    help: consider dereferencing the borrow
       |
    13 |     } else *{
    14 |         &1
    15 |     };
       |

now:

    error[E0308]: `if` and `else` have incompatible types
      --> test.rs:14:9
       |
    11 | /     if true {
    12 | |         1
       | |         - expected because of this
    13 | |     } else {
    14 | |         &1
       | |         ^-
       | |         ||
       | |         |help: consider removing the `&`: `1`
       | |         expected integer, found `&{integer}`
    15 | |     };
       | |_____- `if` and `else` have incompatible types

Fixes rust-lang#82361

---

r? ````@estebank````
@bors bors closed this as completed in fa74d48 Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants