From ad78b50a86c6ac908df1c385642d46d3183740d6 Mon Sep 17 00:00:00 2001 From: Tor Hovland Date: Sat, 24 Apr 2021 13:57:41 +0200 Subject: [PATCH 1/5] Implemented suggestion. --- compiler/rustc_typeck/src/check/coercion.rs | 4 +- .../src/check/fn_ctxt/suggestions.rs | 78 ++++++++++++++++++- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 427f967a9b6bd..236fec94bdba7 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -1494,7 +1494,9 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { if let (Some((expr, _)), Some((fn_decl, _, _))) = (expression, fcx.get_node_fn_decl(parent_item)) { - fcx.suggest_missing_return_expr(&mut err, expr, fn_decl, expected, found, parent_id); + fcx.suggest_missing_break_or_return_expr( + &mut err, expr, fn_decl, expected, found, id, parent_id, + ); } if let (Some(sp), Some(fn_output)) = (fcx.ret_coercion_span.get(), fn_output) { diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index b758334484535..c3417247725fe 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -8,7 +8,7 @@ use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind}; use rustc_hir::lang_items::LangItem; -use rustc_hir::{ExprKind, ItemKind, Node}; +use rustc_hir::{ExprKind, ItemKind, Node, StmtKind}; use rustc_infer::infer; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::{self, Binder, Ty}; @@ -55,7 +55,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pointing_at_return_type = self.suggest_missing_return_type(err, &fn_decl, expected, found, can_suggest); let fn_id = self.tcx.hir().get_return_block(blk_id).unwrap(); - self.suggest_missing_return_expr(err, expr, &fn_decl, expected, found, fn_id); + self.suggest_missing_break_or_return_expr( + err, expr, &fn_decl, expected, found, blk_id, fn_id, + ); } pointing_at_return_type } @@ -472,7 +474,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - pub(in super::super) fn suggest_missing_return_expr( + pub(in super::super) fn suggest_missing_break_or_return_expr( &self, err: &mut DiagnosticBuilder<'_>, expr: &'tcx hir::Expr<'tcx>, @@ -480,14 +482,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected: Ty<'tcx>, found: Ty<'tcx>, id: hir::HirId, + fn_id: hir::HirId, ) { if !expected.is_unit() { return; } let found = self.resolve_vars_with_obligations(found); + + if self.in_loop(id) { + if self.in_local_statement(id) { + err.multipart_suggestion( + "you might have meant to break the loop with this value", + vec![ + (expr.span.shrink_to_lo(), "break ".to_string()), + (expr.span.shrink_to_hi(), ";".to_string()), + ], + Applicability::MaybeIncorrect, + ); + return; + } + } + if let hir::FnRetTy::Return(ty) = fn_decl.output { let ty = >::ast_ty_to_ty(self, ty); - let bound_vars = self.tcx.late_bound_vars(id); + let bound_vars = self.tcx.late_bound_vars(fn_id); let ty = self.tcx.erase_late_bound_regions(Binder::bind_with_vars(ty, bound_vars)); let ty = self.normalize_associated_types_in(expr.span, ty); if self.can_coerce(found, ty) { @@ -514,4 +532,56 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.tcx.sess.parse_sess.expr_parentheses_needed(err, *sp, None); } } + + fn in_loop(&self, id: hir::HirId) -> bool { + if self.is_loop(id) { + return true; + } + + for (parent_id, _) in self.tcx.hir().parent_iter(id) { + if self.is_loop(parent_id) { + return true; + } + } + + false + } + + fn is_loop(&self, id: hir::HirId) -> bool { + let node = self.tcx.hir().get(id); + + if let Node::Expr(expr) = node { + if let ExprKind::Loop(..) = expr.kind { + return true; + } + } + + false + } + + fn in_local_statement(&self, id: hir::HirId) -> bool { + if self.is_local_statement(id) { + return true; + } + + for (parent_id, _) in self.tcx.hir().parent_iter(id) { + if self.is_local_statement(parent_id) { + return true; + } + } + + false + } + + fn is_local_statement(&self, id: hir::HirId) -> bool { + let node = self.tcx.hir().get(id); + + if let Node::Stmt(stmt) = node { + if let StmtKind::Local(..) = stmt.kind { + return true; + } + } + + false + } } From 0e7489a2e9443069d5a7063f0b47d9923921f57f Mon Sep 17 00:00:00 2001 From: Tor Hovland Date: Sat, 24 Apr 2021 18:08:22 +0200 Subject: [PATCH 2/5] Added a test. --- src/test/ui/loops/loop-no-implicit-break.rs | 5 +++++ src/test/ui/loops/loop-no-implicit-break.stderr | 14 ++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 src/test/ui/loops/loop-no-implicit-break.rs create mode 100644 src/test/ui/loops/loop-no-implicit-break.stderr diff --git a/src/test/ui/loops/loop-no-implicit-break.rs b/src/test/ui/loops/loop-no-implicit-break.rs new file mode 100644 index 0000000000000..0c57baf14394b --- /dev/null +++ b/src/test/ui/loops/loop-no-implicit-break.rs @@ -0,0 +1,5 @@ +fn main() { + let x: i8 = loop { + 10 //~ ERROR mismatched types + }; +} diff --git a/src/test/ui/loops/loop-no-implicit-break.stderr b/src/test/ui/loops/loop-no-implicit-break.stderr new file mode 100644 index 0000000000000..99767b78d35e3 --- /dev/null +++ b/src/test/ui/loops/loop-no-implicit-break.stderr @@ -0,0 +1,14 @@ +error[E0308]: mismatched types + --> $DIR/loop-no-implicit-break.rs:3:9 + | +LL | 10 + | ^^ expected `()`, found integer + | +help: you might have meant to break the loop with this value + | +LL | break 10; + | ^^^^^ ^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 8bc81a0e4d9d5edc72af0cffe31a78a0bd2156f2 Mon Sep 17 00:00:00 2001 From: Tor Hovland Date: Sat, 24 Apr 2021 18:49:21 +0200 Subject: [PATCH 3/5] Refactor. --- .../src/check/fn_ctxt/suggestions.rs | 80 ++++++------------- 1 file changed, 23 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index c3417247725fe..d6b1e56316b37 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -8,7 +8,7 @@ use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind}; use rustc_hir::lang_items::LangItem; -use rustc_hir::{ExprKind, ItemKind, Node, StmtKind}; +use rustc_hir::{Expr, ExprKind, ItemKind, Node, Stmt, StmtKind}; use rustc_infer::infer; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::{self, Binder, Ty}; @@ -489,18 +489,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } let found = self.resolve_vars_with_obligations(found); - if self.in_loop(id) { - if self.in_local_statement(id) { - err.multipart_suggestion( - "you might have meant to break the loop with this value", - vec![ - (expr.span.shrink_to_lo(), "break ".to_string()), - (expr.span.shrink_to_hi(), ";".to_string()), - ], - Applicability::MaybeIncorrect, - ); - return; - } + let in_loop = self.is_loop(id) + || self.tcx.hir().parent_iter(id).any(|(parent_id, _)| self.is_loop(parent_id)); + + let in_local_statement = self.is_local_statement(id) + || self + .tcx + .hir() + .parent_iter(id) + .any(|(parent_id, _)| self.is_local_statement(parent_id)); + + if in_loop && in_local_statement { + err.multipart_suggestion( + "you might have meant to break the loop with this value", + vec![ + (expr.span.shrink_to_lo(), "break ".to_string()), + (expr.span.shrink_to_hi(), ";".to_string()), + ], + Applicability::MaybeIncorrect, + ); + return; } if let hir::FnRetTy::Return(ty) = fn_decl.output { @@ -533,55 +541,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - fn in_loop(&self, id: hir::HirId) -> bool { - if self.is_loop(id) { - return true; - } - - for (parent_id, _) in self.tcx.hir().parent_iter(id) { - if self.is_loop(parent_id) { - return true; - } - } - - false - } - fn is_loop(&self, id: hir::HirId) -> bool { let node = self.tcx.hir().get(id); - - if let Node::Expr(expr) = node { - if let ExprKind::Loop(..) = expr.kind { - return true; - } - } - - false - } - - fn in_local_statement(&self, id: hir::HirId) -> bool { - if self.is_local_statement(id) { - return true; - } - - for (parent_id, _) in self.tcx.hir().parent_iter(id) { - if self.is_local_statement(parent_id) { - return true; - } - } - - false + matches!(node, Node::Expr(Expr { kind: ExprKind::Loop(..), .. })) } fn is_local_statement(&self, id: hir::HirId) -> bool { let node = self.tcx.hir().get(id); - - if let Node::Stmt(stmt) = node { - if let StmtKind::Local(..) = stmt.kind { - return true; - } - } - - false + matches!(node, Node::Stmt(Stmt { kind: StmtKind::Local(..), .. })) } } From 05a5a1128fc5da178f9b7bd0ab499258652dfc49 Mon Sep 17 00:00:00 2001 From: Tor Hovland Date: Sat, 24 Apr 2021 19:00:24 +0200 Subject: [PATCH 4/5] More tests. --- src/test/ui/loops/loop-no-implicit-break.rs | 26 +++++++++++++-- .../ui/loops/loop-no-implicit-break.stderr | 32 ++++++++++++++++--- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/test/ui/loops/loop-no-implicit-break.rs b/src/test/ui/loops/loop-no-implicit-break.rs index 0c57baf14394b..fc3b3c4a30fac 100644 --- a/src/test/ui/loops/loop-no-implicit-break.rs +++ b/src/test/ui/loops/loop-no-implicit-break.rs @@ -1,5 +1,27 @@ fn main() { - let x: i8 = loop { - 10 //~ ERROR mismatched types + let a: i8 = loop { + 1 //~ ERROR mismatched types }; + + let b: i8 = loop { + break 1; + }; +} + +fn foo() -> i8 { + let a: i8 = loop { + 1 //~ ERROR mismatched types + }; + + let b: i8 = loop { + break 1; + }; + + loop { + 1 //~ ERROR mismatched types + } + + loop { + return 1; + } } diff --git a/src/test/ui/loops/loop-no-implicit-break.stderr b/src/test/ui/loops/loop-no-implicit-break.stderr index 99767b78d35e3..cde6bbe512b28 100644 --- a/src/test/ui/loops/loop-no-implicit-break.stderr +++ b/src/test/ui/loops/loop-no-implicit-break.stderr @@ -1,14 +1,36 @@ error[E0308]: mismatched types --> $DIR/loop-no-implicit-break.rs:3:9 | -LL | 10 - | ^^ expected `()`, found integer +LL | 1 + | ^ expected `()`, found integer | help: you might have meant to break the loop with this value | -LL | break 10; - | ^^^^^ ^ +LL | break 1; + | ^^^^^ ^ -error: aborting due to previous error +error[E0308]: mismatched types + --> $DIR/loop-no-implicit-break.rs:13:9 + | +LL | 1 + | ^ expected `()`, found integer + | +help: you might have meant to break the loop with this value + | +LL | break 1; + | ^^^^^ ^ + +error[E0308]: mismatched types + --> $DIR/loop-no-implicit-break.rs:21:9 + | +LL | 1 + | ^ expected `()`, found integer + | +help: you might have meant to return this value + | +LL | return 1; + | ^^^^^^ ^ + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0308`. From 3b504610b6768d8cb700bc2a8fa2a6263b9d3a06 Mon Sep 17 00:00:00 2001 From: Tor Hovland Date: Sat, 24 Apr 2021 22:20:08 +0200 Subject: [PATCH 5/5] One more test case. --- src/test/ui/loops/loop-no-implicit-break.rs | 4 ++++ src/test/ui/loops/loop-no-implicit-break.stderr | 13 ++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/test/ui/loops/loop-no-implicit-break.rs b/src/test/ui/loops/loop-no-implicit-break.rs index fc3b3c4a30fac..93078cb4b144b 100644 --- a/src/test/ui/loops/loop-no-implicit-break.rs +++ b/src/test/ui/loops/loop-no-implicit-break.rs @@ -24,4 +24,8 @@ fn foo() -> i8 { loop { return 1; } + + loop { + 1 //~ ERROR mismatched types + } } diff --git a/src/test/ui/loops/loop-no-implicit-break.stderr b/src/test/ui/loops/loop-no-implicit-break.stderr index cde6bbe512b28..5087662e7bfa4 100644 --- a/src/test/ui/loops/loop-no-implicit-break.stderr +++ b/src/test/ui/loops/loop-no-implicit-break.stderr @@ -31,6 +31,17 @@ help: you might have meant to return this value LL | return 1; | ^^^^^^ ^ -error: aborting due to 3 previous errors +error[E0308]: mismatched types + --> $DIR/loop-no-implicit-break.rs:29:9 + | +LL | 1 + | ^ expected `()`, found integer + | +help: you might have meant to return this value + | +LL | return 1; + | ^^^^^^ ^ + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0308`.