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.

src/librustc/middle/cfg/construct.rs

@@ -490,7 +490,7 @@ fn add_exiting_edge(&mut self,
 
     fn find_scope(&self,
                   expr: @ast::Expr,
-                  label: Option<ast::Name>) -> LoopScope {
+                  label: Option<ast::Ident>) -> LoopScope {
         match label {
             None => {
                 return *self.loop_scopes.last().unwrap();

src/librustc/middle/dataflow.rs

@@ -770,7 +770,7 @@ fn walk_pat_alternatives(&mut self,
 
     fn find_scope<'a>(&self,
                       expr: &ast::Expr,
-                      label: Option<ast::Name>,
+                      label: Option<ast::Ident>,
                       loop_scopes: &'a mut ~[LoopScope]) -> &'a mut LoopScope {
         let index = match label {
             None => {

src/librustc/middle/liveness.rs

@@ -747,7 +747,7 @@ pub fn write_vars(&self,
     }
 
     pub fn find_loop_scope(&self,
-                           opt_label: Option<Name>,
+                           opt_label: Option<Ident>,
                            id: NodeId,
                            sp: Span)
                            -> NodeId {

src/librustc/middle/resolve.rs

@@ -5206,13 +5206,13 @@ fn resolve_expr(&mut self, expr: &Expr) {
             ExprLoop(_, Some(label)) => {
                 self.with_label_rib(|this| {
                     let def_like = DlDef(DefLabel(expr.id));
-                    // plain insert (no renaming)
                     {
                         let mut label_ribs = this.label_ribs.borrow_mut();
                         let rib = label_ribs.get()[label_ribs.get().len() -
                                                    1];
                         let mut bindings = rib.bindings.borrow_mut();
-                        bindings.get().insert(label.name, def_like);
+                        let renamed = mtwt_resolve(label);
+                        bindings.get().insert(renamed, def_like);
                     }
 
                     visit::walk_expr(this, expr, ());
@@ -5223,11 +5223,12 @@ fn resolve_expr(&mut self, expr: &Expr) {
 
             ExprBreak(Some(label)) | ExprAgain(Some(label)) => {
                 let mut label_ribs = self.label_ribs.borrow_mut();
-                match self.search_ribs(label_ribs.get(), label, expr.span) {
+                let renamed = mtwt_resolve(label);
+                match self.search_ribs(label_ribs.get(), renamed, expr.span) {
                     None =>
                         self.resolve_error(expr.span,
                                               format!("use of undeclared label `{}`",
-                                                   token::get_name(label))),
+                                                   token::get_ident(label))),
                     Some(DlDef(def @ DefLabel(_))) => {
                         // Since this def is a label, it is never read.
                         self.record_def(expr.id, (def, LastMod(AllPublic)))

src/librustc/middle/trans/controlflow.rs

@@ -25,7 +25,7 @@
 use middle::trans::type_::Type;
 
 use syntax::ast;
-use syntax::ast::Name;
+use syntax::ast::Ident;
 use syntax::ast_util;
 use syntax::codemap::Span;
 use syntax::parse::token::InternedString;
@@ -260,7 +260,7 @@ pub fn trans_loop<'a>(bcx:&'a Block<'a>,
 
 pub fn trans_break_cont<'a>(bcx: &'a Block<'a>,
                             expr_id: ast::NodeId,
-                            opt_label: Option<Name>,
+                            opt_label: Option<Ident>,
                             exit: uint)
                             -> &'a Block<'a> {
     let _icx = push_ctxt("trans_break_cont");
@@ -293,14 +293,14 @@ pub fn trans_break_cont<'a>(bcx: &'a Block<'a>,
 
 pub fn trans_break<'a>(bcx: &'a Block<'a>,
                        expr_id: ast::NodeId,
-                       label_opt: Option<Name>)
+                       label_opt: Option<Ident>)
                        -> &'a Block<'a> {
     return trans_break_cont(bcx, expr_id, label_opt, cleanup::EXIT_BREAK);
 }
 
 pub fn trans_cont<'a>(bcx: &'a Block<'a>,
                       expr_id: ast::NodeId,
-                      label_opt: Option<Name>)
+                      label_opt: Option<Ident>)
                       -> &'a Block<'a> {
     return trans_break_cont(bcx, expr_id, label_opt, cleanup::EXIT_LOOP);
 }

src/libsyntax/ast.rs

@@ -58,8 +58,13 @@ fn eq(&self, other: &Ident) -> bool {
             // 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

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) =>

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
     }
 }

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)))
         }

src/libsyntax/parse/parser.rs

@@ -1822,7 +1822,7 @@ pub fn parse_bottom_expr(&mut self) -> @Expr {
             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 @@ pub fn parse_bottom_expr(&mut self) -> @Expr {
             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 @@ pub fn parse_loop_expr(&mut self, opt_ident: Option<ast::Ident>) -> @Expr {
             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)
             };

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
     }
 }

src/libsyntax/print/pprust.rs

@@ -1471,7 +1471,7 @@ fn print_field(s: &mut State, field: &ast::Field) -> 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 @@ fn print_field(s: &mut State, field: &ast::Field) -> 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))
         }
       }

src/test/compile-fail/hygienic-label-1.rs

@@ -0,0 +1,19 @@
+// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[feature(macro_rules)];
+
+macro_rules! foo {
+    () => { break 'x; }
+}
+
+pub fn main() {
+    'x: loop { foo!() } //~ ERROR use of undeclared label `x`
+}

src/test/compile-fail/hygienic-label-2.rs

@@ -0,0 +1,19 @@
+// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[feature(macro_rules)];
+
+macro_rules! foo {
+    ($e: expr) => { 'x: loop { $e } }
+}
+
+pub fn main() {
+    foo!(break 'x); //~ ERROR use of undeclared label `x`
+}

src/test/compile-fail/hygienic-label-3.rs

@@ -0,0 +1,21 @@
+// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[feature(macro_rules)];
+
+macro_rules! foo {
+    () => { break 'x; }
+}
+
+pub fn main() {
+    'x: for _ in range(0,1) {
+        foo!() //~ ERROR use of undeclared label `x`
+    };
+}

src/test/compile-fail/hygienic-label-4.rs

@@ -0,0 +1,19 @@
+// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[feature(macro_rules)];
+
+macro_rules! foo {
+    ($e: expr) => { 'x: for _ in range(0,1) { $e } }
+}
+
+pub fn main() {
+    foo!(break 'x); //~ ERROR use of undeclared label `x`
+}

src/test/run-pass/hygienic-labels-in-let.rs

@@ -0,0 +1,59 @@
+// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[feature(macro_rules)];
+
+macro_rules! loop_x {
+    ($e: expr) => {
+        // $e shouldn't be able to interact with this 'x
+        'x: loop { $e }
+    }
+}
+
+macro_rules! run_once {
+    ($e: expr) => {
+        // ditto
+        'x: for _ in range(0, 1) { $e }
+    }
+}
+
+pub fn main() {
+    let mut i = 0i;
+
+    let j = {
+        'x: loop {
+            // this 'x should refer to the outer loop, lexically
+            loop_x!(break 'x);
+            i += 1;
+        }
+        i + 1
+    };
+    assert_eq!(j, 1i);
+
+    let k = {
+        'x: for _ in range(0, 1) {
+            // ditto
+            loop_x!(break 'x);
+            i += 1;
+        }
+        i + 1
+    };
+    assert_eq!(k, 1i);
+
+    let n = {
+        'x: for _ in range(0, 1) {
+            // ditto
+            run_once!(continue 'x);
+            i += 1;
+        }
+        i + 1
+    };
+    assert_eq!(n, 1i);
+}

src/test/run-pass/hygienic-labels.rs

@@ -0,0 +1,45 @@
+// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[feature(macro_rules)];
+
+macro_rules! loop_x {
+    ($e: expr) => {
+        // $e shouldn't be able to interact with this 'x
+        'x: loop { $e }
+    }
+}
+
+macro_rules! run_once {
+    ($e: expr) => {
+        // ditto
+        'x: for _ in range(0, 1) { $e }
+    }
+}
+
+pub fn main() {
+    'x: for _ in range(0, 1) {
+        // this 'x should refer to the outer loop, lexically
+        loop_x!(break 'x);
+        fail!("break doesn't act hygienically inside for loop");
+    }
+
+    'x: loop {
+        // ditto
+        loop_x!(break 'x);
+        fail!("break doesn't act hygienically inside infinite loop");
+    }
+
+    'x: for _ in range(0, 1) {
+        // ditto
+        run_once!(continue 'x);
+        fail!("continue doesn't act hygienically inside for loop");
+    }
+}