Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make break and continue hygienic #12338

Merged
merged 1 commit into from
Feb 24, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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