From d84c4cd7188f484083091bee436dc07eb90b47e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 8 Oct 2019 08:26:42 -0700 Subject: [PATCH 1/3] Ignore `ExprKind::DropTemps` for some ref suggestions --- src/librustc/hir/mod.rs | 17 +++++++++++ src/librustc_typeck/check/demand.rs | 3 ++ src/test/ui/if/if-no-match-bindings.stderr | 28 ++++++------------- .../disallowed-positions.stderr | 4 +-- 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 30b0503668877..ead7a08c397f0 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1548,6 +1548,23 @@ impl Expr { } } } + + /// If `Self.kind` is `ExprKind::DropTemps(expr)`, drill down until we get a non-`DropTemps` + /// `Expr`. This is used in suggestions to ignore this `ExprKind` as it is semantically + /// silent, only signaling the ownership system. By doing this, suggestions that check the + /// `ExprKind` of any given `Expr` for presentation don't have to care about `DropTemps` + /// beyond remembering to call this function before doing analysis on it. + pub fn peel_drop_temps(&self) -> &Self { + let mut base_expr = self; + loop { + match &base_expr.kind { + ExprKind::DropTemps(expr) => { + base_expr = &expr; + } + _ => return base_expr, + } + } + } } impl fmt::Debug for Expr { diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 78bd4508e21a4..9f7f8b7680f4b 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -355,6 +355,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; let is_macro = sp.from_expansion() && !is_desugaring; + // `ExprKind::DropTemps` is semantically irrelevant for these suggestions. + let expr = expr.peel_drop_temps(); + match (&expr.kind, &expected.kind, &checked_ty.kind) { (_, &ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.kind, &check.kind) { (&ty::Str, &ty::Array(arr, _)) | diff --git a/src/test/ui/if/if-no-match-bindings.stderr b/src/test/ui/if/if-no-match-bindings.stderr index 53b7aafc430a2..fa2455db2b2a9 100644 --- a/src/test/ui/if/if-no-match-bindings.stderr +++ b/src/test/ui/if/if-no-match-bindings.stderr @@ -2,10 +2,7 @@ error[E0308]: mismatched types --> $DIR/if-no-match-bindings.rs:18:8 | LL | if b_ref() {} - | ^^^^^^^ - | | - | expected bool, found &bool - | help: consider dereferencing the borrow: `*b_ref()` + | ^^^^^^^ expected bool, found &bool | = note: expected type `bool` found type `&bool` @@ -14,10 +11,7 @@ error[E0308]: mismatched types --> $DIR/if-no-match-bindings.rs:19:8 | LL | if b_mut_ref() {} - | ^^^^^^^^^^^ - | | - | expected bool, found &mut bool - | help: consider dereferencing the borrow: `*b_mut_ref()` + | ^^^^^^^^^^^ expected bool, found &mut bool | = note: expected type `bool` found type `&mut bool` @@ -29,7 +23,7 @@ LL | if &true {} | ^^^^^ | | | expected bool, found &bool - | help: consider dereferencing the borrow: `*&true` + | help: consider removing the borrow: `true` | = note: expected type `bool` found type `&bool` @@ -41,7 +35,7 @@ LL | if &mut true {} | ^^^^^^^^^ | | | expected bool, found &mut bool - | help: consider dereferencing the borrow: `*&mut true` + | help: consider removing the borrow: `true` | = note: expected type `bool` found type `&mut bool` @@ -50,10 +44,7 @@ error[E0308]: mismatched types --> $DIR/if-no-match-bindings.rs:24:11 | LL | while b_ref() {} - | ^^^^^^^ - | | - | expected bool, found &bool - | help: consider dereferencing the borrow: `*b_ref()` + | ^^^^^^^ expected bool, found &bool | = note: expected type `bool` found type `&bool` @@ -62,10 +53,7 @@ error[E0308]: mismatched types --> $DIR/if-no-match-bindings.rs:25:11 | LL | while b_mut_ref() {} - | ^^^^^^^^^^^ - | | - | expected bool, found &mut bool - | help: consider dereferencing the borrow: `*b_mut_ref()` + | ^^^^^^^^^^^ expected bool, found &mut bool | = note: expected type `bool` found type `&mut bool` @@ -77,7 +65,7 @@ LL | while &true {} | ^^^^^ | | | expected bool, found &bool - | help: consider dereferencing the borrow: `*&true` + | help: consider removing the borrow: `true` | = note: expected type `bool` found type `&bool` @@ -89,7 +77,7 @@ LL | while &mut true {} | ^^^^^^^^^ | | | expected bool, found &mut bool - | help: consider dereferencing the borrow: `*&mut true` + | help: consider removing the borrow: `true` | = note: expected type `bool` found type `&mut bool` diff --git a/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr b/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr index 619f9c85b24db..ad4686c1915d6 100644 --- a/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr +++ b/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr @@ -520,7 +520,7 @@ LL | if &let 0 = 0 {} | ^^^^^^^^^^ | | | expected bool, found &bool - | help: consider dereferencing the borrow: `*&let 0 = 0` + | help: consider removing the borrow: `let 0 = 0` | = note: expected type `bool` found type `&bool` @@ -708,7 +708,7 @@ LL | while &let 0 = 0 {} | ^^^^^^^^^^ | | | expected bool, found &bool - | help: consider dereferencing the borrow: `*&let 0 = 0` + | help: consider removing the borrow: `let 0 = 0` | = note: expected type `bool` found type `&bool` From ac9025c1979d0b4019d90a8ab7ad4336d468a0de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 8 Oct 2019 08:42:26 -0700 Subject: [PATCH 2/3] Call `Expr::peel_drop_temps()` from more places for more accurate suggestions --- src/librustc_typeck/check/demand.rs | 1 + src/librustc_typeck/check/expr.rs | 6 +----- src/librustc_typeck/check/mod.rs | 9 +++++---- src/test/ui/if/if-no-match-bindings.stderr | 20 ++++++++++++++++---- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 9f7f8b7680f4b..2b672c96dfa6b 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -109,6 +109,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { allow_two_phase: AllowTwoPhase) -> (Ty<'tcx>, Option>) { let expected = self.resolve_type_vars_with_obligations(expected); + let expr = expr.peel_drop_temps(); let e = match self.try_coerce(expr, checked_ty, expected, allow_two_phase) { Ok(ty) => return (ty, None), diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index aa26c74967a1e..ad46a443b8ffa 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -87,12 +87,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } if let Some(mut err) = self.demand_suptype_diag(expr.span, expected_ty, ty) { + let expr = expr.peel_drop_temps(); self.suggest_ref_or_into(&mut err, expr, expected_ty, ty); - - let expr = match &expr.kind { - ExprKind::DropTemps(expr) => expr, - _ => expr, - }; extend_err(&mut err); // Error possibly reported in `check_assign` so avoid emitting error again. err.emit_unless(self.is_assign_to_bool(expr, expected_ty)); diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index f130ee821d17c..7475b9cc3b327 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -4216,20 +4216,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub fn suggest_mismatched_types_on_tail( &self, err: &mut DiagnosticBuilder<'tcx>, - expression: &'tcx hir::Expr, + expr: &'tcx hir::Expr, expected: Ty<'tcx>, found: Ty<'tcx>, cause_span: Span, blk_id: hir::HirId, ) -> bool { - self.suggest_missing_semicolon(err, expression, expected, cause_span); + let expr = expr.peel_drop_temps(); + self.suggest_missing_semicolon(err, expr, expected, cause_span); let mut pointing_at_return_type = false; if let Some((fn_decl, can_suggest)) = self.get_fn_decl(blk_id) { pointing_at_return_type = self.suggest_missing_return_type( err, &fn_decl, expected, found, can_suggest); } - self.suggest_ref_or_into(err, expression, expected, found); - self.suggest_boxing_when_appropriate(err, expression, expected, found); + self.suggest_ref_or_into(err, expr, expected, found); + self.suggest_boxing_when_appropriate(err, expr, expected, found); pointing_at_return_type } diff --git a/src/test/ui/if/if-no-match-bindings.stderr b/src/test/ui/if/if-no-match-bindings.stderr index fa2455db2b2a9..0936f3b9e38e8 100644 --- a/src/test/ui/if/if-no-match-bindings.stderr +++ b/src/test/ui/if/if-no-match-bindings.stderr @@ -2,7 +2,10 @@ error[E0308]: mismatched types --> $DIR/if-no-match-bindings.rs:18:8 | LL | if b_ref() {} - | ^^^^^^^ expected bool, found &bool + | ^^^^^^^ + | | + | expected bool, found &bool + | help: consider dereferencing the borrow: `*b_ref()` | = note: expected type `bool` found type `&bool` @@ -11,7 +14,10 @@ error[E0308]: mismatched types --> $DIR/if-no-match-bindings.rs:19:8 | LL | if b_mut_ref() {} - | ^^^^^^^^^^^ expected bool, found &mut bool + | ^^^^^^^^^^^ + | | + | expected bool, found &mut bool + | help: consider dereferencing the borrow: `*b_mut_ref()` | = note: expected type `bool` found type `&mut bool` @@ -44,7 +50,10 @@ error[E0308]: mismatched types --> $DIR/if-no-match-bindings.rs:24:11 | LL | while b_ref() {} - | ^^^^^^^ expected bool, found &bool + | ^^^^^^^ + | | + | expected bool, found &bool + | help: consider dereferencing the borrow: `*b_ref()` | = note: expected type `bool` found type `&bool` @@ -53,7 +62,10 @@ error[E0308]: mismatched types --> $DIR/if-no-match-bindings.rs:25:11 | LL | while b_mut_ref() {} - | ^^^^^^^^^^^ expected bool, found &mut bool + | ^^^^^^^^^^^ + | | + | expected bool, found &mut bool + | help: consider dereferencing the borrow: `*b_mut_ref()` | = note: expected type `bool` found type `&mut bool` From d0eea6ff6dbdab88ddbcf0ce2dcbbfd456678e39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 8 Oct 2019 09:46:57 -0700 Subject: [PATCH 3/3] review comments --- src/librustc/hir/mod.rs | 12 ++++-------- src/librustc_typeck/check/demand.rs | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index ead7a08c397f0..1f792ecc2da90 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1555,15 +1555,11 @@ impl Expr { /// `ExprKind` of any given `Expr` for presentation don't have to care about `DropTemps` /// beyond remembering to call this function before doing analysis on it. pub fn peel_drop_temps(&self) -> &Self { - let mut base_expr = self; - loop { - match &base_expr.kind { - ExprKind::DropTemps(expr) => { - base_expr = &expr; - } - _ => return base_expr, - } + let mut expr = self; + while let ExprKind::DropTemps(inner) = &expr.kind { + expr = inner; } + expr } } diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 2b672c96dfa6b..d92ea7fd49a72 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -109,13 +109,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { allow_two_phase: AllowTwoPhase) -> (Ty<'tcx>, Option>) { let expected = self.resolve_type_vars_with_obligations(expected); - let expr = expr.peel_drop_temps(); let e = match self.try_coerce(expr, checked_ty, expected, allow_two_phase) { Ok(ty) => return (ty, None), Err(e) => e }; + let expr = expr.peel_drop_temps(); let cause = self.misc(expr.span); let expr_ty = self.resolve_type_vars_with_obligations(checked_ty); let mut err = self.report_mismatched_types(&cause, expected, expr_ty, e);