Skip to content

Commit

Permalink
auto merge of #12338 : edwardw/rust/hygienic-break-continue, r=cmr
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Feb 23, 2014
2 parents cbed332 + 386db05 commit 329fcd4
Show file tree
Hide file tree
Showing 18 changed files with 262 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/librustc/middle/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ impl CFGBuilder {

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();
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/dataflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,7 @@ impl<'a, O:DataFlowOperator> PropagationContext<'a, O> {

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 => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ impl Liveness {
}

pub fn find_loop_scope(&self,
opt_label: Option<Name>,
opt_label: Option<Ident>,
id: NodeId,
sp: Span)
-> NodeId {
Expand Down
9 changes: 5 additions & 4 deletions src/librustc/middle/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, ());
Expand All @@ -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)))
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/middle/trans/controlflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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);
}
Expand Down
13 changes: 9 additions & 4 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
31 changes: 29 additions & 2 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
};
Expand Down Expand Up @@ -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) =>
Expand Down
5 changes: 3 additions & 2 deletions src/libsyntax/ext/tt/macro_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
22 changes: 20 additions & 2 deletions src/libsyntax/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
}
}
Expand Down Expand Up @@ -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)))
}
Expand Down
6 changes: 3 additions & 3 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
};
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)
};
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/parse/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/print/pprust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand All @@ -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))
}
}
Expand Down
19 changes: 19 additions & 0 deletions src/test/compile-fail/hygienic-label-1.rs
Original file line number Diff line number Diff line change
@@ -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`
}
19 changes: 19 additions & 0 deletions src/test/compile-fail/hygienic-label-2.rs
Original file line number Diff line number Diff line change
@@ -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`
}
21 changes: 21 additions & 0 deletions src/test/compile-fail/hygienic-label-3.rs
Original file line number Diff line number Diff line change
@@ -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`
};
}
19 changes: 19 additions & 0 deletions src/test/compile-fail/hygienic-label-4.rs
Original file line number Diff line number Diff line change
@@ -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`
}
Loading

0 comments on commit 329fcd4

Please sign in to comment.