Skip to content

Commit d84b197

Browse files
borsbors
committed
Auto merge of #39864 - cramertj:normalize-breaks, r=nikomatsakis
Normalize labeled and unlabeled breaks Part of #39849.
2 parents 9f082d2 + 6f0447b commit d84b197

File tree

19 files changed

+443
-199
lines changed

19 files changed

+443
-199
lines changed

‎src/librustc/cfg/construct.rs‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -220,15 +220,24 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
220220
// Note that `break` and `continue` statements
221221
// may cause additional edges.
222222

223-
// Is the condition considered part of the loop?
224223
let loopback = self.add_dummy_node(&[pred]); // 1
225-
let cond_exit = self.expr(&cond, loopback); // 2
226-
let expr_exit = self.add_ast_node(expr.id, &[cond_exit]); // 3
224+
225+
// Create expr_exit without pred (cond_exit)
226+
let expr_exit = self.add_ast_node(expr.id, &[]); // 3
227+
228+
// The LoopScope needs to be on the loop_scopes stack while evaluating the
229+
// condition and the body of the loop (both can break out of the loop)
227230
self.loop_scopes.push(LoopScope {
228231
loop_id: expr.id,
229232
continue_index: loopback,
230233
break_index: expr_exit
231234
});
235+
236+
let cond_exit = self.expr(&cond, loopback); // 2
237+
238+
// Add pred (cond_exit) to expr_exit
239+
self.add_contained_edge(cond_exit, expr_exit);
240+
232241
let body_exit = self.block(&body, cond_exit); // 4
233242
self.add_contained_edge(body_exit, loopback); // 5
234243
self.loop_scopes.pop();
@@ -294,17 +303,17 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
294303
self.add_unreachable_node()
295304
}
296305

