Auto merge of #21984 - pnkfelix:make-for-loops-a-terminating-scope, r=nikomatsakis

bors · bors · commit 61626b306314 · 2015-02-07T18:20:55.000Z
make `for PAT in ITER_EXPR { ... }` a terminating-scope for ITER_EXPR. In effect, temporary anonymous values created during the evaluation of ITER_EXPR no longer not live for the entirety of the block surrounding the for-loop; instead they only live for the extent of the for-loop itself, and no longer. ---- There is one case I know of that this breaks, demonstrated to me by @nikomatsakis (but it is also a corner-case that is useless in practice). Here is that case: ``` fn main() { let mut foo: Vec<&i8> = Vec::new(); for i in &[1, 2, 3] { foo.push(i) } } ``` Note that if you add any code following the for-loop above, or even a semicolon to the end of it, then the code will stop compiling (i.e., it gathers a vector of references but the gathered vector cannot actually be used.) (The above code, despite being useless, did occur in one run-pass test by accident; that test is updated here to accommodate the new striction.) ---- So, technically this is a: [breaking-change]
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
@@ -230,15 +230,18 @@ pub fn expand_expr(e: P<ast::Expr>, fld: &mut MacroExpander) -> P<ast::Expr> {
         ast::ExprForLoop(pat, head, body, opt_ident) => {
             // to:
             //
-            //   match ::std::iter::IntoIterator::into_iter(<head>) {
-            //     mut iter => {
-            //       [opt_ident]: loop {
-            //         match ::std::iter::Iterator::next(&mut iter) {
-            //           ::std::option::Option::Some(<pat>) => <body>,
-            //           ::std::option::Option::None => break
+            //   {
+            //     let result = match ::std::iter::IntoIterator::into_iter(<head>) {
+            //       mut iter => {
+            //         [opt_ident]: loop {
+            //           match ::std::iter::Iterator::next(&mut iter) {
+            //             ::std::option::Option::Some(<pat>) => <body>,
+            //             ::std::option::Option::None => break
+            //           }
             //         }
             //       }
-            //     }
+            //     };
+            //     result
             //   }
 
             // expand <head>
@@ -319,7 +322,16 @@ pub fn expand_expr(e: P<ast::Expr>, fld: &mut MacroExpander) -> P<ast::Expr> {
 
                 fld.cx.expr_call(span, fld.cx.expr_path(into_iter_path), vec![head])
             };
-            fld.cx.expr_match(span, into_iter_expr, vec![iter_arm])
+
+            let match_expr = fld.cx.expr_match(span, into_iter_expr, vec![iter_arm]);
+
+            // `{ let result = ...; result }`
+            let result_ident = token::gensym_ident("result");
+            fld.cx.expr_block(
+                fld.cx.block_all(
+                    span,
+                    vec![fld.cx.stmt_let(span, false, result_ident, match_expr)],
+                    Some(fld.cx.expr_ident(span, result_ident))))
         }
 
         ast::ExprClosure(capture_clause, fn_decl, block) => {
diff --git a/src/test/run-pass/loop-label-shadowing.rs b/src/test/run-pass/loop-label-shadowing.rs
@@ -13,7 +13,7 @@
 fn main() {
     let mut foo = Vec::new();
     'foo: for i in &[1, 2, 3] {
-        foo.push(i);
+        foo.push(*i);
     }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@`
`13`	`13`	`fn main() {`
`14`	`14`	`let mut foo = Vec::new();`
`15`	`15`	`'foo: for i in &[1, 2, 3] {`
`16`		`- foo.push(i);`
	`16`	`+ foo.push(*i);`
`17`	`17`	`}`
`18`	`18`	`}`
`19`	`19`