Skip to content

Commit 1cabe21

Browse files
borsbors
authored
Auto merge of #37487 - goffrie:break, r=nikomatsakis
Implement the `loop_break_value` feature. This implements RFC 1624, tracking issue #37339. - `FnCtxt` (in typeck) gets a stack of `LoopCtxt`s, which store the currently deduced type of that loop, the desired type, and a list of break expressions currently seen. `loop` loops get a fresh type variable as their initial type (this logic is stolen from that for arrays). `while` loops get `()`. - `break {expr}` looks up the broken loop, and unifies the type of `expr` with the type of the loop. - `break` with no expr unifies the loop's type with `()`. - When building MIR, loops no longer construct a `()` value at termination of the loop; rather, the `break` expression assigns the result of the loop. - ~~I have also changed the loop scoping in MIR-building so that the test of a while loop is not considered to be part of that loop. This makes the rules consistent with #37360. The new loop scopes in typeck also follow this rule. That means that `loop { while (break) {} }` now terminates instead of looping forever. This is technically a breaking change.~~ - ~~On that note, expressions like `while break {}` and `if break {}` no longer parse because `{}` is interpreted as an expression argument to `break`. But no code except compiler test cases should do that anyway because it makes no sense.~~ - The RFC did not make it clear, but I chose to make `break ()` inside of a `while` loop illegal, just in case we wanted to do anything with that design space in the future. This is my first time dealing with this part of rustc so I'm sure there's plenty of problems to pick on here ^_^
2 parents 3bf2be9 + 9d42549 commit 1cabe21

File tree

35 files changed

+641
-216
lines changed

35 files changed

+641
-216
lines changed

‎src/librustc/cfg/construct.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
223223
expr_exit
224224
}
225225

