Skip to content

Commit

Permalink
Improve error msgs when found type is deref of expected
Browse files Browse the repository at this point in the history
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 #82361
  • Loading branch information
osa1 committed Feb 23, 2021
1 parent a4e595d commit fa74d48
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 4 deletions.
8 changes: 8 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1577,6 +1577,14 @@ impl Expr<'_> {
expr
}

pub fn peel_blocks(&self) -> &Self {
let mut expr = self;
while let ExprKind::Block(Block { expr: Some(inner), .. }, _) = &expr.kind {
expr = inner;
}
expr
}

pub fn can_have_side_effects(&self) -> bool {
match self.peel_drop_temps().kind {
ExprKind::Path(_) | ExprKind::Lit(_) => false,
Expand Down
28 changes: 24 additions & 4 deletions compiler/rustc_typeck/src/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,10 +616,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
_ if sp == expr.span && !is_macro => {
if let Some(steps) = self.deref_steps(checked_ty, expected) {
let expr = expr.peel_blocks();

if steps == 1 {
// For a suggestion to make sense, the type would need to be `Copy`.
if self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp) {
if let Ok(code) = sm.span_to_snippet(sp) {
if let hir::ExprKind::AddrOf(_, mutbl, inner) = expr.kind {
// If the expression has `&`, removing it would fix the error
let prefix_span = expr.span.with_hi(inner.span.lo());
let message = match mutbl {
hir::Mutability::Not => "consider removing the `&`",
hir::Mutability::Mut => "consider removing the `&mut`",
};
let suggestion = String::new();
return Some((
prefix_span,
message,
suggestion,
Applicability::MachineApplicable,
));
} else if self.infcx.type_is_copy_modulo_regions(
self.param_env,
expected,
sp,
) {
// For this suggestion to make sense, the type would need to be `Copy`.
if let Ok(code) = sm.span_to_snippet(expr.span) {
let message = if checked_ty.is_region_ptr() {
"consider dereferencing the borrow"
} else {
Expand All @@ -631,7 +651,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
format!("*{}", code)
};
return Some((
sp,
expr.span,
message,
suggestion,
Applicability::MachineApplicable,
Expand Down
24 changes: 24 additions & 0 deletions src/test/ui/suggestions/issue-82361.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// run-rustfix

fn main() {
let a: usize = 123;
let b: &usize = &a;

if true {
a
} else {
*b //~ ERROR `if` and `else` have incompatible types [E0308]
};

if true {
1
} else {
1 //~ ERROR `if` and `else` have incompatible types [E0308]
};

if true {
1
} else {
1 //~ ERROR `if` and `else` have incompatible types [E0308]
};
}
24 changes: 24 additions & 0 deletions src/test/ui/suggestions/issue-82361.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// run-rustfix

fn main() {
let a: usize = 123;
let b: &usize = &a;

if true {
a
} else {
b //~ ERROR `if` and `else` have incompatible types [E0308]
};

if true {
1
} else {
&1 //~ ERROR `if` and `else` have incompatible types [E0308]
};

if true {
1
} else {
&mut 1 //~ ERROR `if` and `else` have incompatible types [E0308]
};
}
48 changes: 48 additions & 0 deletions src/test/ui/suggestions/issue-82361.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
error[E0308]: `if` and `else` have incompatible types
--> $DIR/issue-82361.rs:10:9
|
LL | / if true {
LL | | a
| | - expected because of this
LL | | } else {
LL | | b
| | ^
| | |
| | expected `usize`, found `&usize`
| | help: consider dereferencing the borrow: `*b`
LL | | };
| |_____- `if` and `else` have incompatible types

error[E0308]: `if` and `else` have incompatible types
--> $DIR/issue-82361.rs:16:9
|
LL | / if true {
LL | | 1
| | - expected because of this
LL | | } else {
LL | | &1
| | -^
| | |
| | expected integer, found `&{integer}`
| | help: consider removing the `&`
LL | | };
| |_____- `if` and `else` have incompatible types

error[E0308]: `if` and `else` have incompatible types
--> $DIR/issue-82361.rs:22:9
|
LL | / if true {
LL | | 1
| | - expected because of this
LL | | } else {
LL | | &mut 1
| | -----^
| | |
| | expected integer, found `&mut {integer}`
| | help: consider removing the `&mut`
LL | | };
| |_____- `if` and `else` have incompatible types

error: aborting due to 3 previous errors

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

0 comments on commit fa74d48

Please sign in to comment.