Skip to content

Point at more causes of expectation of break value when possible #116080

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.

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 5 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
86 changes: 77 additions & 9 deletions compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// When encountering a type error on the value of a `break`, try to point at the reason for the
// expected type.
fn annotate_loop_expected_due_to_inference(
pub fn annotate_loop_expected_due_to_inference(
&self,
err: &mut Diagnostic,
expr: &hir::Expr<'_>,
Expand All @@ -540,16 +540,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return;
};
let mut parent_id = self.tcx.hir().parent_id(expr.hir_id);
loop {
let mut parent;
'outer: loop {
// Climb the HIR tree to see if the current `Expr` is part of a `break;` statement.
let Some(hir::Node::Expr(parent)) = self.tcx.hir().find(parent_id) else {
let Some(
hir::Node::Stmt(hir::Stmt { kind: hir::StmtKind::Semi(&ref p), .. })
| hir::Node::Expr(&ref p),
) = self.tcx.hir().find(parent_id)
else {
break;
};
parent_id = self.tcx.hir().parent_id(parent.hir_id);
parent = p;
parent_id = self.tcx.hir().parent_id(parent_id);
let hir::ExprKind::Break(destination, _) = parent.kind else {
continue;
};
let mut parent_id = parent.hir_id;
let mut parent_id = parent_id;
let mut direct = false;
loop {
// Climb the HIR tree to find the (desugared) `loop` this `break` corresponds to.
let parent = match self.tcx.hir().find(parent_id) {
Expand All @@ -565,14 +572,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
parent_id = self.tcx.hir().parent_id(*hir_id);
parent
}
Some(hir::Node::Block(hir::Block { .. })) => {
Some(hir::Node::Block(_)) => {
parent_id = self.tcx.hir().parent_id(parent_id);
parent
}
_ => break,
};
if let hir::ExprKind::Loop(_, label, _, span) = parent.kind
&& destination.label == label
if let hir::ExprKind::Loop(..) = parent.kind {
// When you have `'a: loop { break; }`, the `break` corresponds to the labeled
// loop, so we need to account for that.
direct = !direct;
}
if let hir::ExprKind::Loop(block, label, _, span) = parent.kind
&& (destination.label == label || direct)
{
if let Some((reason_span, message)) =
self.maybe_get_coercion_reason(parent_id, parent.span)
Expand All @@ -582,8 +594,64 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
span,
format!("this loop is expected to be of type `{expected}`"),
);
break 'outer;
} else {
// Locate all other `break` statements within the same `loop` that might
// have affected inference.
struct FindBreaks<'tcx> {
label: Option<rustc_ast::Label>,
uses: Vec<&'tcx hir::Expr<'tcx>>,
nest_depth: usize,
}
impl<'tcx> Visitor<'tcx> for FindBreaks<'tcx> {
fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) {
let nest_depth = self.nest_depth;
if let hir::ExprKind::Loop(_, label, _, _) = ex.kind {
if label == self.label {
// Account for `'a: loop { 'a: loop {...} }`.
return;
}
self.nest_depth += 1;
}
if let hir::ExprKind::Break(destination, _) = ex.kind
&& (self.label == destination.label
// Account for `loop { 'a: loop { loop { break; } } }`.
|| destination.label.is_none() && self.nest_depth == 0)
{
self.uses.push(ex);
}
hir::intravisit::walk_expr(self, ex);
self.nest_depth = nest_depth;
}
}
let mut expr_finder = FindBreaks { label, uses: vec![], nest_depth: 0 };
expr_finder.visit_block(block);
let mut exit = false;
for ex in expr_finder.uses {
let hir::ExprKind::Break(_, val) = ex.kind else {
continue;
};
let ty = match val {
Some(val) => {
match self.typeck_results.borrow().expr_ty_adjusted_opt(val) {
None => continue,
Some(ty) => ty,
}
}
None => self.tcx.types.unit,
};
if self.can_eq(self.param_env, ty, expected) {
err.span_label(
ex.span,
format!("expected because of this `break`"),
);
exit = true;
}
}
if exit {
break 'outer;
}
}
break;
}
}
}
Expand Down
19 changes: 10 additions & 9 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ use rustc_infer::traits::query::NoSolution;
use rustc_infer::traits::ObligationCause;
use rustc_middle::middle::stability;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AllowTwoPhase};
use rustc_middle::ty::error::TypeError::FieldMisMatch;
use rustc_middle::ty::error::{
ExpectedFound,
TypeError::{FieldMisMatch, Sorts},
};
use rustc_middle::ty::GenericArgsRef;
use rustc_middle::ty::{self, AdtKind, Ty, TypeVisitableExt};
use rustc_session::errors::ExprParenthesesNeeded;
Expand Down Expand Up @@ -664,15 +667,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.suggest_mismatched_types_on_tail(
&mut err, expr, ty, e_ty, target_id,
);
let error = Some(Sorts(ExpectedFound { expected: ty, found: e_ty }));
self.annotate_loop_expected_due_to_inference(&mut err, expr, error);
if let Some(val) = ty_kind_suggestion(ty) {
let label = destination
.label
.map(|l| format!(" {}", l.ident))
.unwrap_or_else(String::new);
err.span_suggestion(
expr.span,
err.span_suggestion_verbose(
expr.span.shrink_to_hi(),
"give it a value of the expected type",
format!("break{label} {val}"),
format!(" {val}"),
Applicability::HasPlaceholders,
);
}
Expand Down Expand Up @@ -717,7 +718,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// ... except when we try to 'break rust;'.
// ICE this expression in particular (see #43162).
if let ExprKind::Path(QPath::Resolved(_, path)) = e.kind {
if path.segments.len() == 1 && path.segments[0].ident.name == sym::rust {
if let [segment] = path.segments && segment.ident.name == sym::rust {
fatally_break_rust(self.tcx);
}
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let expr = expr.peel_drop_temps();
self.suggest_missing_semicolon(err, expr, expected, false);
let mut pointing_at_return_type = false;
if let hir::ExprKind::Break(..) = expr.kind {
// `break` type mismatches provide better context for tail `loop` expressions.
return false;
}
if let Some((fn_id, fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
pointing_at_return_type = self.suggest_missing_return_type(
err,
Expand Down
10 changes: 6 additions & 4 deletions tests/ui/issues/issue-27042.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ error[E0308]: mismatched types
--> $DIR/issue-27042.rs:6:16
|
LL | loop { break };
| ^^^^^
| |
| expected `i32`, found `()`
| help: give it a value of the expected type: `break 42`
| ^^^^^ expected `i32`, found `()`
|
help: give it a value of the expected type
|
LL | loop { break 42 };
| ++

error[E0308]: mismatched types
--> $DIR/issue-27042.rs:8:9
Expand Down
60 changes: 60 additions & 0 deletions tests/ui/loops/loop-break-value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,66 @@ fn main() {
break LOOP;
//~^ ERROR cannot find value `LOOP` in this scope
}

let _ = 'a: loop {
loop {
break; // This doesn't affect the expected break type of the 'a loop
loop {
loop {
break 'a 1;
}
}
}
break; //~ ERROR mismatched types
};

let _ = 'a: loop {
loop {
break; // This doesn't affect the expected break type of the 'a loop
loop {
loop {
break 'a 1;
}
}
}
break 'a; //~ ERROR mismatched types
};

loop {
break;
let _ = loop {
break 2;
loop {
break;
}
};
break 2; //~ ERROR mismatched types
}

'a: loop {
break;
let _ = 'a: loop {
//~^ WARNING label name `'a` shadows a label name that is already in scope
break 2;
loop {
break 'a; //~ ERROR mismatched types
}
};
break 2; //~ ERROR mismatched types
}

'a: loop {
break;
let _ = 'a: loop {
//~^ WARNING label name `'a` shadows a label name that is already in scope
break 'a 2;
loop {
break 'a; //~ ERROR mismatched types
}
};
break 2; //~ ERROR mismatched types
};

loop { // point at the return type
break 2; //~ ERROR mismatched types
}
Expand Down
Loading