Skip to content

Commit cfdbff7

Browse files
ZoxcZoxc
committed
Change for-loop desugar to not borrow the iterator during the loop
1 parent a11c26f commit cfdbff7

File tree

9 files changed

+138
-83
lines changed

9 files changed

+138
-83
lines changed

‎src/libcore/iter/mod.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,11 @@
191191
//! {
192192
//! let result = match IntoIterator::into_iter(values) {
193193
//! mut iter => loop {
194-
//! match iter.next() {
195-
//! Some(x) => { println!("{}", x); },
194+
//! let x = match iter.next() {
195+
//! Some(val) => val,
196196
//! None => break,
197-
//! }
197+
//! };
198+
//! let () = { println!("{}", x); };
198199
//! },
199200
//! };
200201
//! result

‎src/librustc/hir/lowering.rs‎

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -891,6 +891,7 @@ impl<'a> LoweringContext<'a> {
891891
init: l.init.as_ref().map(|e| P(self.lower_expr(e))),
892892
span: l.span,
893893
attrs: l.attrs.clone(),
894+
source: hir::LocalSource::Normal,
894895
})
895896
}
896897

@@ -2167,10 +2168,11 @@ impl<'a> LoweringContext<'a> {
21672168
// let result = match ::std::iter::IntoIterator::into_iter(<head>) {
21682169
// mut iter => {
21692170
// [opt_ident]: loop {
2170-
// match ::std::iter::Iterator::next(&mut iter) {
2171-
// ::std::option::Option::Some(<pat>) => <body>,
2171+
// let <pat> = match ::std::iter::Iterator::next(&mut iter) {
2172+
// ::std::option::Option::Some(val) => val,
21722173
// ::std::option::Option::None => break
2173-
// }
2174+
// };
2175+
// SemiExpr(<body>);
21742176
// }
21752177
// }
21762178
// };
@@ -2182,15 +2184,13 @@ impl<'a> LoweringContext<'a> {
21822184

21832185
let iter = self.str_to_ident("iter");
21842186

2185-
// `::std::option::Option::Some(<pat>) => <body>`
2187+
// `::std::option::Option::Some(val) => val`
21862188
let pat_arm = {
2187-
let body_block = self.with_loop_scope(e.id,
2188-
|this| this.lower_block(body, false));
2189-
let body_expr = P(self.expr_block(body_block, ThinVec::new()));
2190-
let pat = self.lower_pat(pat);
2191-
let some_pat = self.pat_some(e.span, pat);
2192-
2193-
self.arm(hir_vec![some_pat], body_expr)
2189+
let val_ident = self.str_to_ident("val");
2190+
let val_pat = self.pat_ident(e.span, val_ident);
2191+
let val_expr = P(self.expr_ident(e.span, val_ident, val_pat.id));
2192+
let some_pat = self.pat_some(e.span, val_pat);
2193+
self.arm(hir_vec![some_pat], val_expr)
21942194
};
21952195

21962196
// `::std::option::Option::None => break`
@@ -2221,8 +2221,20 @@ impl<'a> LoweringContext<'a> {
22212221
ThinVec::new()))
22222222
};
22232223

