auto merge of #12338 : edwardw/rust/hygienic-break-continue, r=cmr

Makes labelled loops hygiene by performing renaming of the labels defined in e.g. `'x: loop { ... }` and then used in break and continue statements within loop body so that they act hygienically when used with macros.
    
Closes #12262.

7 files changed, 68 insertions, 17 deletions

diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
index 8bc3888b1d0..4e43592ec5c 100644
--- a/src/libsyntax/ast.rs
+++ b/src/libsyntax/ast.rs
@@ -58,8 +58,13 @@ impl Eq for Ident {
             // if it should be non-hygienic (most things are), just compare the
             // 'name' fields of the idents. Or, even better, replace the idents
             // with Name's.
-            fail!("not allowed to compare these idents: {:?}, {:?}.
-                    Probably related to issue \\#6993", self, other);
+            //
+            // On the other hand, if the comparison does need to be hygienic,
+            // one example and its non-hygienic counterpart would be:
+            //      syntax::parse::token::mtwt_token_eq
+            //      syntax::ext::tt::macro_parser::token_name_eq
+            fail!("not allowed to compare these idents: {:?}, {:?}. \
+                   Probably related to issue \\#6993", self, other);
         }
     }
     fn ne(&self, other: &Ident) -> bool {
@@ -564,8 +569,8 @@ pub enum Expr_ {
     ExprPath(Path),
 
     ExprAddrOf(Mutability, @Expr),
-    ExprBreak(Option<Name>),
-    ExprAgain(Option<Name>),
+    ExprBreak(Option<Ident>),
+    ExprAgain(Option<Ident>),
     ExprRet(Option<@Expr>),
 
     /// Gets the log level for the enclosing module
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index 1e0bfb0d3e9..b49f9fb3a38 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -139,6 +139,8 @@ pub fn expand_expr(e: @ast::Expr, fld: &mut MacroExpander) -> @ast::Expr {
             // Expand any interior macros etc.
             // NB: we don't fold pats yet. Curious.
             let src_expr = fld.fold_expr(src_expr).clone();
+            // Rename label before expansion.
+            let (opt_ident, src_loop_block) = rename_loop_label(opt_ident, src_loop_block, fld);
             let src_loop_block = fld.fold_block(src_loop_block);
 
             let span = e.span;
@@ -165,8 +167,7 @@ pub fn expand_expr(e: @ast::Expr, fld: &mut MacroExpander) -> @ast::Expr {
 
             // `None => break ['<ident>];`
             let none_arm = {
-                // FIXME #6993: this map goes away:
-                let break_expr = fld.cx.expr(span, ast::ExprBreak(opt_ident.map(|x| x.name)));
+                let break_expr = fld.cx.expr(span, ast::ExprBreak(opt_ident));
                 let none_pat = fld.cx.pat_ident(span, none_ident);
                 fld.cx.arm(span, ~[none_pat], break_expr)
             };
@@ -199,10 +200,36 @@ pub fn expand_expr(e: @ast::Expr, fld: &mut MacroExpander) -> @ast::Expr {
             fld.cx.expr_match(span, discrim, ~[arm])
         }
 
+        ast::ExprLoop(loop_block, opt_ident) => {
+            let (opt_ident, loop_block) =
+                rename_loop_label(opt_ident, loop_block, fld);
+            let loop_block = fld.fold_block(loop_block);
+            fld.cx.expr(e.span, ast::ExprLoop(loop_block, opt_ident))
+        }
+
         _ => noop_fold_expr(e, fld)
     }
 }
 
+// Rename loop label and its all occurrences inside the loop body
+fn rename_loop_label(opt_ident: Option<Ident>,
+                     loop_block: P<Block>,
+                     fld: &mut MacroExpander) -> (Option<Ident>, P<Block>) {
+    match opt_ident {
+        Some(label) => {
+            // Generate fresh label and add to the existing pending renames
+            let new_label = fresh_name(&label);
+            let rename = (label, new_label);
+            fld.extsbox.info().pending_renames.push(rename);
+            let mut pending_renames = ~[rename];
+            let mut rename_fld = renames_to_fold(&mut pending_renames);
+            (Some(rename_fld.fold_ident(label)),
+             rename_fld.fold_block(loop_block))
+        }
+        None => (None, loop_block)
+    }
+}
+
 // eval $e with a new exts frame:
 macro_rules! with_exts_frame (
     ($extsboxexpr:expr,$macros_escape:expr,$e:expr) =>
diff --git a/src/libsyntax/ext/tt/macro_parser.rs b/src/libsyntax/ext/tt/macro_parser.rs
index 456533de5e9..edd875a57a7 100644
--- a/src/libsyntax/ext/tt/macro_parser.rs
+++ b/src/libsyntax/ext/tt/macro_parser.rs
@@ -218,8 +218,9 @@ pub fn parse_or_else<R: Reader>(sess: @ParseSess,
 // perform a token equality check, ignoring syntax context (that is, an unhygienic comparison)
 pub fn token_name_eq(t1 : &Token, t2 : &Token) -> bool {
     match (t1,t2) {
-        (&token::IDENT(id1,_),&token::IDENT(id2,_)) =>
-        id1.name == id2.name,
+        (&token::IDENT(id1,_),&token::IDENT(id2,_))
+        | (&token::LIFETIME(id1),&token::LIFETIME(id2)) =>
+            id1.name == id2.name,
         _ => *t1 == *t2
     }
 }
diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs
index e150d1685de..5f6eb86c3c8 100644
--- a/src/libsyntax/fold.rs
+++ b/src/libsyntax/fold.rs
@@ -353,6 +353,23 @@ fn fold_arg_<T: Folder>(a: &Arg, fld: &mut T) -> Arg {
 
 // build a new vector of tts by appling the Folder's fold_ident to
 // all of the identifiers in the token trees.
+//
+// This is part of hygiene magic. As far as hygiene is concerned, there
+// are three types of let pattern bindings or loop labels:
+//      - those defined and used in non-macro part of the program
+//      - those used as part of macro invocation arguments
+//      - those defined and used inside macro definitions
+// Lexically, type 1 and 2 are in one group and type 3 the other. If they
+// clash, in order for let and loop label to work hygienically, one group
+// or the other needs to be renamed. The problem is that type 2 and 3 are
+// parsed together (inside the macro expand function). After being parsed and
+// AST being constructed, they can no longer be distinguished from each other.
+//
+// For that reason, type 2 let bindings and loop labels are actually renamed
+// in the form of tokens instead of AST nodes, here. There are wasted effort
+// since many token::IDENT are not necessary part of let bindings and most
+// token::LIFETIME are certainly not loop labels. But we can't tell in their
+// token form. So this is less ideal and hacky but it works.
 pub fn fold_tts<T: Folder>(tts: &[TokenTree], fld: &mut T) -> ~[TokenTree] {
     tts.map(|tt| {
         match *tt {
@@ -376,6 +393,7 @@ fn maybe_fold_ident<T: Folder>(t: &token::Token, fld: &mut T) -> token::Token {
         token::IDENT(id, followed_by_colons) => {
             token::IDENT(fld.fold_ident(id), followed_by_colons)
         }
+        token::LIFETIME(id) => token::LIFETIME(fld.fold_ident(id)),
         _ => (*t).clone()
     }
 }
@@ -802,8 +820,8 @@ pub fn noop_fold_expr<T: Folder>(e: @Expr, folder: &mut T) -> @Expr {
         }
         ExprPath(ref pth) => ExprPath(folder.fold_path(pth)),
         ExprLogLevel => ExprLogLevel,
-        ExprBreak(opt_ident) => ExprBreak(opt_ident),
-        ExprAgain(opt_ident) => ExprAgain(opt_ident),
+        ExprBreak(opt_ident) => ExprBreak(opt_ident.map(|x| folder.fold_ident(x))),
+        ExprAgain(opt_ident) => ExprAgain(opt_ident.map(|x| folder.fold_ident(x))),
         ExprRet(ref e) => {
             ExprRet(e.map(|x| folder.fold_expr(x)))
         }
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index fed2034cd26..31e16cd8c7d 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -1822,7 +1822,7 @@ impl Parser {
             let ex = if Parser::token_is_lifetime(&self.token) {
                 let lifetime = self.get_lifetime();
                 self.bump();
-                ExprAgain(Some(lifetime.name))
+                ExprAgain(Some(lifetime))
             } else {
                 ExprAgain(None)
             };
@@ -1885,7 +1885,7 @@ impl Parser {
             if Parser::token_is_lifetime(&self.token) {
                 let lifetime = self.get_lifetime();
                 self.bump();
-                ex = ExprBreak(Some(lifetime.name));
+                ex = ExprBreak(Some(lifetime));
             } else {
                 ex = ExprBreak(None);
             }
@@ -2579,7 +2579,7 @@ impl Parser {
             let ex = if Parser::token_is_lifetime(&self.token) {
                 let lifetime = self.get_lifetime();
                 self.bump();
-                ExprAgain(Some(lifetime.name))
+                ExprAgain(Some(lifetime))
             } else {
                 ExprAgain(None)
             };
diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs
index a2f3b65f06e..528eb7d54f3 100644
--- a/src/libsyntax/parse/token.rs
+++ b/src/libsyntax/parse/token.rs
@@ -704,8 +704,8 @@ pub fn is_reserved_keyword(tok: &Token) -> bool {
 
 pub fn mtwt_token_eq(t1 : &Token, t2 : &Token) -> bool {
     match (t1,t2) {
-        (&IDENT(id1,_),&IDENT(id2,_)) =>
-        ast_util::mtwt_resolve(id1) == ast_util::mtwt_resolve(id2),
+        (&IDENT(id1,_),&IDENT(id2,_)) | (&LIFETIME(id1),&LIFETIME(id2)) =>
+            ast_util::mtwt_resolve(id1) == ast_util::mtwt_resolve(id2),
         _ => *t1 == *t2
     }
 }
diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs
index f934a9022a6..8fb813407d0 100644
--- a/src/libsyntax/print/pprust.rs
+++ b/src/libsyntax/print/pprust.rs
@@ -1471,7 +1471,7 @@ pub fn print_expr(s: &mut State, expr: &ast::Expr) -> io::IoResult<()> {
         try!(space(&mut s.s));
         for ident in opt_ident.iter() {
             try!(word(&mut s.s, "'"));
-            try!(print_name(s, *ident));
+            try!(print_ident(s, *ident));
             try!(space(&mut s.s));
         }
       }
@@ -1480,7 +1480,7 @@ pub fn print_expr(s: &mut State, expr: &ast::Expr) -> io::IoResult<()> {
         try!(space(&mut s.s));
         for ident in opt_ident.iter() {
             try!(word(&mut s.s, "'"));
-            try!(print_name(s, *ident));
+            try!(print_ident(s, *ident));
             try!(space(&mut s.s))
         }
       }