From 025b7c433c109dad2c84f1cbeae422a3ffbd01b6 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 21 Feb 2022 19:49:15 -0800 Subject: [PATCH] fix ICE when passing empty block to while-loop condition --- compiler/rustc_typeck/src/check/expr.rs | 35 +++-- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 134 ++++++++++-------- src/test/ui/typeck/while-loop-block-cond.rs | 4 + .../ui/typeck/while-loop-block-cond.stderr | 9 ++ 4 files changed, 111 insertions(+), 71 deletions(-) create mode 100644 src/test/ui/typeck/while-loop-block-cond.rs create mode 100644 src/test/ui/typeck/while-loop-block-cond.stderr diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index 8d4ffefda73bb..15284dc008f50 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -842,7 +842,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); err.span_label(lhs.span, "cannot assign to this expression"); - let mut parent = self.tcx.hir().get_parent_node(lhs.hir_id); + self.comes_from_while_condition(lhs.hir_id, |expr| { + err.span_suggestion_verbose( + expr.span.shrink_to_lo(), + "you might have meant to use pattern destructuring", + "let ".to_string(), + Applicability::MachineApplicable, + ); + }); + + err.emit(); + } + + // Check if an expression `original_expr_id` comes from the condition of a while loop, + // as opposed from the body of a while loop, which we can naively check by iterating + // parents until we find a loop... + pub(super) fn comes_from_while_condition( + &self, + original_expr_id: HirId, + then: impl FnOnce(&hir::Expr<'_>), + ) { + let mut parent = self.tcx.hir().get_parent_node(original_expr_id); while let Some(node) = self.tcx.hir().find(parent) { match node { hir::Node::Expr(hir::Expr { @@ -863,8 +883,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ), .. }) => { - // Check if our lhs is a child of the condition of a while loop - let expr_is_ancestor = std::iter::successors(Some(lhs.hir_id), |id| { + // Check if our original expression is a child of the condition of a while loop + let expr_is_ancestor = std::iter::successors(Some(original_expr_id), |id| { self.tcx.hir().find_parent_node(*id) }) .take_while(|id| *id != parent) @@ -872,12 +892,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // if it is, then we have a situation like `while Some(0) = value.get(0) {`, // where `while let` was more likely intended. if expr_is_ancestor { - err.span_suggestion_verbose( - expr.span.shrink_to_lo(), - "you might have meant to use pattern destructuring", - "let ".to_string(), - Applicability::MachineApplicable, - ); + then(expr); } break; } @@ -890,8 +905,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } } - - err.emit(); } // A generic function for checking the 'then' and 'else' clauses in an 'if' diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index 4b6460b62b77a..769b0f191012c 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -770,55 +770,57 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let prev_diverges = self.diverges.get(); let ctxt = BreakableCtxt { coerce: Some(coerce), may_break: false }; - let (ctxt, ()) = self.with_breakable_ctxt(blk.hir_id, ctxt, || { - for (pos, s) in blk.stmts.iter().enumerate() { - self.check_stmt(s, blk.stmts.len() - 1 == pos); - } + let (ctxt, ()) = + self.with_breakable_ctxt(blk.hir_id, ctxt, || { + for (pos, s) in blk.stmts.iter().enumerate() { + self.check_stmt(s, blk.stmts.len() - 1 == pos); + } - // check the tail expression **without** holding the - // `enclosing_breakables` lock below. - let tail_expr_ty = tail_expr.map(|t| self.check_expr_with_expectation(t, expected)); - - let mut enclosing_breakables = self.enclosing_breakables.borrow_mut(); - let ctxt = enclosing_breakables.find_breakable(blk.hir_id); - let coerce = ctxt.coerce.as_mut().unwrap(); - if let Some(tail_expr_ty) = tail_expr_ty { - let tail_expr = tail_expr.unwrap(); - let span = self.get_expr_coercion_span(tail_expr); - let cause = self.cause(span, ObligationCauseCode::BlockTailExpression(blk.hir_id)); - coerce.coerce(self, &cause, tail_expr, tail_expr_ty); - } else { - // Subtle: if there is no explicit tail expression, - // that is typically equivalent to a tail expression - // of `()` -- except if the block diverges. In that - // case, there is no value supplied from the tail - // expression (assuming there are no other breaks, - // this implies that the type of the block will be - // `!`). - // - // #41425 -- label the implicit `()` as being the - // "found type" here, rather than the "expected type". - if !self.diverges.get().is_always() { - // #50009 -- Do not point at the entire fn block span, point at the return type - // span, as it is the cause of the requirement, and - // `consider_hint_about_removing_semicolon` will point at the last expression - // if it were a relevant part of the error. This improves usability in editors - // that highlight errors inline. - let mut sp = blk.span; - let mut fn_span = None; - if let Some((decl, ident)) = self.get_parent_fn_decl(blk.hir_id) { - let ret_sp = decl.output.span(); - if let Some(block_sp) = self.parent_item_span(blk.hir_id) { - // HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the - // output would otherwise be incorrect and even misleading. Make sure - // the span we're aiming at correspond to a `fn` body. - if block_sp == blk.span { - sp = ret_sp; - fn_span = Some(ident.span); + // check the tail expression **without** holding the + // `enclosing_breakables` lock below. + let tail_expr_ty = tail_expr.map(|t| self.check_expr_with_expectation(t, expected)); + + let mut enclosing_breakables = self.enclosing_breakables.borrow_mut(); + let ctxt = enclosing_breakables.find_breakable(blk.hir_id); + let coerce = ctxt.coerce.as_mut().unwrap(); + if let Some(tail_expr_ty) = tail_expr_ty { + let tail_expr = tail_expr.unwrap(); + let span = self.get_expr_coercion_span(tail_expr); + let cause = + self.cause(span, ObligationCauseCode::BlockTailExpression(blk.hir_id)); + coerce.coerce(self, &cause, tail_expr, tail_expr_ty); + } else { + // Subtle: if there is no explicit tail expression, + // that is typically equivalent to a tail expression + // of `()` -- except if the block diverges. In that + // case, there is no value supplied from the tail + // expression (assuming there are no other breaks, + // this implies that the type of the block will be + // `!`). + // + // #41425 -- label the implicit `()` as being the + // "found type" here, rather than the "expected type". + if !self.diverges.get().is_always() { + // #50009 -- Do not point at the entire fn block span, point at the return type + // span, as it is the cause of the requirement, and + // `consider_hint_about_removing_semicolon` will point at the last expression + // if it were a relevant part of the error. This improves usability in editors + // that highlight errors inline. + let mut sp = blk.span; + let mut fn_span = None; + if let Some((decl, ident)) = self.get_parent_fn_decl(blk.hir_id) { + let ret_sp = decl.output.span(); + if let Some(block_sp) = self.parent_item_span(blk.hir_id) { + // HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the + // output would otherwise be incorrect and even misleading. Make sure + // the span we're aiming at correspond to a `fn` body. + if block_sp == blk.span { + sp = ret_sp; + fn_span = Some(ident.span); + } } } - } - coerce.coerce_forced_unit( + coerce.coerce_forced_unit( self, &self.misc(sp), &mut |err| { @@ -827,19 +829,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if expected_ty == self.tcx.types.bool { // If this is caused by a missing `let` in a `while let`, // silence this redundant error, as we already emit E0070. - let parent = self.tcx.hir().get_parent_node(blk.hir_id); - let parent = self.tcx.hir().get_parent_node(parent); - let parent = self.tcx.hir().get_parent_node(parent); - let parent = self.tcx.hir().get_parent_node(parent); - let parent = self.tcx.hir().get_parent_node(parent); - match self.tcx.hir().find(parent) { - Some(hir::Node::Expr(hir::Expr { - kind: hir::ExprKind::Loop(_, _, hir::LoopSource::While, _), - .. - })) => { + + // Our block must be a `assign desugar local; assignment` + if let Some(hir::Node::Block(hir::Block { + stmts: + [hir::Stmt { + kind: + hir::StmtKind::Local(hir::Local { + source: hir::LocalSource::AssignDesugar(_), + .. + }), + .. + }, hir::Stmt { + kind: + hir::StmtKind::Expr(hir::Expr { + kind: hir::ExprKind::Assign(..), + .. + }), + .. + }], + .. + })) = self.tcx.hir().find(blk.hir_id) + { + self.comes_from_while_condition(blk.hir_id, |_| { err.downgrade_to_delayed_bug(); - } - _ => {} + }) } } } @@ -853,9 +867,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }, false, ); + } } - } - }); + }); if ctxt.may_break { // If we can break from the block, then the block's exit is always reachable diff --git a/src/test/ui/typeck/while-loop-block-cond.rs b/src/test/ui/typeck/while-loop-block-cond.rs new file mode 100644 index 0000000000000..929759766f279 --- /dev/null +++ b/src/test/ui/typeck/while-loop-block-cond.rs @@ -0,0 +1,4 @@ +fn main() { + while {} {} + //~^ ERROR mismatched types [E0308] +} diff --git a/src/test/ui/typeck/while-loop-block-cond.stderr b/src/test/ui/typeck/while-loop-block-cond.stderr new file mode 100644 index 0000000000000..598273af9cfc4 --- /dev/null +++ b/src/test/ui/typeck/while-loop-block-cond.stderr @@ -0,0 +1,9 @@ +error[E0308]: mismatched types + --> $DIR/while-loop-block-cond.rs:2:11 + | +LL | while {} {} + | ^^ expected `bool`, found `()` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`.