Skip to content

Commit 5205e2f

Browse files
cramertjcramertj
committed
Normalize labeled and unlabeled breaks
1 parent 48bc082 commit 5205e2f

File tree

13 files changed

+138
-139
lines changed

13 files changed

+138
-139
lines changed

‎src/librustc/cfg/construct.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -579,17 +579,12 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
579579

580580
fn find_scope(&self,
581581
expr: &hir::Expr,
582-
label: Option<hir::Label>) -> LoopScope {
583-
match label {
584-
None => *self.loop_scopes.last().unwrap(),
585-
Some(label) => {
586-
for l in &self.loop_scopes {
587-
if l.loop_id == label.loop_id {
588-
return *l;
589-
}
590-
}
591-
span_bug!(expr.span, "no loop scope for id {}", label.loop_id);
582+
label: hir::Label) -> LoopScope {
583+
for l in &self.loop_scopes {
584+
if l.loop_id == label.loop_id {
585+
return *l;
592586
}
593587
}
588+
span_bug!(expr.span, "no loop scope for id {}", label.loop_id);
594589
}
595590
}

‎src/librustc/hir/intravisit.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,18 +1006,18 @@ 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+
visitor.visit_def_mention(Def::Label(label.loop_id));
1012+
visitor.visit_name(ident.span, ident.node.name);
1013+
});
10101014
walk_list!(visitor, visit_expr, opt_expr);
10111015
}
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);
1016+
ExprAgain(label) => {
1017+
label.ident.map(|ident| {
1018+
visitor.visit_def_mention(Def::Label(label.loop_id));
1019+
visitor.visit_name(ident.span, ident.node.name);
1020+
});
10211021
}
10221022
ExprRet(ref optional_expression) => {
10231023
walk_list!(visitor, visit_expr, optional_expression);

‎src/librustc/hir/lowering.rs

Lines changed: 81 additions & 34 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,8 @@ 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+
8285
type_def_lifetime_params: DefIdMap<usize>,
8386
}
8487

@@ -112,6 +115,7 @@ pub fn lower_crate(sess: &Session,
112115
trait_items: BTreeMap::new(),
113116
impl_items: BTreeMap::new(),
114117
bodies: FxHashMap(),
118+
loop_scopes: Vec::new(),
115119
type_def_lifetime_params: DefIdMap(),
116120
}.lower_crate(krate)
117121
}
@@ -244,6 +248,27 @@ impl<'a> LoweringContext<'a> {
244248
span
245249
}
246250

251+
fn with_loop_scope<T, F>(&mut self, loop_id: NodeId, f: F) -> T
252+
where F: FnOnce(&mut LoweringContext) -> T
253+
{
254+
let len = self.loop_scopes.len();
255+
self.loop_scopes.push(loop_id);
256+
let result = f(self);
257+
assert_eq!(len + 1, self.loop_scopes.len(),
258+
"Loop scopes should be added and removed in stack order");
259+
self.loop_scopes.pop().unwrap();
260+
result
261+
}
262+
263+
fn with_new_loop_scopes<T, F>(&mut self, f: F) -> T
264+
where F: FnOnce(&mut LoweringContext) -> T
265+
{
266+
let loop_scopes = mem::replace(&mut self.loop_scopes, Vec::new());
267+
let result = f(self);
268+
mem::replace(&mut self.loop_scopes, loop_scopes);
269+
result
270+
}
271+
247272
fn with_parent_def<T, F>(&mut self, parent_id: NodeId, f: F) -> T
248273
where F: FnOnce(&mut LoweringContext) -> T
249274
{
@@ -271,17 +296,23 @@ impl<'a> LoweringContext<'a> {
271296
o_id.map(|sp_ident| respan(sp_ident.span, sp_ident.node.name))
272297
}
273298

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,
299+
fn lower_label(&mut self, label: Option<(NodeId, Spanned<Ident>)>) -> hir::Label {
300+
match label {
301+
Some((id, label_ident)) => hir::Label {
302+
ident: Some(label_ident),
279303
loop_id: match self.expect_full_def(id) {
280304
Def::Label(loop_id) => loop_id,
281305
_ => DUMMY_NODE_ID
282306
}
307+
},
308+
None => hir::Label {
309+
ident: None,
310+
loop_id: match self.loop_scopes.last() {
311+
Some(innermost_loop_id) => *innermost_loop_id,
312+
_ => DUMMY_NODE_ID
313+
}
283314
}
284-
})
315+
}
285316
}
286317

