Skip to content

Commit 9340791

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of rust-lang#94248 - compiler-errors:fix-while-loop-bad-delay, r=petrochenkov
Fix ICE when passing block to while-loop condition We were incorrectly delaying a bug when we passed _any_ block (that evaluated to `()`) to a while loop. This PR makes the check a bit more sophisticated. We should only suppress the error here in cases that are equivalent to those we find in rust-lang#93574 (i.e. only while loop conditions that have destructuring assignment expressions in them). Fixes rust-lang#93997 cc `@estebank` who added this code I would not be opposed to removing the delay-bug altogether, and just emitting this error always. I much prefer duplicate errors over no errors.
2 parents a040e2f + 025b7c4 commit 9340791

File tree

4 files changed

+111
-71
lines changed

4 files changed

+111
-71
lines changed

‎compiler/rustc_typeck/src/check/expr.rs

+24-11
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
840840
);
841841
err.span_label(lhs.span, "cannot assign to this expression");
842842

843-
let mut parent = self.tcx.hir().get_parent_node(lhs.hir_id);
843+
self.comes_from_while_condition(lhs.hir_id, |expr| {
844+
err.span_suggestion_verbose(
845+
expr.span.shrink_to_lo(),
846+
"you might have meant to use pattern destructuring",
847+
"let ".to_string(),
848+
Applicability::MachineApplicable,
849+
);
850+
});
851+
852+
err.emit();
853+
}
854+
855+
// Check if an expression `original_expr_id` comes from the condition of a while loop,
856+
// as opposed from the body of a while loop, which we can naively check by iterating
857+
// parents until we find a loop...
858+
pub(super) fn comes_from_while_condition(
859+
&self,
860+
original_expr_id: HirId,
861+
then: impl FnOnce(&hir::Expr<'_>),
862+
) {
863+
let mut parent = self.tcx.hir().get_parent_node(original_expr_id);
844864
while let Some(node) = self.tcx.hir().find(parent) {
845865
match node {
846866
hir::Node::Expr(hir::Expr {
@@ -861,21 +881,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
861881
),
862882
..
863883
}) => {
864-
// Check if our lhs is a child of the condition of a while loop
865-
let expr_is_ancestor = std::iter::successors(Some(lhs.hir_id), |id| {
884+
// Check if our original expression is a child of the condition of a while loop
885+
let expr_is_ancestor = std::iter::successors(Some(original_expr_id), |id| {
866886
self.tcx.hir().find_parent_node(*id)
867887
})
868888
.take_while(|id| *id != parent)
869889
.any(|id| id == expr.hir_id);
870890
// if it is, then we have a situation like `while Some(0) = value.get(0) {`,
871891
// where `while let` was more likely intended.
872892
if expr_is_ancestor {
873-
err.span_suggestion_verbose(
874-
expr.span.shrink_to_lo(),
875-
"you might have meant to use pattern destructuring",
876-
"let ".to_string(),
877-
Applicability::MachineApplicable,
878-
);
893+
then(expr);
879894
}
880895
break;
881896
}
@@ -888,8 +903,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
888903
}
889904
}
890905
}
891-
892-
err.emit();
893906
}
894907

895908
// A generic function for checking the 'then' and 'else' clauses in an 'if'

‎compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

+74-60
Original file line numberDiff line numberDiff line change
@@ -768,55 +768,57 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
768768
let prev_diverges = self.diverges.get();
769769
let ctxt = BreakableCtxt { coerce: Some(coerce), may_break: false };
770770

771-
let (ctxt, ()) = self.with_breakable_ctxt(blk.hir_id, ctxt, || {
772-
for (pos, s) in blk.stmts.iter().enumerate() {
773-
self.check_stmt(s, blk.stmts.len() - 1 == pos);
774-
}
771+
let (ctxt, ()) =
772+
self.with_breakable_ctxt(blk.hir_id, ctxt, || {
773+
for (pos, s) in blk.stmts.iter().enumerate() {
774+
self.check_stmt(s, blk.stmts.len() - 1 == pos);
775+
}
775776

776-
// check the tail expression **without** holding the
777-
// `enclosing_breakables` lock below.
778-
let tail_expr_ty = tail_expr.map(|t| self.check_expr_with_expectation(t, expected));
779-
780-
let mut enclosing_breakables = self.enclosing_breakables.borrow_mut();
781-
let ctxt = enclosing_breakables.find_breakable(blk.hir_id);
782-
let coerce = ctxt.coerce.as_mut().unwrap();
783-
if let Some(tail_expr_ty) = tail_expr_ty {
784-
let tail_expr = tail_expr.unwrap();
785-
let span = self.get_expr_coercion_span(tail_expr);
786-
let cause = self.cause(span, ObligationCauseCode::BlockTailExpression(blk.hir_id));
787-
coerce.coerce(self, &cause, tail_expr, tail_expr_ty);
788-
} else {
789-
// Subtle: if there is no explicit tail expression,
790-
// that is typically equivalent to a tail expression
791-
// of `()` -- except if the block diverges. In that
792-
// case, there is no value supplied from the tail
793-
// expression (assuming there are no other breaks,
794-
// this implies that the type of the block will be
795-
// `!`).
796-
//
797-
// #41425 -- label the implicit `()` as being the
798-
// "found type" here, rather than the "expected type".
799-
if !self.diverges.get().is_always() {
800-
// #50009 -- Do not point at the entire fn block span, point at the return type
801-
// span, as it is the cause of the requirement, and
802-
// `consider_hint_about_removing_semicolon` will point at the last expression
803-
// if it were a relevant part of the error. This improves usability in editors
804-
// that highlight errors inline.
805-
let mut sp = blk.span;
806-
let mut fn_span = None;
807-
if let Some((decl, ident)) = self.get_parent_fn_decl(blk.hir_id) {
808-
let ret_sp = decl.output.span();
809-
if let Some(block_sp) = self.parent_item_span(blk.hir_id) {
810-
// HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the
811-
// output would otherwise be incorrect and even misleading. Make sure
812-
// the span we're aiming at correspond to a `fn` body.
813-
if block_sp == blk.span {
814-
sp = ret_sp;
815-
fn_span = Some(ident.span);
777+
// check the tail expression **without** holding the
778+
// `enclosing_breakables` lock below.
779+
let tail_expr_ty = tail_expr.map(|t| self.check_expr_with_expectation(t, expected));
780+
781+
let mut enclosing_breakables = self.enclosing_breakables.borrow_mut();
782+
let ctxt = enclosing_breakables.find_breakable(blk.hir_id);
783+
let coerce = ctxt.coerce.as_mut().unwrap();
784+
if let Some(tail_expr_ty) = tail_expr_ty {
785+
let tail_expr = tail_expr.unwrap();
786+
let span = self.get_expr_coercion_span(tail_expr);
787+
let cause =
788+
self.cause(span, ObligationCauseCode::BlockTailExpression(blk.hir_id));
789+
coerce.coerce(self, &cause, tail_expr, tail_expr_ty);
790+
} else {
791+
// Subtle: if there is no explicit tail expression,
792+
// that is typically equivalent to a tail expression
793+
// of `()` -- except if the block diverges. In that
794+
// case, there is no value supplied from the tail
795+
// expression (assuming there are no other breaks,
796+
// this implies that the type of the block will be
797+
// `!`).
798+
//
799+
// #41425 -- label the implicit `()` as being the
800+
// "found type" here, rather than the "expected type".
801+
if !self.diverges.get().is_always() {
802+
// #50009 -- Do not point at the entire fn block span, point at the return type
803+
// span, as it is the cause of the requirement, and
804+
// `consider_hint_about_removing_semicolon` will point at the last expression
805+
// if it were a relevant part of the error. This improves usability in editors
806+
// that highlight errors inline.
807+
let mut sp = blk.span;
808+
let mut fn_span = None;
809+
if let Some((decl, ident)) = self.get_parent_fn_decl(blk.hir_id) {
810+
let ret_sp = decl.output.span();
811+
if let Some(block_sp) = self.parent_item_span(blk.hir_id) {
812+
// HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the
813+
// output would otherwise be incorrect and even misleading. Make sure
814+
// the span we're aiming at correspond to a `fn` body.
815+
if block_sp == blk.span {
816+
sp = ret_sp;
817+
fn_span = Some(ident.span);
818+
}
816819
}
817820
}
818-
}
819-
coerce.coerce_forced_unit(
821+
coerce.coerce_forced_unit(
820822
self,
821823
&self.misc(sp),
822824
&mut |err| {
@@ -825,19 +827,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
825827
if expected_ty == self.tcx.types.bool {
826828
// If this is caused by a missing `let` in a `while let`,
827829
// silence this redundant error, as we already emit E0070.
828-
let parent = self.tcx.hir().get_parent_node(blk.hir_id);
829-
let parent = self.tcx.hir().get_parent_node(parent);
830-
let parent = self.tcx.hir().get_parent_node(parent);
831-
let parent = self.tcx.hir().get_parent_node(parent);
832-
let parent = self.tcx.hir().get_parent_node(parent);
833-
match self.tcx.hir().find(parent) {
834-
Some(hir::Node::Expr(hir::Expr {
835-
kind: hir::ExprKind::Loop(_, _, hir::LoopSource::While, _),
836-
..
837-
})) => {
830+
831+
// Our block must be a `assign desugar local; assignment`
832+
if let Some(hir::Node::Block(hir::Block {
833+
stmts:
834+
[hir::Stmt {
835+
kind:
836+
hir::StmtKind::Local(hir::Local {
837+
source: hir::LocalSource::AssignDesugar(_),
838+
..
839+
}),
840+
..
841+
}, hir::Stmt {
842+
kind:
843+
hir::StmtKind::Expr(hir::Expr {
844+
kind: hir::ExprKind::Assign(..),
845+
..
846+
}),
847+
..
848+
}],
849+
..
850+
})) = self.tcx.hir().find(blk.hir_id)
851+
{
852+
self.comes_from_while_condition(blk.hir_id, |_| {
838853
err.downgrade_to_delayed_bug();
839-
}
840-
_ => {}
854+
})
841855
}
842856
}
843857
}
@@ -851,9 +865,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
851865
},
852866
false,
853867
);
868+
}
854869
}
855-
}
856-
});
870+
});
857871

858872
if ctxt.may_break {
859873
// If we can break from the block, then the block's exit is always reachable

‎src/test/ui/typeck/while-loop-block-cond.rs

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
fn main() {
2+
while {} {}
3+
//~^ ERROR mismatched types [E0308]
4+
}

‎src/test/ui/typeck/while-loop-block-cond.stderr

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/while-loop-block-cond.rs:2:11
3+
|
4+
LL | while {} {}
5+
| ^^ expected `bool`, found `()`
6+
7+
error: aborting due to previous error
8+
9+
For more information about this error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)