2224+
let pat = self.lower_pat(pat);
2225+
let pat_let = self.stmt_let_pat(e.span,
2226+
match_expr,
2227+
pat,
2228+
hir::LocalSource::ForLoopDesugar);
2229+
2230+
let body_block = self.with_loop_scope(e.id,
2231+
|this| this.lower_block(body, false));
2232+
let body_expr = P(self.expr_block(body_block, ThinVec::new()));
2233+
let body_stmt = respan(e.span, hir::StmtExpr(body_expr, self.next_id()));
2234+
2235+
let loop_block = P(self.block_all(e.span, hir_vec![pat_let, body_stmt], None));
2236+
22242237
// `[opt_ident]: loop { ... }`
2225-
let loop_block = P(self.block_expr(match_expr));
22262238
let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident),
22272239
hir::LoopSource::ForLoop);
22282240
let loop_expr = P(hir::Expr {
@@ -2585,24 +2597,34 @@ impl<'a> LoweringContext<'a> {
25852597
}
25862598
}
25872599

2588-
fn stmt_let(&mut self, sp: Span, mutbl: bool, ident: Name, ex: P<hir::Expr>)
2589-
-> (hir::Stmt, NodeId) {
2590-
let pat = if mutbl {
2591-
self.pat_ident_binding_mode(sp, ident, hir::BindByValue(hir::MutMutable))
2592-
} else {
2593-
self.pat_ident(sp, ident)
2594-
};
2595-
let pat_id = pat.id;
2600+
fn stmt_let_pat(&mut self,
2601+
sp: Span,
2602+
ex: P<hir::Expr>,
2603+
pat: P<hir::Pat>,
2604+
source: hir::LocalSource)
2605+
-> hir::Stmt {
25962606
let local = P(hir::Local {
25972607
pat: pat,
25982608
ty: None,
25992609
init: Some(ex),
26002610
id: self.next_id(),
26012611
span: sp,
26022612
attrs: ThinVec::new(),
2613+
source,
26032614
});
26042615
let decl = respan(sp, hir::DeclLocal(local));
2605-
(respan(sp, hir::StmtDecl(P(decl), self.next_id())), pat_id)
2616+
respan(sp, hir::StmtDecl(P(decl), self.next_id()))
2617+
}
2618+
2619+
fn stmt_let(&mut self, sp: Span, mutbl: bool, ident: Name, ex: P<hir::Expr>)
2620+
-> (hir::Stmt, NodeId) {
2621+
let pat = if mutbl {
2622+
self.pat_ident_binding_mode(sp, ident, hir::BindByValue(hir::MutMutable))
2623+
} else {
2624+
self.pat_ident(sp, ident)
2625+
};
2626+
let pat_id = pat.id;
2627+
(self.stmt_let_pat(sp, ex, pat, hir::LocalSource::Normal), pat_id)
26062628
}
26072629

26082630
fn block_expr(&mut self, expr: P<hir::Expr>) -> hir::Block {

‎src/librustc/hir/mod.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,7 @@ pub struct Local {
872872
pub id: NodeId,
873873
pub span: Span,
874874
pub attrs: ThinVec<Attribute>,
875+
pub source: LocalSource,
875876
}
876877

877878
pub type Decl = Spanned<Decl_>;
@@ -1080,6 +1081,15 @@ pub enum QPath {
10801081
TypeRelative(P<Ty>, P<PathSegment>)
10811082
}
10821083

1084+
/// Hints at the original code for a let statement
1085+
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
1086+
pub enum LocalSource {
1087+
/// A `match _ { .. }`
1088+
Normal,
1089+
/// A desugared `for _ in _ { .. }` loop
1090+
ForLoopDesugar,
1091+
}
1092+
10831093
/// Hints at the original code for a `match _ { .. }`
10841094
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
10851095
pub enum MatchSource {

‎src/librustc/ich/impls_hir.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,8 @@ impl_stable_hash_for!(struct hir::Local {
490490
init,
491491
id,
492492
span,
493-
attrs
493+
attrs,
494+
source
494495
});
495496

496497
impl_stable_hash_for_spanned!(hir::Decl_);
@@ -640,6 +641,11 @@ impl_stable_hash_for!(enum hir::Expr_ {
640641
ExprRepeat(val, times)
641642
});
642643

644+
impl_stable_hash_for!(enum hir::LocalSource {
645+
Normal,
646+
ForLoopDesugar
647+
});
648+
643649
impl_stable_hash_for!(enum hir::LoopSource {
644650
Loop,
645651
WhileLet,

‎src/librustc_const_eval/check_match.rs‎

Lines changed: 32 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ impl<'a, 'tcx> Visitor<'tcx> for MatchVisitor<'a, 'tcx> {
9292
fn visit_local(&mut self, loc: &'tcx hir::Local) {
9393
intravisit::walk_local(self, loc);
9494

95-
self.check_irrefutable(&loc.pat, false);
95+
self.check_irrefutable(&loc.pat, match loc.source {
96+
hir::LocalSource::Normal => "local binding",
97+
hir::LocalSource::ForLoopDesugar => "`for` loop binding",
98+
});
9699

97100
// Check legality of move bindings and `@` patterns.
98101
self.check_patterns(false, slice::ref_slice(&loc.pat));
@@ -102,7 +105,7 @@ impl<'a, 'tcx> Visitor<'tcx> for MatchVisitor<'a, 'tcx> {
102105
intravisit::walk_body(self, body);
103106

104107
for arg in &body.arguments {
105-
self.check_irrefutable(&arg.pat, true);
108+
self.check_irrefutable(&arg.pat, "function argument");
106109
self.check_patterns(false, slice::ref_slice(&arg.pat));
107110
}
108111
}
@@ -211,7 +214,7 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
211214
.map(|pat| vec![pat.0])
212215
.collect();
213216
let scrut_ty = self.tables.node_id_to_type(scrut.id);
214-
check_exhaustive(cx, scrut_ty, scrut.span, &matrix, source);
217+
check_exhaustive(cx, scrut_ty, scrut.span, &matrix);
215218
})
216219
}
217220

@@ -224,13 +227,7 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
224227
}
225228
}
226229

227-
fn check_irrefutable(&self, pat: &Pat, is_fn_arg: bool) {
228-
let origin = if is_fn_arg {
229-
"function argument"
230-
} else {
231-
"local binding"
232-
};
233-
230+
fn check_irrefutable(&self, pat: &Pat, origin: &str) {
234231
let module = self.tcx.hir.get_module_parent(pat.id);
235232
MatchCheckCtxt::create_and_enter(self.tcx, module, |ref mut cx| {
236233
let mut patcx = PatternContext::new(self.tcx, self.tables);
@@ -396,8 +393,7 @@ fn check_arms<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>,
396393
fn check_exhaustive<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>,
397394
scrut_ty: Ty<'tcx>,
398395
sp: Span,
399-
matrix: &Matrix<'a, 'tcx>,
400-
source: hir::MatchSource) {
396+
matrix: &Matrix<'a, 'tcx>) {
401397
let wild_pattern = Pattern {
402398
ty: scrut_ty,
403399
span: DUMMY_SP,
@@ -410,52 +406,32 @@ fn check_exhaustive<'a, 'tcx>(cx: &mut MatchCheckCtxt<'a, 'tcx>,
410406
} else {
411407
pats.iter().map(|w| w.single_pattern()).collect()
412408
};
413-
match source {
414-
hir::MatchSource::ForLoopDesugar => {
415-
// `witnesses[0]` has the form `Some(<head>)`, peel off the `Some`
416-
let witness = match *witnesses[0].kind {
417-
PatternKind::Variant { ref subpatterns, .. } => match &subpatterns[..] {
418-
&[ref pat] => &pat.pattern,
419-
_ => bug!(),
420-
},
421-
_ => bug!(),
422-
};
423-
let pattern_string = witness.to_string();
424-
struct_span_err!(cx.tcx.sess, sp, E0297,
425-
"refutable pattern in `for` loop binding: \
426-
`{}` not covered",
427-
pattern_string)
428-
.span_label(sp, format!("pattern `{}` not covered", pattern_string))
429-
.emit();
409+
410+
const LIMIT: usize = 3;
411+
let joined_patterns = match witnesses.len() {
412+
0 => bug!(),
413+
1 => format!("`{}`", witnesses[0]),
414+
2...LIMIT => {
415+
let (tail, head) = witnesses.split_last().unwrap();
416+
let head: Vec<_> = head.iter().map(|w| w.to_string()).collect();
417+
format!("`{}` and `{}`", head.join("`, `"), tail)
430418
},
431419
_ => {
432-
const LIMIT: usize = 3;
433-
let joined_patterns = match witnesses.len() {
434-
0 => bug!(),
435-
1 => format!("`{}`", witnesses[0]),
436-
2...LIMIT => {
437-
let (tail, head) = witnesses.split_last().unwrap();
438-
let head: Vec<_> = head.iter().map(|w| w.to_string()).collect();
439-
format!("`{}` and `{}`", head.join("`, `"), tail)
440-
},
441-
_ => {
442-
let (head, tail) = witnesses.split_at(LIMIT);
443-
let head: Vec<_> = head.iter().map(|w| w.to_string()).collect();
444-
format!("`{}` and {} more", head.join("`, `"), tail.len())
445-
}
446-
};
447-
448-
let label_text = match witnesses.len() {
449-
1 => format!("pattern {} not covered", joined_patterns),
450-
_ => format!("patterns {} not covered", joined_patterns)
451-
};
452-
create_e0004(cx.tcx.sess, sp,
453-
format!("non-exhaustive patterns: {} not covered",
454-
joined_patterns))
455-
.span_label(sp, label_text)
456-
.emit();
457-
},
458-
}
420+
let (head, tail) = witnesses.split_at(LIMIT);
421+
let head: Vec<_> = head.iter().map(|w| w.to_string()).collect();
422+
format!("`{}` and {} more", head.join("`, `"), tail.len())
423+
}
424+
};
425+
426+
let label_text = match witnesses.len() {
427+
1 => format!("pattern {} not covered", joined_patterns),
428+
_ => format!("patterns {} not covered", joined_patterns)
429+
};
430+
create_e0004(cx.tcx.sess, sp,
431+
format!("non-exhaustive patterns: {} not covered",
432+
joined_patterns))
433+
.span_label(sp, label_text)
434+
.emit();
459435
}
460436
NotUseful => {
461437
// This is good, wildcard pattern isn't reachable

‎src/librustc_const_eval/diagnostics.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,12 +452,14 @@ enum Method { GET, POST }
452452

453453

454454
E0297: r##"
455+
#### Note: this error code is no longer emitted by the compiler.
456+
455457
Patterns used to bind names must be irrefutable. That is, they must guarantee
456458
that a name will be extracted in all cases. Instead of pattern matching the
457459
loop variable, consider using a `match` or `if let` inside the loop body. For
458460
instance:
459461
460-
```compile_fail,E0297
462+
```compile_fail,E0005
461463
let xs : Vec<Option<i32>> = vec![Some(1), None];
462464
463465
// This fails because `None` is not covered.

‎src/test/compile-fail/E0297.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,6 @@ fn main() {
1212
let xs : Vec<Option<i32>> = vec![Some(1), None];
1313

1414
for Some(x) in xs {}
15-
//~^ ERROR E0297
15+
//~^ ERROR E0005
1616
//~| NOTE pattern `None` not covered
1717
}

‎src/test/compile-fail/for-loop-has-unit-body.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
fn main() {
12+
for x in 0..3 {
13+
x //~ ERROR mismatched types
14+
//~| NOTE expected ()
15+
//~| NOTE expected type `()`
16+
}
17+
}

‎src/test/run-pass/for-loop-has-unit-body.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
fn main() {
12+
// Check that the tail statement in the body unifies with something
13+
for _ in 0..3 {
14+
unsafe { std::mem::uninitialized() }
15+
}
16+
17+
// Check that the tail statement in the body can be unit
18+
for _ in 0..3 {
19+
()
20+
}
21+
}

0 commit comments

Comments
 (0)