Skip to content

Fix ICE when passing block to while-loop condition #94248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 28, 2022
Merged

Fix ICE when passing block to while-loop condition #94248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 24 additions & 11 deletions compiler/rustc_typeck/src/check/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -863,21 +883,16 @@ 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)
.any(|id| id == expr.hir_id);
// 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;
}
Expand All @@ -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'
Expand Down
134 changes: 74 additions & 60 deletions compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand All @@ -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();
}
_ => {}
})
}
}
}
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/typeck/while-loop-block-cond.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn main() {
while {} {}
//~^ ERROR mismatched types [E0308]
}
9 changes: 9 additions & 0 deletions src/test/ui/typeck/while-loop-block-cond.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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`.