diff --git a/src/librustc/middle/cfg/construct.rs b/src/librustc/middle/cfg/construct.rs index d4eb72ac577af..d575c94942105 100644 --- a/src/librustc/middle/cfg/construct.rs +++ b/src/librustc/middle/cfg/construct.rs @@ -490,7 +490,7 @@ impl CFGBuilder { fn find_scope(&self, expr: @ast::Expr, - label: Option) -> LoopScope { + label: Option) -> LoopScope { match label { None => { return *self.loop_scopes.last().unwrap(); diff --git a/src/librustc/middle/dataflow.rs b/src/librustc/middle/dataflow.rs index ba79d71bcb64f..e71079b6dd0f3 100644 --- a/src/librustc/middle/dataflow.rs +++ b/src/librustc/middle/dataflow.rs @@ -770,7 +770,7 @@ impl<'a, O:DataFlowOperator> PropagationContext<'a, O> { fn find_scope<'a>(&self, expr: &ast::Expr, - label: Option, + label: Option, loop_scopes: &'a mut ~[LoopScope]) -> &'a mut LoopScope { let index = match label { None => { diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 3b8eb682065bf..3b9f81adbac26 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -747,7 +747,7 @@ impl Liveness { } pub fn find_loop_scope(&self, - opt_label: Option, + opt_label: Option, id: NodeId, sp: Span) -> NodeId { diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index 72967ff8195c1..b3bbeae92100d 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -5206,13 +5206,13 @@ impl Resolver { 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 @@ impl Resolver { 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))) diff --git a/src/librustc/middle/trans/controlflow.rs b/src/librustc/middle/trans/controlflow.rs index 15f34ed2d196a..ad575eb0effcb 100644 --- a/src/librustc/middle/trans/controlflow.rs +++ b/src/librustc/middle/trans/controlflow.rs @@ -25,7 +25,7 @@ use util::ppaux::Repr; 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, + opt_label: Option, 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) + label_opt: Option) -> &'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) + label_opt: Option) -> &'a Block<'a> { return trans_break_cont(bcx, expr_id, label_opt, cleanup::EXIT_LOOP); } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index db1243b18bc2d..d9d207ae6db35 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), - ExprAgain(Option), + ExprBreak(Option), + ExprAgain(Option), 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 1e0bfb0d3e9f0..b49f9fb3a384d 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 ['];` 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, + loop_block: P, + fld: &mut MacroExpander) -> (Option, P) { + 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 456533de5e9a4..edd875a57a749 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(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 e150d1685de24..5f6eb86c3c80c 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -353,6 +353,23 @@ fn fold_arg_(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(tts: &[TokenTree], fld: &mut T) -> ~[TokenTree] { tts.map(|tt| { match *tt { @@ -376,6 +393,7 @@ fn maybe_fold_ident(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(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 fed2034cd26ae..31e16cd8c7d4d 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 b264e8d7467ba..5da4e4c44dfdb 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 f934a9022a65c..8fb813407d093 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)) } } diff --git a/src/test/compile-fail/hygienic-label-1.rs b/src/test/compile-fail/hygienic-label-1.rs new file mode 100644 index 0000000000000..d2720bc457050 --- /dev/null +++ b/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 or the MIT license +// , 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` +} diff --git a/src/test/compile-fail/hygienic-label-2.rs b/src/test/compile-fail/hygienic-label-2.rs new file mode 100644 index 0000000000000..c97317217fc5b --- /dev/null +++ b/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 or the MIT license +// , 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` +} diff --git a/src/test/compile-fail/hygienic-label-3.rs b/src/test/compile-fail/hygienic-label-3.rs new file mode 100644 index 0000000000000..d5284f5766e0e --- /dev/null +++ b/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 or the MIT license +// , 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` + }; +} diff --git a/src/test/compile-fail/hygienic-label-4.rs b/src/test/compile-fail/hygienic-label-4.rs new file mode 100644 index 0000000000000..79ac46ac59ab4 --- /dev/null +++ b/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 or the MIT license +// , 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` +} diff --git a/src/test/run-pass/hygienic-labels-in-let.rs b/src/test/run-pass/hygienic-labels-in-let.rs new file mode 100644 index 0000000000000..125160c368511 --- /dev/null +++ b/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 or the MIT license +// , 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); +} diff --git a/src/test/run-pass/hygienic-labels.rs b/src/test/run-pass/hygienic-labels.rs new file mode 100644 index 0000000000000..7d341a6762358 --- /dev/null +++ b/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 or the MIT license +// , 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"); + } +}