287318
fn lower_attrs(&mut self, attrs: &Vec<Attribute>) -> hir::HirVec<Attribute> {
@@ -992,15 +1023,17 @@ impl<'a> LoweringContext<'a> {
9921023
self.record_body(value, None))
9931024
}
9941025
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)
1026+
self.with_new_loop_scopes(|this| {
1027+
let body = this.lower_block(body);
1028+
let body = this.expr_block(body, ThinVec::new());
1029+
let body_id = this.record_body(body, Some(decl));
1030+
hir::ItemFn(this.lower_fn_decl(decl),
1031+
this.lower_unsafety(unsafety),
1032+
this.lower_constness(constness),
1033+
abi,
1034+
this.lower_generics(generics),
1035+
body_id)
1036+
})
10041037
}
10051038
ItemKind::Mod(ref m) => hir::ItemMod(self.lower_mod(m)),
10061039
ItemKind::ForeignMod(ref nm) => hir::ItemForeignMod(self.lower_foreign_mod(nm)),
@@ -1562,26 +1595,32 @@ impl<'a> LoweringContext<'a> {
15621595
hir::ExprIf(P(self.lower_expr(cond)), self.lower_block(blk), else_opt)
15631596
}
15641597
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))
1598+
self.with_loop_scope(e.id, |this|
1599+
hir::ExprWhile(
1600+
P(this.lower_expr(cond)),
1601+
this.lower_block(body),
1602+
this.lower_opt_sp_ident(opt_ident)))
15671603
}
15681604
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)
1605+
self.with_loop_scope(e.id, |this|
1606+
hir::ExprLoop(this.lower_block(body),
1607+
this.lower_opt_sp_ident(opt_ident),
1608+
hir::LoopSource::Loop))
15721609
}
15731610
ExprKind::Match(ref expr, ref arms) => {
15741611
hir::ExprMatch(P(self.lower_expr(expr)),
15751612
arms.iter().map(|x| self.lower_arm(x)).collect(),
15761613
hir::MatchSource::Normal)
15771614
}
15781615
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)
1616+
self.with_new_loop_scopes(|this| {
1617+
this.with_parent_def(e.id, |this| {
1618+
let expr = this.lower_expr(body);
1619+
hir::ExprClosure(this.lower_capture_clause(capture_clause),
1620+
this.lower_fn_decl(decl),
1621+
this.record_body(expr, Some(decl)),
1622+
fn_decl_span)
1623+
})
15851624
})
15861625
}
15871626
ExprKind::Block(ref blk) => hir::ExprBlock(self.lower_block(blk)),
@@ -1660,10 +1699,13 @@ impl<'a> LoweringContext<'a> {
16601699
hir::ExprPath(self.lower_qpath(e.id, qself, path, ParamMode::Optional))
16611700
}
16621701
ExprKind::Break(opt_ident, ref opt_expr) => {
1663-
hir::ExprBreak(self.lower_label(e.id, opt_ident),
1702+
hir::ExprBreak(
1703+
self.lower_label(opt_ident.map(|ident| (e.id, ident))),
16641704
opt_expr.as_ref().map(|x| P(self.lower_expr(x))))
16651705
}
1666-
ExprKind::Continue(opt_ident) => hir::ExprAgain(self.lower_label(e.id, opt_ident)),
1706+
ExprKind::Continue(opt_ident) =>
1707+
hir::ExprAgain(
1708+
self.lower_label(opt_ident.map(|ident| (e.id, ident)))),
16671709
ExprKind::Ret(ref e) => hir::ExprRet(e.as_ref().map(|x| P(self.lower_expr(x)))),
16681710
ExprKind::InlineAsm(ref asm) => {
16691711
let hir_asm = hir::InlineAsm {
@@ -1804,9 +1846,14 @@ impl<'a> LoweringContext<'a> {
18041846
// }
18051847
// }
18061848

1849+
let (body, break_expr, sub_expr) = self.with_loop_scope(e.id, |this| (
1850+
this.lower_block(body),
1851+
this.expr_break(e.span, ThinVec::new()),
1852+
P(this.lower_expr(sub_expr)),
1853+
));
1854+
18071855
// `<pat> => <body>`
18081856
let pat_arm = {
1809-
let body = self.lower_block(body);
18101857
let body_expr = P(self.expr_block(body, ThinVec::new()));
18111858
let pat = self.lower_pat(pat);
18121859
self.arm(hir_vec![pat], body_expr)
@@ -1815,13 +1862,11 @@ impl<'a> LoweringContext<'a> {
18151862
// `_ => break`
18161863
let break_arm = {
18171864
let pat_under = self.pat_wild(e.span);
1818-
let break_expr = self.expr_break(e.span, ThinVec::new());
18191865
self.arm(hir_vec![pat_under], break_expr)
18201866
};
18211867

18221868
// `match <sub_expr> { ... }`
18231869
let arms = hir_vec![pat_arm, break_arm];
1824-
let sub_expr = P(self.lower_expr(sub_expr));
18251870
let match_expr = self.expr(e.span,
18261871
hir::ExprMatch(sub_expr,
18271872
arms,
@@ -1863,7 +1908,7 @@ impl<'a> LoweringContext<'a> {
18631908

18641909
// `::std::option::Option::Some(<pat>) => <body>`
18651910
let pat_arm = {
1866-
let body_block = self.lower_block(body);
1911+
let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body));
18671912
let body_expr = P(self.expr_block(body_block, ThinVec::new()));
18681913
let pat = self.lower_pat(pat);
18691914
let some_pat = self.pat_some(e.span, pat);
@@ -1873,7 +1918,8 @@ impl<'a> LoweringContext<'a> {
18731918

18741919
// `::std::option::Option::None => break`
18751920
let break_arm = {
1876-
let break_expr = self.expr_break(e.span, ThinVec::new());
1921+
let break_expr = self.with_loop_scope(e.id, |this|
1922+
this.expr_break(e.span, ThinVec::new()));
18771923
let pat = self.pat_none(e.span);
18781924
self.arm(hir_vec![pat], break_expr)
18791925
};
@@ -2151,7 +2197,8 @@ impl<'a> LoweringContext<'a> {
21512197
}
21522198

21532199
fn expr_break(&mut self, span: Span, attrs: ThinVec<Attribute>) -> P<hir::Expr> {
2154-
P(self.expr(span, hir::ExprBreak(None, None), attrs))
2200+
let expr_break = hir::ExprBreak(self.lower_label(None), None);
2201+
P(self.expr(span, expr_break, attrs))
21552202
}
21562203

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

‎src/librustc/hir/mod.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ use util::nodemap::{NodeMap, FxHashMap, FxHashSet};
3636
use syntax_pos::{Span, ExpnId, DUMMY_SP};
3737
use syntax::codemap::{self, Spanned};
3838
use syntax::abi::Abi;
39-
use syntax::ast::{Name, NodeId, DUMMY_NODE_ID, AsmDialect};
39+
use syntax::ast::{Ident, Name, NodeId, DUMMY_NODE_ID, AsmDialect};
4040
use syntax::ast::{Attribute, Lit, StrStyle, FloatTy, IntTy, UintTy, MetaItem};
4141
use syntax::ptr::P;
4242
use syntax::symbol::{Symbol, keywords};
@@ -959,9 +959,9 @@ pub enum Expr_ {
959959
/// A referencing operation (`&a` or `&mut a`)
960960
ExprAddrOf(Mutability, P<Expr>),
961961
/// A `break`, with an optional label to break
962-
ExprBreak(Option<Label>, Option<P<Expr>>),
962+
ExprBreak(Label, Option<P<Expr>>),
963963
/// A `continue`, with an optional label
964-
ExprAgain(Option<Label>),
964+
ExprAgain(Label),
965965
/// A `return`, with an optional value to be returned
966966
ExprRet(Option<P<Expr>>),
967967

@@ -1033,8 +1033,7 @@ pub enum LoopSource {
10331033

10341034
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
10351035
pub struct Label {
1036-
pub span: Span,
1037-
pub name: Name,
1036+
pub ident: Option<Spanned<Ident>>,
10381037
pub loop_id: NodeId
10391038
}
10401039

‎src/librustc/hir/print.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1354,23 +1354,23 @@ impl<'a> State<'a> {
13541354
hir::ExprPath(ref qpath) => {
13551355
self.print_qpath(qpath, true)?
13561356
}
1357-
hir::ExprBreak(opt_label, ref opt_expr) => {
1357+
hir::ExprBreak(label, ref opt_expr) => {
13581358
word(&mut self.s, "break")?;
13591359
space(&mut self.s)?;
1360-
if let Some(label) = opt_label {
1361-
self.print_name(label.name)?;
1360+
if let Some(label_ident) = label.ident {
1361+
self.print_name(label_ident.node.name)?;
13621362
space(&mut self.s)?;
13631363
}
13641364
if let Some(ref expr) = *opt_expr {
13651365
self.print_expr(expr)?;
13661366
space(&mut self.s)?;
13671367
}
13681368
}
1369-
hir::ExprAgain(opt_label) => {
1369+
hir::ExprAgain(label) => {
13701370
word(&mut self.s, "continue")?;
13711371
space(&mut self.s)?;
1372-
if let Some(label) = opt_label {
1373-
self.print_name(label.name)?;
1372+
if let Some(label_ident) = label.ident {
1373+
self.print_name(label_ident.node.name)?;
13741374
space(&mut self.s)?
13751375
}
13761376
}

‎src/librustc/middle/liveness.rs

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -675,23 +675,6 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
675675
Ok(())
676676
}
677677

678-
fn find_loop_scope(&self,
679-
opt_label: Option<hir::Label>,
680-
sp: Span)
681-
-> NodeId {
682-
match opt_label {
683-
Some(label) => label.loop_id,
684-
None => {
685-
// Vanilla 'break' or 'continue', so use the enclosing
686-
// loop scope
687-
if self.loop_scope.is_empty() {
688-
span_bug!(sp, "break outside loop");
689-
} else {
690-
*self.loop_scope.last().unwrap()
691-
}
692-
}
693-
}
694-
}
695678

696679
#[allow(unused_must_use)]
697680
fn ln_str(&self, ln: LiveNode) -> String {
@@ -1018,9 +1001,9 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
10181001
self.propagate_through_opt_expr(o_e.as_ref().map(|e| &**e), exit_ln)
10191002
}
10201003

1021-
hir::ExprBreak(opt_label, ref opt_expr) => {
1004+
hir::ExprBreak(label, ref opt_expr) => {
10221005
// Find which label this break jumps to
1023-
let sc = self.find_loop_scope(opt_label, expr.span);
1006+
let sc = label.loop_id;
10241007

10251008
// Now that we know the label we're going to,
10261009
// look it up in the break loop nodes table
@@ -1031,9 +1014,9 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
10311014
}
10321015
}
10331016

1034-
hir::ExprAgain(opt_label) => {
1017+
hir::ExprAgain(label) => {
10351018
// Find which label this expr continues to
1036-
let sc = self.find_loop_scope(opt_label, expr.span);
1019+
let sc = label.loop_id;
10371020

10381021
// Now that we know the label we're going to,
10391022
// look it up in the continue loop nodes table

0 commit comments

Comments
 (0)