Skip to content

Commit 41f234d

Browse files
VeykrilVeykril
committed
Diagnose value breaks in incorrect breakables
1 parent 02eb2d7 commit 41f234d

File tree

5 files changed

+71
-38
lines changed

5 files changed

+71
-38
lines changed

‎crates/hir-ty/src/infer.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ pub enum InferenceDiagnostic {
167167
NoSuchField { expr: ExprId },
168168
PrivateField { expr: ExprId, field: FieldId },
169169
PrivateAssocItem { id: ExprOrPatId, item: AssocItemId },
170-
BreakOutsideOfLoop { expr: ExprId, is_break: bool },
170+
// FIXME: Make this proper
171+
BreakOutsideOfLoop { expr: ExprId, is_break: bool, bad_value_break: bool },
171172
MismatchedArgCount { call_expr: ExprId, expected: usize, found: usize },
172173
}
173174

@@ -411,7 +412,7 @@ struct BreakableContext {
411412
/// Whether this context contains at least one break expression.
412413
may_break: bool,
413414
/// The coercion target of the context.
414-
coerce: CoerceMany,
415+
coerce: Option<CoerceMany>,
415416
/// The optional label of the context.
416417
label: Option<name::Name>,
417418
kind: BreakableKind,

‎crates/hir-ty/src/infer/expr.rs‎

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ impl<'a> InferenceContext<'a> {
133133
let break_ty = self.table.new_type_var();
134134
let (breaks, ty) = self.with_breakable_ctx(
135135
BreakableKind::Block,
136-
break_ty.clone(),
136+
Some(break_ty.clone()),
137137
*label,
138138
|this| {
139139
this.infer_block(
@@ -153,7 +153,7 @@ impl<'a> InferenceContext<'a> {
153153
}
154154
Expr::Unsafe { body } => self.infer_expr(*body, expected),
155155
Expr::Const { body } => {
156-
self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| {
156+
self.with_breakable_ctx(BreakableKind::Border, None, None, |this| {
157157
this.infer_expr(*body, expected)
158158
})
159159
.1
@@ -169,7 +169,7 @@ impl<'a> InferenceContext<'a> {
169169
let ok_ty =
170170
self.resolve_associated_type(try_ty.clone(), self.resolve_ops_try_output());
171171

172-
self.with_breakable_ctx(BreakableKind::Block, ok_ty.clone(), None, |this| {
172+
self.with_breakable_ctx(BreakableKind::Block, Some(ok_ty.clone()), None, |this| {
173173
this.infer_expr(*body, &Expectation::has_type(ok_ty));
174174
});
175175

@@ -183,7 +183,7 @@ impl<'a> InferenceContext<'a> {
183183
mem::replace(&mut self.return_coercion, Some(CoerceMany::new(ret_ty.clone())));
184184

185185
let (_, inner_ty) =
186-
self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| {
186+
self.with_breakable_ctx(BreakableKind::Border, None, None, |this| {
187187
this.infer_expr_coerce(*body, &Expectation::has_type(ret_ty))
188188
});
189189

@@ -203,7 +203,7 @@ impl<'a> InferenceContext<'a> {
203203
// let ty = expected.coercion_target_type(&mut self.table);
204204
let ty = self.table.new_type_var();
205205
let (breaks, ()) =
206-
self.with_breakable_ctx(BreakableKind::Loop, ty, label, |this| {
206+
self.with_breakable_ctx(BreakableKind::Loop, Some(ty), label, |this| {
207207
this.infer_expr(body, &Expectation::HasType(TyBuilder::unit()));
208208
});
209209

@@ -216,7 +216,7 @@ impl<'a> InferenceContext<'a> {
216216
}
217217
}
218218
&Expr::While { condition, body, label } => {
219-
self.with_breakable_ctx(BreakableKind::Loop, self.err_ty(), label, |this| {
219+
self.with_breakable_ctx(BreakableKind::Loop, None, label, |this| {
220220
this.infer_expr(
221221
condition,
222222
&Expectation::HasType(this.result.standard_types.bool_.clone()),
@@ -236,7 +236,7 @@ impl<'a> InferenceContext<'a> {
236236
self.resolve_associated_type(into_iter_ty, self.resolve_iterator_item());
237237

238238
self.infer_top_pat(pat, &pat_ty);
239-
self.with_breakable_ctx(BreakableKind::Loop, self.err_ty(), label, |this| {
239+
self.with_breakable_ctx(BreakableKind::Loop, None, label, |this| {
240240
this.infer_expr(body, &Expectation::HasType(TyBuilder::unit()));
241241
});
242242

@@ -321,7 +321,7 @@ impl<'a> InferenceContext<'a> {
321321
let prev_resume_yield_tys =
322322
mem::replace(&mut self.resume_yield_tys, resume_yield_tys);
323323

324-
self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| {
324+
self.with_breakable_ctx(BreakableKind::Border, None, None, |this| {
325325
this.infer_return(*body);
326326
});
327327

@@ -439,42 +439,50 @@ impl<'a> InferenceContext<'a> {
439439
self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
440440
expr: tgt_expr,
441441
is_break: false,
442+
bad_value_break: false,
442443
});
443444
};
444445
self.result.standard_types.never.clone()
445446
}
446447
Expr::Break { expr, label } => {
447448
let val_ty = if let Some(expr) = *expr {
448-
let opt_coerce_to = find_breakable(&mut self.breakables, label.as_ref())
449-
.map(|ctxt| ctxt.coerce.expected_ty());
450-
self.infer_expr_inner(
451-
expr,
452-
&Expectation::HasType(opt_coerce_to.unwrap_or_else(|| self.err_ty())),
453-
)
449+
let opt_coerce_to = match find_breakable(&mut self.breakables, label.as_ref()) {
450+
Some(ctxt) => match &ctxt.coerce {
451+
Some(coerce) => coerce.expected_ty(),
452+
None => {
453+
self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
454+
expr: tgt_expr,
455+
is_break: true,
456+
bad_value_break: true,
457+
});
458+
self.err_ty()
459+
}
460+
},
461+
None => self.err_ty(),
462+
};
463+
self.infer_expr_inner(expr, &Expectation::HasType(opt_coerce_to))
454464
} else {
455465
TyBuilder::unit()
456466
};
457467

458468
match find_breakable(&mut self.breakables, label.as_ref()) {
459-
Some(ctxt) => {
460-
// avoiding the borrowck
461-
let mut coerce = mem::replace(
462-
&mut ctxt.coerce,
463-
CoerceMany::new(expected.coercion_target_type(&mut self.table)),
464-
);
465-
466-
// FIXME: create a synthetic `()` during lowering so we have something to refer to here?
467-
coerce.coerce(self, *expr, &val_ty);
468-
469-
let ctxt = find_breakable(&mut self.breakables, label.as_ref())
470-
.expect("breakable stack changed during coercion");
471-
ctxt.coerce = coerce;
472-
ctxt.may_break = true;
473-
}
469+
Some(ctxt) => match ctxt.coerce.take() {
470+
Some(mut coerce) => {
471+
coerce.coerce(self, *expr, &val_ty);
472+
473+
// Avoiding borrowck
474+
let ctxt = find_breakable(&mut self.breakables, label.as_ref())
475+
.expect("breakable stack changed during coercion");
476+
ctxt.may_break = true;
477+
ctxt.coerce = Some(coerce);
478+
}
479+
None => ctxt.may_break = true,
480+
},
474481
None => {
475482
self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
476483
expr: tgt_expr,
477484
is_break: true,
485+
bad_value_break: false,
478486
});
479487
}
480488
}
@@ -1712,16 +1720,16 @@ impl<'a> InferenceContext<'a> {
17121720
fn with_breakable_ctx<T>(
17131721
&mut self,
17141722
kind: BreakableKind,
1715-
ty: Ty,
1723+
ty: Option<Ty>,
17161724
label: Option<LabelId>,
17171725
cb: impl FnOnce(&mut Self) -> T,
17181726
) -> (Option<Ty>, T) {
17191727
self.breakables.push({
17201728
let label = label.map(|label| self.body[label].name.clone());
1721-
BreakableContext { kind, may_break: false, coerce: CoerceMany::new(ty), label }
1729+
BreakableContext { kind, may_break: false, coerce: ty.map(CoerceMany::new), label }
17221730
});
17231731
let res = cb(self);
17241732
let ctx = self.breakables.pop().expect("breakable stack broken");
1725-
(ctx.may_break.then(|| ctx.coerce.complete(self)), res)
1733+
(if ctx.may_break { ctx.coerce.map(|ctx| ctx.complete(self)) } else { None }, res)
17261734
}
17271735
}

‎crates/hir/src/diagnostics.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ pub struct PrivateField {
140140
pub struct BreakOutsideOfLoop {
141141
pub expr: InFile<AstPtr<ast::Expr>>,
142142
pub is_break: bool,
143+
pub bad_value_break: bool,
143144
}
144145

145146
#[derive(Debug)]

‎crates/hir/src/lib.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,11 +1373,15 @@ impl DefWithBody {
13731373
let field = source_map.field_syntax(*expr);
13741374
acc.push(NoSuchField { field }.into())
13751375
}
1376-
&hir_ty::InferenceDiagnostic::BreakOutsideOfLoop { expr, is_break } => {
1376+
&hir_ty::InferenceDiagnostic::BreakOutsideOfLoop {
1377+
expr,
1378+
is_break,
1379+
bad_value_break,
1380+
} => {
13771381
let expr = source_map
13781382
.expr_syntax(expr)
13791383
.expect("break outside of loop in synthetic syntax");
1380-
acc.push(BreakOutsideOfLoop { expr, is_break }.into())
1384+
acc.push(BreakOutsideOfLoop { expr, is_break, bad_value_break }.into())
13811385
}
13821386
hir_ty::InferenceDiagnostic::MismatchedArgCount { call_expr, expected, found } => {
13831387
match source_map.expr_syntax(*call_expr) {

‎crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,15 @@ pub(crate) fn break_outside_of_loop(
77
ctx: &DiagnosticsContext<'_>,
88
d: &hir::BreakOutsideOfLoop,
99
) -> Diagnostic {
10-
let construct = if d.is_break { "break" } else { "continue" };
10+
let message = if d.bad_value_break {
11+
"can't break with a value in this position".to_owned()
12+
} else {
13+
let construct = if d.is_break { "break" } else { "continue" };
14+
format!("{construct} outside of loop")
15+
};
1116
Diagnostic::new(
1217
"break-outside-of-loop",
13-
format!("{construct} outside of loop"),
18+
message,
1419
ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range,
1520
)
1621
}
@@ -132,6 +137,20 @@ fn foo() {
132137
//^^^^^^^^^^^ error: continue outside of loop
133138
}
134139
}
140+
"#,
141+
);
142+
}
143+
144+
#[test]
145+
fn value_break_in_for_loop() {
146+
check_diagnostics(
147+
r#"
148+
fn test() {
149+
for _ in [()] {
150+
break 3;
151+
// ^^^^^^^ error: can't break with a value in this position
152+
}
153+
}
135154
"#,
136155
);
137156
}

0 commit comments

Comments
 (0)