226-
hir::ExprLoop(ref body, _) => {
226+
hir::ExprLoop(ref body, _, _) => {
227227
//
228228
// [pred]
229229
// |
@@ -282,9 +282,10 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
282282
self.add_unreachable_node()
283283
}
284284

285-
hir::ExprBreak(label) => {
285+
hir::ExprBreak(label, ref opt_expr) => {
286+
let v = self.opt_expr(opt_expr, pred);
286287
let loop_scope = self.find_scope(expr, label.map(|l| l.node));
287-
let b = self.add_ast_node(expr.id, &[pred]);
288+
let b = self.add_ast_node(expr.id, &[v]);
288289
self.add_exiting_edge(expr, b,
289290
loop_scope, loop_scope.break_index);
290291
self.add_unreachable_node()

‎src/librustc/hir/intravisit.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -882,7 +882,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
882882
visitor.visit_block(block);
883883
walk_opt_sp_name(visitor, opt_sp_name);
884884
}
885-
ExprLoop(ref block, ref opt_sp_name) => {
885+
ExprLoop(ref block, ref opt_sp_name, _) => {
886886
visitor.visit_block(block);
887887
walk_opt_sp_name(visitor, opt_sp_name);
888888
}
@@ -923,7 +923,11 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
923923
}
924924
visitor.visit_path(path, expression.id)
925925
}
926-
ExprBreak(ref opt_sp_name) | ExprAgain(ref opt_sp_name) => {
926+
ExprBreak(ref opt_sp_name, ref opt_expr) => {
927+
walk_opt_sp_name(visitor, opt_sp_name);
928+
walk_list!(visitor, visit_expr, opt_expr);
929+
}
930+
ExprAgain(ref opt_sp_name) => {
927931
walk_opt_sp_name(visitor, opt_sp_name);
928932
}
929933
ExprRet(ref optional_expression) => {

‎src/librustc/hir/lowering.rs

+12-5
Original file line numberDiff line numberDiff line change
@@ -1136,7 +1136,9 @@ impl<'a> LoweringContext<'a> {
11361136
self.lower_opt_sp_ident(opt_ident))
11371137
}
11381138
ExprKind::Loop(ref body, opt_ident) => {
1139-
hir::ExprLoop(self.lower_block(body), self.lower_opt_sp_ident(opt_ident))
1139+
hir::ExprLoop(self.lower_block(body),
1140+
self.lower_opt_sp_ident(opt_ident),
1141+
hir::LoopSource::Loop)
11401142
}
11411143
ExprKind::Match(ref expr, ref arms) => {
11421144
hir::ExprMatch(P(self.lower_expr(expr)),
@@ -1242,7 +1244,10 @@ impl<'a> LoweringContext<'a> {
12421244
});
12431245
hir::ExprPath(hir_qself, self.lower_path(path))
12441246
}
1245-
ExprKind::Break(opt_ident) => hir::ExprBreak(self.lower_opt_sp_ident(opt_ident)),
1247+
ExprKind::Break(opt_ident, ref opt_expr) => {
1248+
hir::ExprBreak(self.lower_opt_sp_ident(opt_ident),
1249+
opt_expr.as_ref().map(|x| P(self.lower_expr(x))))
1250+
}
12461251
ExprKind::Continue(opt_ident) => hir::ExprAgain(self.lower_opt_sp_ident(opt_ident)),
12471252
ExprKind::Ret(ref e) => hir::ExprRet(e.as_ref().map(|x| P(self.lower_expr(x)))),
12481253
ExprKind::InlineAsm(ref asm) => {
@@ -1410,7 +1415,8 @@ impl<'a> LoweringContext<'a> {
14101415

14111416
// `[opt_ident]: loop { ... }`
14121417
let loop_block = P(self.block_expr(P(match_expr)));
1413-
let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident));
1418+
let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident),
1419+
hir::LoopSource::WhileLet);
14141420
// add attributes to the outer returned expr node
14151421
let attrs = e.attrs.clone();
14161422
return hir::Expr { id: e.id, node: loop_expr, span: e.span, attrs: attrs };
@@ -1485,7 +1491,8 @@ impl<'a> LoweringContext<'a> {
14851491

14861492
// `[opt_ident]: loop { ... }`
14871493
let loop_block = P(self.block_expr(match_expr));
1488-
let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident));
1494+
let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident),
1495+
hir::LoopSource::ForLoop);
14891496
let loop_expr = P(hir::Expr {
14901497
id: e.id,
14911498
node: loop_expr,
@@ -1723,7 +1730,7 @@ impl<'a> LoweringContext<'a> {
17231730
}
17241731

17251732
fn expr_break(&mut self, span: Span, attrs: ThinVec<Attribute>) -> P<hir::Expr> {
1726-
P(self.expr(span, hir::ExprBreak(None), attrs))
1733+
P(self.expr(span, hir::ExprBreak(None, None), attrs))
17271734
}
17281735

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

‎src/librustc/hir/mod.rs

+14-2
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ pub enum Expr_ {
908908
/// Conditionless loop (can be exited with break, continue, or return)
909909
///
910910
/// `'label: loop { block }`
911-
ExprLoop(P<Block>, Option<Spanned<Name>>),
911+
ExprLoop(P<Block>, Option<Spanned<Name>>, LoopSource),
912912
/// A `match` block, with a source that indicates whether or not it is
913913
/// the result of a desugaring, and if so, which kind.
914914
ExprMatch(P<Expr>, HirVec<Arm>, MatchSource),
@@ -944,7 +944,7 @@ pub enum Expr_ {
944944
/// A referencing operation (`&a` or `&mut a`)
945945
ExprAddrOf(Mutability, P<Expr>),
946946
/// A `break`, with an optional label to break
947-
ExprBreak(Option<Spanned<Name>>),
947+
ExprBreak(Option<Spanned<Name>>, Option<P<Expr>>),
948948
/// A `continue`, with an optional label
949949
ExprAgain(Option<Spanned<Name>>),
950950
/// A `return`, with an optional value to be returned
@@ -1002,6 +1002,18 @@ pub enum MatchSource {
10021002
TryDesugar,
10031003
}
10041004

1005+
/// The loop type that yielded an ExprLoop
1006+
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
1007+
pub enum LoopSource {
1008+
/// A `loop { .. }` loop
1009+
Loop,
1010+
/// A `while let _ = _ { .. }` loop
1011+
WhileLet,
1012+
/// A `for _ in _ { .. }` loop
1013+
ForLoop,
1014+
}
1015+
1016+
10051017
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
10061018
pub enum CaptureClause {
10071019
CaptureByValue,

‎src/librustc/hir/print.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -1393,7 +1393,7 @@ impl<'a> State<'a> {
13931393
space(&mut self.s)?;
13941394
self.print_block(&blk)?;
13951395
}
1396-
hir::ExprLoop(ref blk, opt_sp_name) => {
1396+
hir::ExprLoop(ref blk, opt_sp_name, _) => {
13971397
if let Some(sp_name) = opt_sp_name {
13981398
self.print_name(sp_name.node)?;
13991399
self.word_space(":")?;
@@ -1471,13 +1471,17 @@ impl<'a> State<'a> {
14711471
hir::ExprPath(Some(ref qself), ref path) => {
14721472
self.print_qpath(path, qself, true)?
14731473
}
1474-
hir::ExprBreak(opt_name) => {
1474+
hir::ExprBreak(opt_name, ref opt_expr) => {
14751475
word(&mut self.s, "break")?;
14761476
space(&mut self.s)?;
14771477
if let Some(name) = opt_name {
14781478
self.print_name(name.node)?;
14791479
space(&mut self.s)?;
14801480
}
1481+
if let Some(ref expr) = *opt_expr {
1482+
self.print_expr(expr)?;
1483+
space(&mut self.s)?;
1484+
}
14811485
}
14821486
hir::ExprAgain(opt_name) => {
14831487
word(&mut self.s, "continue")?;

‎src/librustc/middle/expr_use_visitor.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -472,11 +472,10 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
472472
self.consume_exprs(inputs);
473473
}
474474

475-
hir::ExprBreak(..) |
476475
hir::ExprAgain(..) |
477476
hir::ExprLit(..) => {}
478477

479-
hir::ExprLoop(ref blk, _) => {
478+
hir::ExprLoop(ref blk, _, _) => {
480479
self.walk_block(&blk);
481480
}
482481

@@ -514,7 +513,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
514513
self.walk_block(&blk);
515514
}
516515

517-
hir::ExprRet(ref opt_expr) => {
516+
hir::ExprBreak(_, ref opt_expr) | hir::ExprRet(ref opt_expr) => {
518517
if let Some(ref expr) = *opt_expr {
519518
self.consume_expr(&expr);
520519
}

‎src/librustc/middle/liveness.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ fn visit_expr(ir: &mut IrMaps, expr: &Expr) {
490490
hir::ExprIndex(..) | hir::ExprField(..) | hir::ExprTupField(..) |
491491
hir::ExprArray(..) | hir::ExprCall(..) | hir::ExprMethodCall(..) |
492492
hir::ExprTup(..) | hir::ExprBinary(..) | hir::ExprAddrOf(..) |
493-
hir::ExprCast(..) | hir::ExprUnary(..) | hir::ExprBreak(_) |
493+
hir::ExprCast(..) | hir::ExprUnary(..) | hir::ExprBreak(..) |
494494
hir::ExprAgain(_) | hir::ExprLit(_) | hir::ExprRet(..) |
495495
hir::ExprBlock(..) | hir::ExprAssign(..) | hir::ExprAssignOp(..) |
496496
hir::ExprStruct(..) | hir::ExprRepeat(..) |
@@ -990,7 +990,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
990990

991991
// Note that labels have been resolved, so we don't need to look
992992
// at the label ident
993-
hir::ExprLoop(ref blk, _) => {
993+
hir::ExprLoop(ref blk, _, _) => {
994994
self.propagate_through_loop(expr, LoopLoop, &blk, succ)
995995
}
996996

@@ -1035,15 +1035,15 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
10351035
self.propagate_through_opt_expr(o_e.as_ref().map(|e| &**e), exit_ln)
10361036
}
10371037

1038-
hir::ExprBreak(opt_label) => {
1038+
hir::ExprBreak(opt_label, ref opt_expr) => {
10391039
// Find which label this break jumps to
10401040
let sc = self.find_loop_scope(opt_label.map(|l| l.node), expr.id, expr.span);
10411041

10421042
// Now that we know the label we're going to,
10431043
// look it up in the break loop nodes table
10441044

10451045
match self.break_ln.get(&sc) {
1046-
Some(&b) => b,
1046+
Some(&b) => self.propagate_through_opt_expr(opt_expr.as_ref().map(|e| &**e), b),
10471047
None => span_bug!(expr.span, "break to unknown label")
10481048
}
10491049
}
@@ -1057,7 +1057,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
10571057

10581058
match self.cont_ln.get(&sc) {
10591059
Some(&b) => b,
1060-
None => span_bug!(expr.span, "loop to unknown label")
1060+
None => span_bug!(expr.span, "continue to unknown label")
10611061
}
10621062
}
10631063

‎src/librustc/middle/region.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -805,7 +805,7 @@ fn resolve_expr(visitor: &mut RegionResolutionVisitor, expr: &hir::Expr) {
805805
terminating(then.id);
806806
}
807807

808-
hir::ExprLoop(ref body, _) => {
808+
hir::ExprLoop(ref body, _, _) => {
809809
terminating(body.id);
810810
}
811811

‎src/librustc/middle/resolve_lifetime.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ fn extract_labels(ctxt: &mut LifetimeContext, b: &hir::Expr) {
462462
fn expression_label(ex: &hir::Expr) -> Option<(ast::Name, Span)> {
463463
match ex.node {
464464
hir::ExprWhile(.., Some(label)) |
465-
hir::ExprLoop(_, Some(label)) => Some((label.node, label.span)),
465+
hir::ExprLoop(_, Some(label), _) => Some((label.node, label.span)),
466466
_ => None,
467467
}
468468
}

‎src/librustc/util/common.rs

-55
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,6 @@ use std::iter::repeat;
1919
use std::path::Path;
2020
use std::time::{Duration, Instant};
2121

22-
use hir;
23-
use hir::intravisit;
24-
use hir::intravisit::Visitor;
25-
2622
// The name of the associated type for `Fn` return types
2723
pub const FN_OUTPUT_NAME: &'static str = "Output";
2824

@@ -186,57 +182,6 @@ pub fn indenter() -> Indenter {
186182
Indenter { _cannot_construct_outside_of_this_module: () }
187183
}
188184

189-
struct LoopQueryVisitor<P> where P: FnMut(&hir::Expr_) -> bool {
190-
p: P,
191-
flag: bool,
192-
}
193-
194-
impl<'v, P> Visitor<'v> for LoopQueryVisitor<P> where P: FnMut(&hir::Expr_) -> bool {
195-
fn visit_expr(&mut self, e: &hir::Expr) {
196-
self.flag |= (self.p)(&e.node);
197-
match e.node {
198-
// Skip inner loops, since a break in the inner loop isn't a
199-
// break inside the outer loop
200-
hir::ExprLoop(..) | hir::ExprWhile(..) => {}
201-
_ => intravisit::walk_expr(self, e)
202-
}
203-
}
204-
}
205-
206-
// Takes a predicate p, returns true iff p is true for any subexpressions
207-
// of b -- skipping any inner loops (loop, while, loop_body)
208-
pub fn loop_query<P>(b: &hir::Block, p: P) -> bool where P: FnMut(&hir::Expr_) -> bool {
209-
let mut v = LoopQueryVisitor {
210-
p: p,
211-
flag: false,
212-
};
213-
intravisit::walk_block(&mut v, b);
214-
return v.flag;
215-
}
216-
217-
struct BlockQueryVisitor<P> where P: FnMut(&hir::Expr) -> bool {
218-
p: P,
219-
flag: bool,
220-
}
221-
222-
impl<'v, P> Visitor<'v> for BlockQueryVisitor<P> where P: FnMut(&hir::Expr) -> bool {
223-
fn visit_expr(&mut self, e: &hir::Expr) {
224-
self.flag |= (self.p)(e);
225-
intravisit::walk_expr(self, e)
226-
}
227-
}
228-
229-
// Takes a predicate p, returns true iff p is true for any subexpressions
230-
// of b -- skipping any inner loops (loop, while, loop_body)
231-
pub fn block_query<P>(b: &hir::Block, p: P) -> bool where P: FnMut(&hir::Expr) -> bool {
232-
let mut v = BlockQueryVisitor {
233-
p: p,
234-
flag: false,
235-
};
236-
intravisit::walk_block(&mut v, &b);
237-
return v.flag;
238-
}
239-
240185
pub trait MemoizationMap {
241186
type Key: Clone;
242187
type Value: Clone;

‎src/librustc_driver/driver.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -855,7 +855,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
855855

856856
time(time_passes,
857857
"loop checking",
858-
|| loops::check_crate(sess, &hir_map));
858+
|| loops::check_crate(sess, &resolutions.def_map, &hir_map));
859859

860860
time(time_passes,
861861
"static item recursion checking",

‎src/librustc_incremental/calculate_svh/svh_visitor.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ fn saw_expr<'a>(node: &'a Expr_,
322322
ExprType(..) => (SawExprType, false),
323323
ExprIf(..) => (SawExprIf, false),
324324
ExprWhile(..) => (SawExprWhile, false),
325-
ExprLoop(_, id) => (SawExprLoop(id.map(|id| id.node.as_str())), false),
325+
ExprLoop(_, id, _) => (SawExprLoop(id.map(|id| id.node.as_str())), false),
326326
ExprMatch(..) => (SawExprMatch, false),
327327
ExprClosure(cc, _, _, _) => (SawExprClosure(cc), false),
328328
ExprBlock(..) => (SawExprBlock, false),
@@ -335,7 +335,7 @@ fn saw_expr<'a>(node: &'a Expr_,
335335
ExprIndex(..) => (SawExprIndex, true),
336336
ExprPath(ref qself, _) => (SawExprPath(qself.as_ref().map(|q| q.position)), false),
337337
ExprAddrOf(m, _) => (SawExprAddrOf(m), false),
338-
ExprBreak(id) => (SawExprBreak(id.map(|id| id.node.as_str())), false),
338+
ExprBreak(id, _) => (SawExprBreak(id.map(|id| id.node.as_str())), false),
339339
ExprAgain(id) => (SawExprAgain(id.map(|id| id.node.as_str())), false),
340340
ExprRet(..) => (SawExprRet, false),
341341
ExprInlineAsm(ref a,..) => (SawExprInlineAsm(a), false),

0 commit comments

Comments
 (0)