hygiene for for loops, if let, while let

and some unrelated test cleanups

Author: Nick Cameron

src/librustc/middle/region.rs

@@ -377,7 +377,7 @@ impl RegionMaps {
         self.code_extents.borrow()[e.0 as usize]
     }
     pub fn each_encl_scope<E>(&self, mut e:E) where E: FnMut(&CodeExtent, &CodeExtent) {
-        for child_id in (1..self.code_extents.borrow().len()) {
+        for child_id in 1..self.code_extents.borrow().len() {
             let child = CodeExtent(child_id as u32);
             if let Some(parent) = self.opt_encl_scope(child) {
                 e(&child, &parent)

src/librustc_lint/unused.rs

@@ -363,12 +363,10 @@ impl EarlyLintPass for UnusedParens {
         let (value, msg, struct_lit_needs_parens) = match e.node {
             ast::ExprIf(ref cond, _, _) => (cond, "`if` condition", true),
             ast::ExprWhile(ref cond, _, _) => (cond, "`while` condition", true),
-            ast::ExprMatch(ref head, _, source) => match source {
-                ast::MatchSource::Normal => (head, "`match` head expression", true),
-                ast::MatchSource::IfLetDesugar { .. } => (head, "`if let` head expression", true),
-                ast::MatchSource::WhileLetDesugar => (head, "`while let` head expression", true),
-                ast::MatchSource::ForLoopDesugar => (head, "`for` head expression", true),
-            },
+            ast::ExprIfLet(_, ref cond, _, _) => (cond, "`if let` head expression", true),
+            ast::ExprWhileLet(_, ref cond, _, _) => (cond, "`while let` head expression", true),
+            ast::ExprForLoop(_, ref cond, _, _) => (cond, "`for` head expression", true),
+            ast::ExprMatch(ref head, _, _) => (head, "`match` head expression", true),
             ast::ExprRet(Some(ref value)) => (value, "`return` value", false),
             ast::ExprAssign(_, ref value) => (value, "assigned value", false),
             ast::ExprAssignOp(_, _, ref value) => (value, "assigned value", false),

src/librustc_typeck/check/mod.rs

@@ -1810,7 +1810,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         // There is a possibility that this algorithm will have to run an arbitrary number of times
         // to terminate so we bound it by the compiler's recursion limit.
-        for _ in (0..self.tcx().sess.recursion_limit.get()) {
+        for _ in 0..self.tcx().sess.recursion_limit.get() {
             // First we try to solve all obligations, it is possible that the last iteration
             // has made it possible to make more progress.
             self.select_obligations_where_possible();

src/libsyntax/ext/expand.rs

@@ -82,8 +82,18 @@ pub fn expand_expr(e: P<ast::Expr>, fld: &mut MacroExpander) -> P<ast::Expr> {
         ast::ExprWhileLet(pat, expr, body, opt_ident) => {
             let pat = fld.fold_pat(pat);
             let expr = fld.fold_expr(expr);
-            let (body, opt_ident) = expand_loop_block(body, opt_ident, fld);
-            fld.cx.expr(span, ast::ExprWhileLet(pat, expr, body, opt_ident))
+
+            // Hygienic renaming of the body.
+            let ((body, opt_ident), mut rewritten_pats) =
+                rename_in_scope(vec![pat],
+                                fld,
+                                (body, opt_ident),
+                                |rename_fld, fld, (body, opt_ident)| {
+                expand_loop_block(rename_fld.fold_block(body), opt_ident, fld)
+            });
+            assert!(rewritten_pats.len() == 1);
+
+            fld.cx.expr(span, ast::ExprWhileLet(rewritten_pats.remove(0), expr, body, opt_ident))
         }
 
         ast::ExprLoop(loop_block, opt_ident) => {
@@ -93,9 +103,37 @@ pub fn expand_expr(e: P<ast::Expr>, fld: &mut MacroExpander) -> P<ast::Expr> {
 
         ast::ExprForLoop(pat, head, body, opt_ident) => {
             let pat = fld.fold_pat(pat);
+
+            // Hygienic renaming of the for loop body (for loop binds its pattern).
+            let ((body, opt_ident), mut rewritten_pats) =
+                rename_in_scope(vec![pat],
+                                fld,
+                                (body, opt_ident),
+                                |rename_fld, fld, (body, opt_ident)| {
+                expand_loop_block(rename_fld.fold_block(body), opt_ident, fld)
+            });
+            assert!(rewritten_pats.len() == 1);
+
             let head = fld.fold_expr(head);
-            let (body, opt_ident) = expand_loop_block(body, opt_ident, fld);
-            fld.cx.expr(span, ast::ExprForLoop(pat, head, body, opt_ident))
+            fld.cx.expr(span, ast::ExprForLoop(rewritten_pats.remove(0), head, body, opt_ident))
+        }
+
+        ast::ExprIfLet(pat, sub_expr, body, else_opt) => {
+            let pat = fld.fold_pat(pat);
+
+            // Hygienic renaming of the body.
+            let (body, mut rewritten_pats) =
+                rename_in_scope(vec![pat],
+                                fld,
+                                body,
+                                |rename_fld, fld, body| {
+                fld.fold_block(rename_fld.fold_block(body))
+            });
+            assert!(rewritten_pats.len() == 1);
+
+            let else_opt = else_opt.map(|else_opt| fld.fold_expr(else_opt));
+            let sub_expr = fld.fold_expr(sub_expr);
+            fld.cx.expr(span, ast::ExprIfLet(rewritten_pats.remove(0), sub_expr, body, else_opt))
         }
 
         ast::ExprClosure(capture_clause, fn_decl, block) => {
@@ -569,18 +607,18 @@ fn expand_arm(arm: ast::Arm, fld: &mut MacroExpander) -> ast::Arm {
     if expanded_pats.is_empty() {
         panic!("encountered match arm with 0 patterns");
     }
-    // all of the pats must have the same set of bindings, so use the
-    // first one to extract them and generate new names:
-    let idents = pattern_bindings(&*expanded_pats[0]);
-    let new_renames = idents.into_iter().map(|id| (id, fresh_name(id))).collect();
-    // apply the renaming, but only to the PatIdents:
-    let mut rename_pats_fld = PatIdentRenamer{renames:&new_renames};
-    let rewritten_pats = expanded_pats.move_map(|pat| rename_pats_fld.fold_pat(pat));
+
     // apply renaming and then expansion to the guard and the body:
-    let mut rename_fld = IdentRenamer{renames:&new_renames};
-    let rewritten_guard =
-        arm.guard.map(|g| fld.fold_expr(rename_fld.fold_expr(g)));
-    let rewritten_body = fld.fold_expr(rename_fld.fold_expr(arm.body));
+    let ((rewritten_guard, rewritten_body), rewritten_pats) =
+        rename_in_scope(expanded_pats,
+                        fld,
+                        (arm.guard, arm.body),
+                        |rename_fld, fld, (ag, ab)|{
+        let rewritten_guard = ag.map(|g| fld.fold_expr(rename_fld.fold_expr(g)));
+        let rewritten_body = fld.fold_expr(rename_fld.fold_expr(ab));
+        (rewritten_guard, rewritten_body)
+    });
+
     ast::Arm {
         attrs: fold::fold_attrs(arm.attrs, fld),
         pats: rewritten_pats,
@@ -589,6 +627,25 @@ fn expand_arm(arm: ast::Arm, fld: &mut MacroExpander) -> ast::Arm {
     }
 }
 
+fn rename_in_scope<X, F>(pats: Vec<P<ast::Pat>>,
+                         fld: &mut MacroExpander,
+                         x: X,
+                         f: F)
+                         -> (X, Vec<P<ast::Pat>>)
+    where F: Fn(&mut IdentRenamer, &mut MacroExpander, X) -> X
+{
+    // all of the pats must have the same set of bindings, so use the
+    // first one to extract them and generate new names:
+    let idents = pattern_bindings(&*pats[0]);
+    let new_renames = idents.into_iter().map(|id| (id, fresh_name(id))).collect();
+    // apply the renaming, but only to the PatIdents:
+    let mut rename_pats_fld = PatIdentRenamer{renames:&new_renames};
+    let rewritten_pats = pats.move_map(|pat| rename_pats_fld.fold_pat(pat));
+
+    let mut rename_fld = IdentRenamer{ renames:&new_renames };
+    (f(&mut rename_fld, fld, x), rewritten_pats)
+}
+
 /// A visitor that extracts the PatIdent (binding) paths
 /// from a given thingy and puts them in a mutable
 /// array

src/test/compile-fail/for-expn-2.rs

@@ -9,7 +9,5 @@
 // except according to those terms.
 
 fn main() {
-    for
-        &1 //~ ERROR refutable pattern in `for` loop binding
-        in [1].iter() {}
+    for &1 in [1].iter() {} //~ ERROR refutable pattern in `for` loop binding
 }

src/test/compile-fail/for-loop-refutable-pattern-error-message.rs

@@ -16,4 +16,17 @@ fn main() -> (){
     for n in 0..1 {
         println!("{}", f!()); //~ ERROR unresolved name `n`
     }
+
+    if let Some(n) = None {
+        println!("{}", f!()); //~ ERROR unresolved name `n`
+    }
+
+    if false {
+    } else if let Some(n) = None {
+        println!("{}", f!()); //~ ERROR unresolved name `n`
+    }
+
+    while let Some(n) = None {
+        println!("{}", f!()); //~ ERROR unresolved name `n`
+    }
 }

src/test/compile-fail/issue-15167.rs

@@ -13,10 +13,8 @@
 fn main() {
     let values: Vec<u8> = vec![1,2,3,4,5,6,7,8];
 
-    for
-        [x,y,z]
-//~^ ERROR refutable pattern in `for` loop binding: `[]` not covered
-        in values.chunks(3).filter(|&xs| xs.len() == 3) {
+    for [x,y,z] in values.chunks(3).filter(|&xs| xs.len() == 3) {
+        //~^ ERROR refutable pattern in `for` loop binding: `[]` not covered
         println!("y={}", y);
     }
 }