297-
hir::ExprBreak(label, ref opt_expr) => {
306+
hir::ExprBreak(destination, ref opt_expr) => {
298307
let v = self.opt_expr(opt_expr, pred);
299-
let loop_scope = self.find_scope(expr, label);
308+
let loop_scope = self.find_scope(expr, destination);
300309
let b = self.add_ast_node(expr.id, &[v]);
301310
self.add_exiting_edge(expr, b,
302311
loop_scope, loop_scope.break_index);
303312
self.add_unreachable_node()
304313
}
305314

306-
hir::ExprAgain(label) => {
307-
let loop_scope = self.find_scope(expr, label);
315+
hir::ExprAgain(destination) => {
316+
let loop_scope = self.find_scope(expr, destination);
308317
let a = self.add_ast_node(expr.id, &[pred]);
309318
self.add_exiting_edge(expr, a,
310319
loop_scope, loop_scope.continue_index);
@@ -579,17 +588,18 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
579588

580589
fn find_scope(&self,
581590
expr: &hir::Expr,
582-
label: Option<hir::Label>) -> LoopScope {
583-
match label {
584-
None => *self.loop_scopes.last().unwrap(),
585-
Some(label) => {
591+
destination: hir::Destination) -> LoopScope {
592+
593+
match destination.loop_id.into() {
594+
Ok(loop_id) => {
586595
for l in &self.loop_scopes {
587-
if l.loop_id == label.loop_id {
596+
if l.loop_id == loop_id {
588597
return *l;
589598
}
590599
}
591-
span_bug!(expr.span, "no loop scope for id {}", label.loop_id);
600+
span_bug!(expr.span, "no loop scope for id {}", loop_id);
592601
}
602+
Err(err) => span_bug!(expr.span, "loop scope error: {}", err)
593603
}
594604
}
595605
}

‎src/librustc/hir/intravisit.rs‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,18 +1006,22 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
10061006
ExprPath(ref qpath) => {
10071007
visitor.visit_qpath(qpath, expression.id, expression.span);
10081008
}
1009-
ExprBreak(None, ref opt_expr) => {
1009+
ExprBreak(label, ref opt_expr) => {
1010+
label.ident.map(|ident| {
1011+
if let Ok(loop_id) = label.loop_id.into() {
1012+
visitor.visit_def_mention(Def::Label(loop_id));
1013+
}
1014+
visitor.visit_name(ident.span, ident.node.name);
1015+
});
10101016
walk_list!(visitor, visit_expr, opt_expr);
10111017
}
1012-
ExprBreak(Some(label), ref opt_expr) => {
1013-
visitor.visit_def_mention(Def::Label(label.loop_id));
1014-
visitor.visit_name(label.span, label.name);
1015-
walk_list!(visitor, visit_expr, opt_expr);
1016-
}
1017-
ExprAgain(None) => {}
1018-
ExprAgain(Some(label)) => {
1019-
visitor.visit_def_mention(Def::Label(label.loop_id));
1020-
visitor.visit_name(label.span, label.name);
1018+
ExprAgain(label) => {
1019+
label.ident.map(|ident| {
1020+
if let Ok(loop_id) = label.loop_id.into() {
1021+
visitor.visit_def_mention(Def::Label(loop_id));
1022+
}
1023+
visitor.visit_name(ident.span, ident.node.name);
1024+
});
10211025
}
10221026
ExprRet(ref optional_expression) => {
10231027
walk_list!(visitor, visit_expr, optional_expression);

‎src/librustc/hir/lowering.rs‎

Lines changed: 134 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ use util::nodemap::{DefIdMap, NodeMap, FxHashMap};
5050

5151
use std::collections::BTreeMap;
5252
use std::iter;
53+
use std::mem;
5354

5455
use syntax::attr;
5556
use syntax::ast::*;
@@ -79,6 +80,9 @@ pub struct LoweringContext<'a> {
7980
impl_items: BTreeMap<hir::ImplItemId, hir::ImplItem>,
8081
bodies: FxHashMap<hir::BodyId, hir::Body>,
8182

83+
loop_scopes: Vec<NodeId>,
84+
is_in_loop_condition: bool,
85+
8286
type_def_lifetime_params: DefIdMap<usize>,
8387
}
8488

@@ -112,6 +116,8 @@ pub fn lower_crate(sess: &Session,
112116
trait_items: BTreeMap::new(),
113117
impl_items: BTreeMap::new(),
114118
bodies: FxHashMap(),
119+
loop_scopes: Vec::new(),
120+
is_in_loop_condition: false,
115121
type_def_lifetime_params: DefIdMap(),
116122
}.lower_crate(krate)
117123
}
@@ -244,6 +250,55 @@ impl<'a> LoweringContext<'a> {
244250
span
245251
}
246252

253+
fn with_loop_scope<T, F>(&mut self, loop_id: NodeId, f: F) -> T
254+
where F: FnOnce(&mut LoweringContext) -> T
255+
{
256+
// We're no longer in the base loop's condition; we're in another loop.
257+
let was_in_loop_condition = self.is_in_loop_condition;
258+
self.is_in_loop_condition = false;
259+
260+
let len = self.loop_scopes.len();
261+
self.loop_scopes.push(loop_id);
262+
263+
let result = f(self);
264+
assert_eq!(len + 1, self.loop_scopes.len(),
265+
"Loop scopes should be added and removed in stack order");
266+
267+
self.loop_scopes.pop().unwrap();
268+
269+
self.is_in_loop_condition = was_in_loop_condition;
270+
271+
result
272+
}
273+
274+
fn with_loop_condition_scope<T, F>(&mut self, f: F) -> T
275+
where F: FnOnce(&mut LoweringContext) -> T
276+
{
277+
let was_in_loop_condition = self.is_in_loop_condition;
278+
self.is_in_loop_condition = true;
279+
280+
let result = f(self);
281+
282+
self.is_in_loop_condition = was_in_loop_condition;
283+
284+
result
285+
}
286+
287+
fn with_new_loop_scopes<T, F>(&mut self, f: F) -> T
288+
where F: FnOnce(&mut LoweringContext) -> T
289+
{
290+
let was_in_loop_condition = self.is_in_loop_condition;
291+
self.is_in_loop_condition = false;
292+
293+
let loop_scopes = mem::replace(&mut self.loop_scopes, Vec::new());
294+
let result = f(self);
295+
mem::replace(&mut self.loop_scopes, loop_scopes);
296+
297+
self.is_in_loop_condition = was_in_loop_condition;
298+
299+
result
300+
}
301+
247302
fn with_parent_def<T, F>(&mut self, parent_id: NodeId, f: F) -> T
248303
where F: FnOnce(&mut LoweringContext) -> T
249304
{
@@ -271,17 +326,24 @@ impl<'a> LoweringContext<'a> {
271326
o_id.map(|sp_ident| respan(sp_ident.span, sp_ident.node.name))
272327
}
273328

274-
fn lower_label(&mut self, id: NodeId, label: Option<Spanned<Ident>>) -> Option<hir::Label> {
275-
label.map(|sp_ident| {
276-
hir::Label {
277-
span: sp_ident.span,
278-
name: sp_ident.node.name,
279-
loop_id: match self.expect_full_def(id) {
280-
Def::Label(loop_id) => loop_id,
281-
_ => DUMMY_NODE_ID
329+
fn lower_destination(&mut self, destination: Option<(NodeId, Spanned<Ident>)>)
330+
-> hir::Destination
331+
{
332+
match destination {
333+
Some((id, label_ident)) => hir::Destination {
334+
ident: Some(label_ident),
335+
loop_id: if let Def::Label(loop_id) = self.expect_full_def(id) {
336+
hir::LoopIdResult::Ok(loop_id)
337+
} else {
338+
hir::LoopIdResult::Err(hir::LoopIdError::UnresolvedLabel)
282339
}
340+
},
341+
None => hir::Destination {
342+
ident: None,
343+
loop_id: self.loop_scopes.last().map(|innermost_loop_id| Ok(*innermost_loop_id))
344+
.unwrap_or(Err(hir::LoopIdError::OutsideLoopScope)).into()
283345
}
284-
})
346+
}
285347
}
286348

287349
fn lower_attrs(&mut self, attrs: &Vec<Attribute>) -> hir::HirVec<Attribute> {
@@ -992,15 +1054,17 @@ impl<'a> LoweringContext<'a> {
9921054
self.record_body(value, None))
9931055
}
9941056
ItemKind::Fn(ref decl, unsafety, constness, abi, ref generics, ref body) => {
995-
let body = self.lower_block(body);
996-
let body = self.expr_block(body, ThinVec::new());
997-
let body_id = self.record_body(body, Some(decl));
998-
hir::ItemFn(self.lower_fn_decl(decl),
999-
self.lower_unsafety(unsafety),
1000-
self.lower_constness(constness),
1001-
abi,
1002-
self.lower_generics(generics),
1003-
body_id)
1057+
self.with_new_loop_scopes(|this| {
1058+
let body = this.lower_block(body);
1059+
let body = this.expr_block(body, ThinVec::new());
1060+
let body_id = this.record_body(body, Some(decl));
1061+
hir::ItemFn(this.lower_fn_decl(decl),
1062+
this.lower_unsafety(unsafety),
1063+
this.lower_constness(constness),
1064+
abi,
1065+
this.lower_generics(generics),
1066+
body_id)
1067+
})
10041068
}
10051069
ItemKind::Mod(ref m) => hir::ItemMod(self.lower_mod(m)),
10061070
ItemKind::ForeignMod(ref nm) => hir::ItemForeignMod(self.lower_foreign_mod(nm)),
@@ -1562,26 +1626,32 @@ impl<'a> LoweringContext<'a> {
15621626
hir::ExprIf(P(self.lower_expr(cond)), self.lower_block(blk), else_opt)
15631627
}
15641628
ExprKind::While(ref cond, ref body, opt_ident) => {
1565-
hir::ExprWhile(P(self.lower_expr(cond)), self.lower_block(body),
1566-
self.lower_opt_sp_ident(opt_ident))
1629+
self.with_loop_scope(e.id, |this|
1630+
hir::ExprWhile(
1631+
this.with_loop_condition_scope(|this| P(this.lower_expr(cond))),
1632+
this.lower_block(body),
1633+
this.lower_opt_sp_ident(opt_ident)))
15671634
}
15681635
ExprKind::Loop(ref body, opt_ident) => {
1569-
hir::ExprLoop(self.lower_block(body),
1570-
self.lower_opt_sp_ident(opt_ident),
1571-
hir::LoopSource::Loop)
1636+
self.with_loop_scope(e.id, |this|
1637+
hir::ExprLoop(this.lower_block(body),
1638+
this.lower_opt_sp_ident(opt_ident),
1639+
hir::LoopSource::Loop))
15721640
}
15731641
ExprKind::Match(ref expr, ref arms) => {
15741642
hir::ExprMatch(P(self.lower_expr(expr)),
15751643
arms.iter().map(|x| self.lower_arm(x)).collect(),
15761644
hir::MatchSource::Normal)
15771645
}
15781646
ExprKind::Closure(capture_clause, ref decl, ref body, fn_decl_span) => {
1579-
self.with_parent_def(e.id, |this| {
1580-
let expr = this.lower_expr(body);
1581-
hir::ExprClosure(this.lower_capture_clause(capture_clause),
1582-
this.lower_fn_decl(decl),
1583-
this.record_body(expr, Some(decl)),
1584-
fn_decl_span)
1647+
self.with_new_loop_scopes(|this| {
1648+
this.with_parent_def(e.id, |this| {
1649+
let expr = this.lower_expr(body);
1650+
hir::ExprClosure(this.lower_capture_clause(capture_clause),
1651+
this.lower_fn_decl(decl),
1652+
this.record_body(expr, Some(decl)),
1653+
fn_decl_span)
1654+
})
15851655
})
15861656
}
15871657
ExprKind::Block(ref blk) => hir::ExprBlock(self.lower_block(blk)),
@@ -1660,10 +1730,29 @@ impl<'a> LoweringContext<'a> {
16601730
hir::ExprPath(self.lower_qpath(e.id, qself, path, ParamMode::Optional))
16611731
}
16621732
ExprKind::Break(opt_ident, ref opt_expr) => {
1663-
hir::ExprBreak(self.lower_label(e.id, opt_ident),
1664-
opt_expr.as_ref().map(|x| P(self.lower_expr(x))))
1733+
let label_result = if self.is_in_loop_condition && opt_ident.is_none() {
1734+
hir::Destination {
1735+
ident: opt_ident,
1736+
loop_id: Err(hir::LoopIdError::UnlabeledCfInWhileCondition).into(),
1737+
}
1738+
} else {
1739+
self.lower_destination(opt_ident.map(|ident| (e.id, ident)))
1740+
};
1741+
hir::ExprBreak(
1742+
label_result,
1743+
opt_expr.as_ref().map(|x| P(self.lower_expr(x))))
16651744
}
1666-
ExprKind::Continue(opt_ident) => hir::ExprAgain(self.lower_label(e.id, opt_ident)),
1745+
ExprKind::Continue(opt_ident) =>
1746+
hir::ExprAgain(
1747+
if self.is_in_loop_condition && opt_ident.is_none() {
1748+
hir::Destination {
1749+
ident: opt_ident,
1750+
loop_id: Err(
1751+
hir::LoopIdError::UnlabeledCfInWhileCondition).into(),
1752+
}
1753+
} else {
1754+
self.lower_destination(opt_ident.map( |ident| (e.id, ident)))
1755+
}),
16671756
ExprKind::Ret(ref e) => hir::ExprRet(e.as_ref().map(|x| P(self.lower_expr(x)))),
16681757
ExprKind::InlineAsm(ref asm) => {
16691758
let hir_asm = hir::InlineAsm {
@@ -1804,9 +1893,16 @@ impl<'a> LoweringContext<'a> {
18041893
// }
18051894
// }
18061895

1896+
// Note that the block AND the condition are evaluated in the loop scope.
1897+
// This is done to allow `break` from inside the condition of the loop.
1898+
let (body, break_expr, sub_expr) = self.with_loop_scope(e.id, |this| (
1899+
this.lower_block(body),
1900+
this.expr_break(e.span, ThinVec::new()),
1901+
this.with_loop_condition_scope(|this| P(this.lower_expr(sub_expr))),
1902+
));
1903+
18071904
// `<pat> => <body>`
18081905
let pat_arm = {
1809-
let body = self.lower_block(body);
18101906
let body_expr = P(self.expr_block(body, ThinVec::new()));
18111907
let pat = self.lower_pat(pat);
18121908
self.arm(hir_vec![pat], body_expr)
@@ -1815,13 +1911,11 @@ impl<'a> LoweringContext<'a> {
18151911
// `_ => break`
18161912
let break_arm = {
18171913
let pat_under = self.pat_wild(e.span);
1818-
let break_expr = self.expr_break(e.span, ThinVec::new());
18191914
self.arm(hir_vec![pat_under], break_expr)
18201915
};
18211916

18221917
// `match <sub_expr> { ... }`
18231918
let arms = hir_vec![pat_arm, break_arm];
1824-
let sub_expr = P(self.lower_expr(sub_expr));
18251919
let match_expr = self.expr(e.span,
18261920
hir::ExprMatch(sub_expr,
18271921
arms,
@@ -1863,7 +1957,7 @@ impl<'a> LoweringContext<'a> {
18631957

18641958
// `::std::option::Option::Some(<pat>) => <body>`
18651959
let pat_arm = {
1866-
let body_block = self.lower_block(body);
1960+
let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body));
18671961
let body_expr = P(self.expr_block(body_block, ThinVec::new()));
18681962
let pat = self.lower_pat(pat);
18691963
let some_pat = self.pat_some(e.span, pat);
@@ -1873,7 +1967,8 @@ impl<'a> LoweringContext<'a> {
18731967

18741968
// `::std::option::Option::None => break`
18751969
let break_arm = {
1876-
let break_expr = self.expr_break(e.span, ThinVec::new());
1970+
let break_expr = self.with_loop_scope(e.id, |this|
1971+
this.expr_break(e.span, ThinVec::new()));
18771972
let pat = self.pat_none(e.span);
18781973
self.arm(hir_vec![pat], break_expr)
18791974
};
@@ -2151,7 +2246,8 @@ impl<'a> LoweringContext<'a> {
21512246
}
21522247

21532248
fn expr_break(&mut self, span: Span, attrs: ThinVec<Attribute>) -> P<hir::Expr> {
2154-
P(self.expr(span, hir::ExprBreak(None, None), attrs))
2249+
let expr_break = hir::ExprBreak(self.lower_destination(None), None);
2250+
P(self.expr(span, expr_break, attrs))
21552251
}
21562252

21572253
fn expr_call(&mut self, span: Span, e: P<hir::Expr>, args: hir::HirVec<hir::Expr>)

0 commit comments

Comments
 (0)