From fa74d489a227054f20b0ffdda85e864e53cc7617 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Sun, 21 Feb 2021 13:29:43 +0300 Subject: [PATCH] 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 #82361 --- compiler/rustc_hir/src/hir.rs | 8 ++++ compiler/rustc_typeck/src/check/demand.rs | 28 +++++++++++-- src/test/ui/suggestions/issue-82361.fixed | 24 +++++++++++ src/test/ui/suggestions/issue-82361.rs | 24 +++++++++++ src/test/ui/suggestions/issue-82361.stderr | 48 ++++++++++++++++++++++ 5 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/suggestions/issue-82361.fixed create mode 100644 src/test/ui/suggestions/issue-82361.rs create mode 100644 src/test/ui/suggestions/issue-82361.stderr diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 2fef7c2cc087d..f4402843afcbe 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -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, diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index 8d2004a543b7b..39b973ed371ae 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -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 { @@ -631,7 +651,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { format!("*{}", code) }; return Some(( - sp, + expr.span, message, suggestion, Applicability::MachineApplicable, diff --git a/src/test/ui/suggestions/issue-82361.fixed b/src/test/ui/suggestions/issue-82361.fixed new file mode 100644 index 0000000000000..d72de982bf98a --- /dev/null +++ b/src/test/ui/suggestions/issue-82361.fixed @@ -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] + }; +} diff --git a/src/test/ui/suggestions/issue-82361.rs b/src/test/ui/suggestions/issue-82361.rs new file mode 100644 index 0000000000000..c068f6d22b476 --- /dev/null +++ b/src/test/ui/suggestions/issue-82361.rs @@ -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] + }; +} diff --git a/src/test/ui/suggestions/issue-82361.stderr b/src/test/ui/suggestions/issue-82361.stderr new file mode 100644 index 0000000000000..c19d59ccd4c66 --- /dev/null +++ b/src/test/ui/suggestions/issue-82361.stderr @@ -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`.