From b8c5e4650598f43a2e362bcf6f4e919440a997bb Mon Sep 17 00:00:00 2001 From: John Clements Date: Wed, 25 Jun 2014 15:20:01 -0700 Subject: working on hygiene --- src/libsyntax/ext/expand.rs | 62 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 12 deletions(-) (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 321c56d4bbf..11f50d685f8 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -31,6 +31,7 @@ use util::small_vector::SmallVector; use std::gc::{Gc, GC}; + pub fn expand_expr(e: Gc, fld: &mut MacroExpander) -> Gc { match e.node { // expr_mac should really be expr_ext or something; it's the @@ -130,8 +131,6 @@ pub fn expand_expr(e: Gc, fld: &mut MacroExpander) -> Gc { // From: `[':] for in ` // FIXME #6993: change type of opt_ident to Option ast::ExprForLoop(src_pat, src_expr, src_loop_block, opt_ident) => { - // Expand any interior macros etc. - // NB: we don't fold pats yet. Curious. let span = e.span; @@ -281,7 +280,7 @@ macro_rules! with_exts_frame ( ) // When we enter a module, record it, for the sake of `module!` -pub fn expand_item(it: Gc, fld: &mut MacroExpander) +fn expand_item(it: Gc, fld: &mut MacroExpander) -> SmallVector> { let it = expand_item_modifiers(it, fld); @@ -386,13 +385,13 @@ fn expand_item_modifiers(mut it: Gc, fld: &mut MacroExpander) } // does this attribute list contain "macro_escape" ? -pub fn contains_macro_escape(attrs: &[ast::Attribute]) -> bool { +fn contains_macro_escape(attrs: &[ast::Attribute]) -> bool { attr::contains_name(attrs, "macro_escape") } // Support for item-position macro invocations, exactly the same // logic as for expression-position macro invocations. -pub fn expand_item_mac(it: Gc, fld: &mut MacroExpander) +fn expand_item_mac(it: Gc, fld: &mut MacroExpander) -> SmallVector> { let (pth, tts) = match it.node { ItemMac(codemap::Spanned { @@ -498,7 +497,7 @@ pub fn expand_item_mac(it: Gc, fld: &mut MacroExpander) } // expand a stmt -pub fn expand_stmt(s: &Stmt, fld: &mut MacroExpander) -> SmallVector> { +fn expand_stmt(s: &Stmt, fld: &mut MacroExpander) -> SmallVector> { // why the copying here and not in expand_expr? // looks like classic changed-in-only-one-place let (pth, tts, semi) = match s.node { @@ -659,6 +658,42 @@ fn expand_non_macro_stmt(s: &Stmt, fld: &mut MacroExpander) } } +fn expand_arm(arm: &ast::Arm, fld: &mut MacroExpander) -> ast::Arm { + if a.pats.len() == 0 { + fail!("encountered match arm with 0 patterns"); + } + let first_pat = match a.pats.get(0) { + + } + // code duplicated from 'let', above. Perhaps this can be lifted + // into a separate function: + let expanded_pat = fld.fold_pat(pat); + let mut name_finder = new_name_finder(Vec::new()); + name_finder.visit_pat(&*expanded_pat,()); + let mut new_pending_renames = Vec::new(); + for ident in name_finder.ident_accumulator.iter() { + let new_name = fresh_name(ident); + new_pending_renames.push((*ident,new_name)); + } + let rewritten_pat = { + let mut rename_fld = + renames_to_fold(&mut new_pending_renames); + // rewrite the pattern using the new names (the old + // ones have already been applied): + rename_fld.fold_pat(expanded_pat) + }; + + let bound_names + ast::Arm { + attrs: a.attrs.iter().map(|x| self.fold_attribute(*x)).collect(), + pats: a.pats.iter().map(|x| self.fold_pat(*x)).collect(), + guard: a.guard.map(|x| self.fold_expr(x)), + body: self.fold_expr(a.body), + } +} + + + // a visitor that extracts the pat_ident (binding) paths // from a given thingy and puts them in a mutable // array (passed in to the traversal). @@ -711,14 +746,14 @@ fn new_name_finder(idents: Vec ) -> NameFinderContext { } // expand a block. pushes a new exts_frame, then calls expand_block_elts -pub fn expand_block(blk: &Block, fld: &mut MacroExpander) -> P { +fn expand_block(blk: &Block, fld: &mut MacroExpander) -> P { // see note below about treatment of exts table with_exts_frame!(fld.extsbox,false, expand_block_elts(blk, fld)) } // expand the elements of a block. -pub fn expand_block_elts(b: &Block, fld: &mut MacroExpander) -> P { +fn expand_block_elts(b: &Block, fld: &mut MacroExpander) -> P { let new_view_items = b.view_items.iter().map(|x| fld.fold_view_item(x)).collect(); let new_stmts = b.stmts.iter().flat_map(|x| { @@ -747,7 +782,7 @@ pub fn expand_block_elts(b: &Block, fld: &mut MacroExpander) -> P { }) } -pub fn expand_pat(p: Gc, fld: &mut MacroExpander) -> Gc { +fn expand_pat(p: Gc, fld: &mut MacroExpander) -> Gc { let (pth, tts) = match p.node { PatMac(ref mac) => { match mac.node { @@ -842,13 +877,13 @@ impl<'a> Folder for IdentRenamer<'a> { // given a mutable list of renames, return a tree-folder that applies those // renames. -pub fn renames_to_fold<'a>(renames: &'a mut RenameList) -> IdentRenamer<'a> { +fn renames_to_fold<'a>(renames: &'a mut RenameList) -> IdentRenamer<'a> { IdentRenamer { renames: renames, } } -pub fn new_span(cx: &ExtCtxt, sp: Span) -> Span { +fn new_span(cx: &ExtCtxt, sp: Span) -> Span { /* this discards information in the case of macro-defining macros */ Span { lo: sp.lo, @@ -883,6 +918,10 @@ impl<'a, 'b> Folder for MacroExpander<'a, 'b> { expand_block(&*block, self) } + fn fold_arm(&mut self, arm: &ast::Arm) -> ast::Arm { + expand_arm(arm, self) + } + fn new_span(&mut self, span: Span) -> Span { new_span(self.cx, span) } @@ -1248,7 +1287,6 @@ mod test { // FIXME #9384, match variable hygiene. Should expand into // fn z() {match 8 {x_1 => {match 9 {x_2 | x_2 => x_2 + x_1}}}} - #[ignore] #[test] fn issue_9384(){ run_renaming_test( &("macro_rules! bad_macro (($ex:expr) => ({match 9 {x | x => x + $ex}})) -- cgit 1.4.1-3-g733a5 From 84e8143c4f423011cd337d2a7bfdc54a753eea81 Mon Sep 17 00:00:00 2001 From: John Clements Date: Thu, 26 Jun 2014 13:18:52 -0700 Subject: get rid of needless wrapper function --- src/libsyntax/ext/expand.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 11f50d685f8..073de40382a 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -251,7 +251,7 @@ fn expand_loop_block(loop_block: P, // the same context will pick that up in the deferred renaming pass // and be renamed incorrectly. let mut rename_list = vec!(rename); - let mut rename_fld = renames_to_fold(&mut rename_list); + let mut rename_fld = IdentRenamer{renames: &mut rename_list}; let renamed_ident = rename_fld.fold_ident(label); // The rename *must* be added to the enclosed syntax context for @@ -624,7 +624,7 @@ fn expand_non_macro_stmt(s: &Stmt, fld: &mut MacroExpander) } let rewritten_pat = { let mut rename_fld = - renames_to_fold(&mut new_pending_renames); + IdentRenamer{renames: &mut new_pending_renames}; // rewrite the pattern using the new names (the old // ones have already been applied): rename_fld.fold_pat(expanded_pat) @@ -676,8 +676,7 @@ fn expand_arm(arm: &ast::Arm, fld: &mut MacroExpander) -> ast::Arm { new_pending_renames.push((*ident,new_name)); } let rewritten_pat = { - let mut rename_fld = - renames_to_fold(&mut new_pending_renames); + let mut rename_fld = IdentRenamer{renames:&mut new_pending_renames}; // rewrite the pattern using the new names (the old // ones have already been applied): rename_fld.fold_pat(expanded_pat) @@ -757,17 +756,19 @@ fn expand_block_elts(b: &Block, fld: &mut MacroExpander) -> P { let new_view_items = b.view_items.iter().map(|x| fld.fold_view_item(x)).collect(); let new_stmts = b.stmts.iter().flat_map(|x| { + // perform all pending renames let renamed_stmt = { let pending_renames = &mut fld.extsbox.info().pending_renames; - let mut rename_fld = renames_to_fold(pending_renames); + let mut rename_fld = IdentRenamer{renames:pending_renames}; rename_fld.fold_stmt(&**x).expect_one("rename_fold didn't return one value") }; + // expand macros in the statement fld.fold_stmt(&*renamed_stmt).move_iter() }).collect(); let new_expr = b.expr.map(|x| { let expr = { let pending_renames = &mut fld.extsbox.info().pending_renames; - let mut rename_fld = renames_to_fold(pending_renames); + let mut rename_fld = IdentRenamer{renames:pending_renames}; rename_fld.fold_expr(x) }; fld.fold_expr(expr) @@ -859,6 +860,7 @@ fn expand_pat(p: Gc, fld: &mut MacroExpander) -> Gc { } } +// a tree-folder that applies every rename in its (mutable) list pub struct IdentRenamer<'a> { renames: &'a mut RenameList, } @@ -875,14 +877,6 @@ impl<'a> Folder for IdentRenamer<'a> { } } -// given a mutable list of renames, return a tree-folder that applies those -// renames. -fn renames_to_fold<'a>(renames: &'a mut RenameList) -> IdentRenamer<'a> { - IdentRenamer { - renames: renames, - } -} - fn new_span(cx: &ExtCtxt, sp: Span) -> Span { /* this discards information in the case of macro-defining macros */ Span { -- cgit 1.4.1-3-g733a5 From a18a63185ca79126842d94505746fccef3ade1b8 Mon Sep 17 00:00:00 2001 From: John Clements Date: Thu, 26 Jun 2014 13:19:34 -0700 Subject: WIP match hygiene, compiles --- src/libsyntax/ext/expand.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 073de40382a..c40049a58a7 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -659,15 +659,15 @@ fn expand_non_macro_stmt(s: &Stmt, fld: &mut MacroExpander) } fn expand_arm(arm: &ast::Arm, fld: &mut MacroExpander) -> ast::Arm { - if a.pats.len() == 0 { + if arm.pats.len() == 0 { fail!("encountered match arm with 0 patterns"); } - let first_pat = match a.pats.get(0) { - - } + // all of the pats must have the same set of bindings, so use the + // first one to extract them and generate new names: + let first_pat = arm.pats.get(0); // code duplicated from 'let', above. Perhaps this can be lifted // into a separate function: - let expanded_pat = fld.fold_pat(pat); + let expanded_pat = fld.fold_pat(*first_pat); let mut name_finder = new_name_finder(Vec::new()); name_finder.visit_pat(&*expanded_pat,()); let mut new_pending_renames = Vec::new(); @@ -681,13 +681,13 @@ fn expand_arm(arm: &ast::Arm, fld: &mut MacroExpander) -> ast::Arm { // ones have already been applied): rename_fld.fold_pat(expanded_pat) }; - - let bound_names - ast::Arm { - attrs: a.attrs.iter().map(|x| self.fold_attribute(*x)).collect(), - pats: a.pats.iter().map(|x| self.fold_pat(*x)).collect(), - guard: a.guard.map(|x| self.fold_expr(x)), - body: self.fold_expr(a.body), + /* + */ + ast::Arm { + attrs: arm.attrs.iter().map(|x| fld.fold_attribute(*x)).collect(), + pats: arm.pats.iter().map(|x| fld.fold_pat(*x)).collect(), + guard: arm.guard.map(|x| fld.fold_expr(x)), + body: fld.fold_expr(arm.body), } } -- cgit 1.4.1-3-g733a5 From 977b380cd2bb45a6556f77b79274460377c6ae94 Mon Sep 17 00:00:00 2001 From: John Clements Date: Thu, 26 Jun 2014 14:39:54 -0700 Subject: cleanup and shiny new more-functional interface --- src/libsyntax/ext/expand.rs | 91 ++++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 55 deletions(-) (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index c40049a58a7..69f4fdb9f3f 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -614,14 +614,10 @@ fn expand_non_macro_stmt(s: &Stmt, fld: &mut MacroExpander) // oh dear heaven... this is going to include the enum // names, as well... but that should be okay, as long as // the new names are gensyms for the old ones. - let mut name_finder = new_name_finder(Vec::new()); - name_finder.visit_pat(&*expanded_pat,()); // generate fresh names, push them to a new pending list - let mut new_pending_renames = Vec::new(); - for ident in name_finder.ident_accumulator.iter() { - let new_name = fresh_name(ident); - new_pending_renames.push((*ident,new_name)); - } + let idents = pattern_bindings(expanded_pat); + let mut new_pending_renames = + idents.iter().map(|ident| (*ident, fresh_name(ident))).collect(); let rewritten_pat = { let mut rename_fld = IdentRenamer{renames: &mut new_pending_renames}; @@ -668,10 +664,8 @@ fn expand_arm(arm: &ast::Arm, fld: &mut MacroExpander) -> ast::Arm { // code duplicated from 'let', above. Perhaps this can be lifted // into a separate function: let expanded_pat = fld.fold_pat(*first_pat); - let mut name_finder = new_name_finder(Vec::new()); - name_finder.visit_pat(&*expanded_pat,()); let mut new_pending_renames = Vec::new(); - for ident in name_finder.ident_accumulator.iter() { + for ident in pattern_bindings(expanded_pat).iter() { let new_name = fresh_name(ident); new_pending_renames.push((*ident,new_name)); } @@ -681,8 +675,6 @@ fn expand_arm(arm: &ast::Arm, fld: &mut MacroExpander) -> ast::Arm { // ones have already been applied): rename_fld.fold_pat(expanded_pat) }; - /* - */ ast::Arm { attrs: arm.attrs.iter().map(|x| fld.fold_attribute(*x)).collect(), pats: arm.pats.iter().map(|x| fld.fold_pat(*x)).collect(), @@ -695,7 +687,7 @@ fn expand_arm(arm: &ast::Arm, fld: &mut MacroExpander) -> ast::Arm { // a visitor that extracts the pat_ident (binding) paths // from a given thingy and puts them in a mutable -// array (passed in to the traversal). +// array #[deriving(Clone)] struct NameFinderContext { ident_accumulator: Vec , @@ -735,13 +727,11 @@ impl Visitor<()> for NameFinderContext { } -// return a visitor that extracts the pat_ident paths -// from a given thingy and puts them in a mutable -// array (passed in to the traversal) -fn new_name_finder(idents: Vec ) -> NameFinderContext { - NameFinderContext { - ident_accumulator: idents, - } +// find the pat_ident paths in a pattern +fn pattern_bindings(pat : &ast::Pat) -> Vec { + let mut name_finder = NameFinderContext{ident_accumulator:Vec::new()}; + name_finder.visit_pat(pat,()); + name_finder.ident_accumulator } // expand a block. pushes a new exts_frame, then calls expand_block_elts @@ -1046,7 +1036,8 @@ fn original_span(cx: &ExtCtxt) -> Gc { #[cfg(test)] mod test { - use super::{new_name_finder, expand_crate, contains_macro_escape}; + use super::{pattern_bindings, expand_crate, contains_macro_escape}; + use super::{NameFinderContext}; use ast; use ast::{Attribute_, AttrOuter, MetaWord}; use attr; @@ -1076,22 +1067,22 @@ mod test { match *expr { ast::Expr{id:_,span:_,node:ast::ExprPath(ref p)} => { self.path_accumulator.push(p.clone()); - // not calling visit_path, should be fine. + // not calling visit_path, but it should be fine. } _ => visit::walk_expr(self,expr,()) } } } - // return a visitor that extracts the paths - // from a given thingy and puts them in a mutable - // array (passed in to the traversal) - fn new_path_finder(paths: Vec ) -> PathExprFinderContext { - PathExprFinderContext { - path_accumulator: paths - } + // find the variable references in a crate + fn crate_varrefs(the_crate : &ast::Crate) -> Vec { + let mut path_finder = PathExprFinderContext{path_accumulator:Vec::new()}; + visit::walk_crate(&mut path_finder, the_crate, ()); + path_finder.path_accumulator } + + // these following tests are quite fragile, in that they don't test what // *kind* of failure occurs. @@ -1183,6 +1174,14 @@ mod test { expand_crate(&ps,cfg,vec!(),vec!(),crate_ast) } + // find the pat_ident paths in a crate + fn crate_bindings(the_crate : &ast::Crate) -> Vec { + let mut name_finder = NameFinderContext{ident_accumulator:Vec::new()}; + visit::walk_crate(&mut name_finder, the_crate, ()); + name_finder.ident_accumulator + } + + //fn expand_and_resolve(crate_str: @str) -> ast::crate { //let expanded_ast = expand_crate_str(crate_str); // println!("expanded: {:?}\n",expanded_ast); @@ -1315,15 +1314,8 @@ mod test { (ref str,ref conns, bic) => (str.to_owned(), conns.clone(), bic) }; let cr = expand_crate_str(teststr.to_string()); - // find the bindings: - let mut name_finder = new_name_finder(Vec::new()); - visit::walk_crate(&mut name_finder,&cr,()); - let bindings = name_finder.ident_accumulator; - - // find the varrefs: - let mut path_finder = new_path_finder(Vec::new()); - visit::walk_crate(&mut path_finder,&cr,()); - let varrefs = path_finder.path_accumulator; + let bindings = crate_bindings(&cr); + let varrefs = crate_varrefs(&cr); // must be one check clause for each binding: assert_eq!(bindings.len(),bound_connections.len()); @@ -1392,10 +1384,7 @@ foo_module!() ".to_string(); let cr = expand_crate_str(crate_str); // find the xx binding - let mut name_finder = new_name_finder(Vec::new()); - visit::walk_crate(&mut name_finder, &cr, ()); - let bindings = name_finder.ident_accumulator; - + let bindings = crate_bindings(&cr); let cxbinds: Vec<&ast::Ident> = bindings.iter().filter(|b| { let ident = token::get_ident(**b); @@ -1408,10 +1397,7 @@ foo_module!() _ => fail!("expected just one binding for ext_cx") }; let resolved_binding = mtwt::resolve(*cxbind); - // find all the xx varrefs: - let mut path_finder = new_path_finder(Vec::new()); - visit::walk_crate(&mut path_finder, &cr, ()); - let varrefs = path_finder.path_accumulator; + let varrefs = crate_varrefs(&cr); // the xx binding should bind all of the xx varrefs: for (idx,v) in varrefs.iter().filter(|p| { @@ -1437,10 +1423,8 @@ foo_module!() fn pat_idents(){ let pat = string_to_pat( "(a,Foo{x:c @ (b,9),y:Bar(4,d)})".to_string()); - let mut pat_idents = new_name_finder(Vec::new()); - pat_idents.visit_pat(pat, ()); - assert_eq!(pat_idents.ident_accumulator, - strs_to_idents(vec!("a","c","b","d"))); + let idents = pattern_bindings(pat); + assert_eq!(idents, strs_to_idents(vec!("a","c","b","d"))); } // test the list of identifier patterns gathered by the visitor. Note that @@ -1450,11 +1434,8 @@ foo_module!() fn crate_idents(){ let the_crate = string_to_crate("fn main (a : int) -> int {|b| { match 34 {None => 3, Some(i) | i => j, Foo{k:z,l:y} => \"banana\"}} }".to_string()); - let mut idents = new_name_finder(Vec::new()); - //visit::walk_crate(&mut idents, &the_crate, ()); - idents.visit_mod(&the_crate.module, the_crate.span, ast::CRATE_NODE_ID, ()); - assert_eq!(idents.ident_accumulator, - strs_to_idents(vec!("a","b","None","i","i","z","y"))); + let idents = crate_bindings(&the_crate); + assert_eq!(idents, strs_to_idents(vec!("a","b","None","i","i","z","y"))); } -- cgit 1.4.1-3-g733a5 From 47eec97cdab94989cc4c5f20a1e1f2310df997aa Mon Sep 17 00:00:00 2001 From: John Clements Date: Thu, 26 Jun 2014 15:37:18 -0700 Subject: remove unnecessary abstraction --- src/libsyntax/ext/expand.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 69f4fdb9f3f..832325f897b 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -988,35 +988,30 @@ impl Folder for Marker { } } -// just a convenience: -fn new_mark_folder(m: Mrk) -> Marker { - Marker {mark: m} -} - // apply a given mark to the given token trees. Used prior to expansion of a macro. fn mark_tts(tts: &[TokenTree], m: Mrk) -> Vec { - fold_tts(tts, &mut new_mark_folder(m)) + fold_tts(tts, &mut Marker{mark:m}) } // apply a given mark to the given expr. Used following the expansion of a macro. fn mark_expr(expr: Gc, m: Mrk) -> Gc { - new_mark_folder(m).fold_expr(expr) + Marker{mark:m}.fold_expr(expr) } // apply a given mark to the given pattern. Used following the expansion of a macro. fn mark_pat(pat: Gc, m: Mrk) -> Gc { - new_mark_folder(m).fold_pat(pat) + Marker{mark:m}.fold_pat(pat) } // apply a given mark to the given stmt. Used following the expansion of a macro. fn mark_stmt(expr: &ast::Stmt, m: Mrk) -> Gc { - new_mark_folder(m).fold_stmt(expr) + Marker{mark:m}.fold_stmt(expr) .expect_one("marking a stmt didn't return a stmt") } // apply a given mark to the given item. Used following the expansion of a macro. fn mark_item(expr: Gc, m: Mrk) -> SmallVector> { - new_mark_folder(m).fold_item(expr) + Marker{mark:m}.fold_item(expr) } fn original_span(cx: &ExtCtxt) -> Gc { -- cgit 1.4.1-3-g733a5 From 7bad96e742b187b671500702bccb37ab1724bae5 Mon Sep 17 00:00:00 2001 From: John Clements Date: Thu, 26 Jun 2014 15:37:45 -0700 Subject: improve match test case to include guard --- src/libsyntax/ext/expand.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 832325f897b..941a4bb68c2 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -1274,13 +1274,13 @@ mod test { } // FIXME #9384, match variable hygiene. Should expand into - // fn z() {match 8 {x_1 => {match 9 {x_2 | x_2 => x_2 + x_1}}}} + // fn z() {match 8 {x_1 => {match 9 {x_2 | x_2 if x_2 == x_1 => x_2 + x_1}}}} #[test] fn issue_9384(){ run_renaming_test( - &("macro_rules! bad_macro (($ex:expr) => ({match 9 {x | x => x + $ex}})) - fn z() {match 8 {x => bad_macro!(_x)}}", + &("macro_rules! bad_macro (($ex:expr) => ({match 9 {x | x if x == $ex => x + $ex}})) + fn z() {match 8 {x => bad_macro!(x)}}", // NB: the third "binding" is the repeat of the second one. - vec!(vec!(1),vec!(0),vec!(0)), + vec!(vec!(1,3),vec!(0,2),vec!(0,2)), true), 0) } -- cgit 1.4.1-3-g733a5 From 4b46c700f4bf04331b1858e7fe3c363c0451c4cb Mon Sep 17 00:00:00 2001 From: John Clements Date: Thu, 26 Jun 2014 16:49:13 -0700 Subject: hygiene for match-bound vars now implemented Closes #9384 --- src/libsyntax/ext/expand.rs | 57 ++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 24 deletions(-) (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 941a4bb68c2..c2bb2cd7347 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -608,7 +608,7 @@ fn expand_non_macro_stmt(s: &Stmt, fld: &mut MacroExpander) span: span, source: source, } = **local; - // expand the pat (it might contain exprs... #:(o)> + // expand the pat (it might contain macro uses): let expanded_pat = fld.fold_pat(pat); // find the pat_idents in the pattern: // oh dear heaven... this is going to include the enum @@ -618,11 +618,11 @@ fn expand_non_macro_stmt(s: &Stmt, fld: &mut MacroExpander) let idents = pattern_bindings(expanded_pat); let mut new_pending_renames = idents.iter().map(|ident| (*ident, fresh_name(ident))).collect(); + // rewrite the pattern using the new names (the old + // ones have already been applied): let rewritten_pat = { - let mut rename_fld = - IdentRenamer{renames: &mut new_pending_renames}; - // rewrite the pattern using the new names (the old - // ones have already been applied): + // nested binding to allow borrow to expire: + let mut rename_fld = IdentRenamer{renames: &mut new_pending_renames}; rename_fld.fold_pat(expanded_pat) }; // add them to the existing pending renames: @@ -655,31 +655,38 @@ fn expand_non_macro_stmt(s: &Stmt, fld: &mut MacroExpander) } fn expand_arm(arm: &ast::Arm, fld: &mut MacroExpander) -> ast::Arm { - if arm.pats.len() == 0 { + // expand pats... they might contain macro uses: + let expanded_pats : Vec> = arm.pats.iter().map(|pat| fld.fold_pat(*pat)).collect(); + if expanded_pats.len() == 0 { fail!("encountered match arm with 0 patterns"); } // all of the pats must have the same set of bindings, so use the // first one to extract them and generate new names: - let first_pat = arm.pats.get(0); + let first_pat = expanded_pats.get(0); // code duplicated from 'let', above. Perhaps this can be lifted // into a separate function: - let expanded_pat = fld.fold_pat(*first_pat); - let mut new_pending_renames = Vec::new(); - for ident in pattern_bindings(expanded_pat).iter() { - let new_name = fresh_name(ident); - new_pending_renames.push((*ident,new_name)); - } - let rewritten_pat = { - let mut rename_fld = IdentRenamer{renames:&mut new_pending_renames}; - // rewrite the pattern using the new names (the old - // ones have already been applied): - rename_fld.fold_pat(expanded_pat) - }; + let idents = pattern_bindings(*first_pat); + let mut new_pending_renames = + idents.iter().map(|id| (*id,fresh_name(id))).collect(); + // rewrite all of the patterns using the new names (the old + // ones have already been applied). Note that we depend here + // on the guarantee that after expansion, there can't be any + // Path expressions (a.k.a. varrefs) left in the pattern. If + // this were false, we'd need to apply this renaming only to + // the bindings, and not to the varrefs, using a more targeted + // fold-er. + let mut rename_fld = IdentRenamer{renames:&mut new_pending_renames}; + let rewritten_pats = + expanded_pats.iter().map(|pat| rename_fld.fold_pat(*pat)).collect(); + // apply renaming and then expansion to the guard and the body: + let rewritten_guard = + arm.guard.map(|g| fld.fold_expr(rename_fld.fold_expr(g))); + let rewritten_body = fld.fold_expr(rename_fld.fold_expr(arm.body)); ast::Arm { attrs: arm.attrs.iter().map(|x| fld.fold_attribute(*x)).collect(), - pats: arm.pats.iter().map(|x| fld.fold_pat(*x)).collect(), - guard: arm.guard.map(|x| fld.fold_expr(x)), - body: fld.fold_expr(arm.body), + pats: rewritten_pats, + guard: rewritten_guard, + body: rewritten_body, } } @@ -851,6 +858,8 @@ fn expand_pat(p: Gc, fld: &mut MacroExpander) -> Gc { } // a tree-folder that applies every rename in its (mutable) list +// to every identifier, including both bindings and varrefs +// (and lots of things that will turn out to be neither) pub struct IdentRenamer<'a> { renames: &'a mut RenameList, } @@ -1335,8 +1344,8 @@ mod test { invalid_name); if !(varref_name==binding_name) { println!("uh oh, should match but doesn't:"); - println!("varref: {:?}",varref); - println!("binding: {:?}", *bindings.get(binding_idx)); + println!("varref #{:?}: {:?}",idx, varref); + println!("binding #{:?}: {:?}", binding_idx, *bindings.get(binding_idx)); mtwt::with_sctable(|x| mtwt::display_sctable(x)); } assert_eq!(varref_name,binding_name); -- cgit 1.4.1-3-g733a5 From 235ca1801ec8053b7cab46ed6708c382e58df63b Mon Sep 17 00:00:00 2001 From: John Clements Date: Thu, 26 Jun 2014 16:53:33 -0700 Subject: remove trailing whitespace --- src/libsyntax/ext/expand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index c2bb2cd7347..ab040a5964a 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -687,7 +687,7 @@ fn expand_arm(arm: &ast::Arm, fld: &mut MacroExpander) -> ast::Arm { pats: rewritten_pats, guard: rewritten_guard, body: rewritten_body, - } + } } -- cgit 1.4.1-3-g733a5 From e100d26d1d779b4759f3f754b03a35755cb89b84 Mon Sep 17 00:00:00 2001 From: John Clements Date: Fri, 27 Jun 2014 12:57:48 -0700 Subject: undo helpful attempt to spell-check Yes, that word is spelled \'memoization\' --- src/libsyntax/ext/mtwt.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/mtwt.rs b/src/libsyntax/ext/mtwt.rs index 6c97a8aed1f..6f6eeb3debd 100644 --- a/src/libsyntax/ext/mtwt.rs +++ b/src/libsyntax/ext/mtwt.rs @@ -160,7 +160,7 @@ fn with_resolve_table_mut(op: |&mut ResolveTable| -> T) -> T { } // Resolve a syntax object to a name, per MTWT. -// adding memorization to possibly resolve 500+ seconds in resolve for librustc (!) +// adding memoization to possibly resolve 500+ seconds in resolve for librustc (!) fn resolve_internal(id: Ident, table: &SCTable, resolve_table: &mut ResolveTable) -> Name { -- cgit 1.4.1-3-g733a5 From 351a5fd2b40c2be90f94fe2580903e93353b95ee Mon Sep 17 00:00:00 2001 From: John Clements Date: Fri, 27 Jun 2014 13:10:47 -0700 Subject: added unit and standalone test for 15221, extra debugging output --- src/libsyntax/ext/expand.rs | 31 ++++++++++++++++++++++++++----- src/test/run-pass/issue-15221.rs | 22 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 src/test/run-pass/issue-15221.rs (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index ab040a5964a..0a8ec5bc40e 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -54,7 +54,6 @@ pub fn expand_expr(e: Gc, fld: &mut MacroExpander) -> Gc { } let extname = pth.segments.get(0).identifier; let extnamestr = token::get_ident(extname); - // leaving explicit deref here to highlight unbox op: let marked_after = match fld.extsbox.find(&extname.name) { None => { fld.cx.span_err( @@ -1294,6 +1293,19 @@ mod test { 0) } + // FIXME #15221, somehow pats aren't getting labeled correctly? + // should expand into + // fn main(){let g1_1 = 13; g1_1}} + #[test] fn pat_expand_issue_15221(){ + run_renaming_test( + &("macro_rules! inner ( ($e:pat ) => ($e)) + macro_rules! outer ( ($e:pat ) => (inner!($e))) + fn main() { let outer!(g) = 13; g;}", + vec!(vec!(0)), + true), + 0) + } + // create a really evil test case where a $x appears inside a binding of $x // but *shouldnt* bind because it was inserted by a different macro.... // can't write this test case until we have macro-generating macros. @@ -1343,9 +1355,13 @@ mod test { .ctxt, invalid_name); if !(varref_name==binding_name) { + let varref_idents : Vec + = varref.segments.iter().map(|s| + s.identifier) + .collect(); println!("uh oh, should match but doesn't:"); - println!("varref #{:?}: {:?}",idx, varref); - println!("binding #{:?}: {:?}", binding_idx, *bindings.get(binding_idx)); + println!("varref #{}: {}",idx, varref_idents); + println!("binding #{}: {}", binding_idx, *bindings.get(binding_idx)); mtwt::with_sctable(|x| mtwt::display_sctable(x)); } assert_eq!(varref_name,binding_name); @@ -1360,11 +1376,15 @@ mod test { == binding_name); // temp debugging: if fail { + let varref_idents : Vec + = varref.segments.iter().map(|s| + s.identifier) + .collect(); println!("failure on test {}",test_idx); println!("text of test case: \"{}\"", teststr); println!(""); println!("uh oh, matches but shouldn't:"); - println!("varref: {:?}",varref); + println!("varref: {}",varref_idents); // good lord, you can't make a path with 0 segments, can you? let string = token::get_ident(varref.segments .get(0) @@ -1372,7 +1392,7 @@ mod test { println!("varref's first segment's uint: {}, and string: \"{}\"", varref.segments.get(0).identifier.name, string.get()); - println!("binding: {:?}", *bindings.get(binding_idx)); + println!("binding: {}", *bindings.get(binding_idx)); mtwt::with_sctable(|x| mtwt::display_sctable(x)); } assert!(!fail); @@ -1442,5 +1462,6 @@ foo_module!() assert_eq!(idents, strs_to_idents(vec!("a","b","None","i","i","z","y"))); } + // } diff --git a/src/test/run-pass/issue-15221.rs b/src/test/run-pass/issue-15221.rs new file mode 100644 index 00000000000..e74ba9b85ee --- /dev/null +++ b/src/test/run-pass/issue-15221.rs @@ -0,0 +1,22 @@ +// Copyright 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! inner_bind ( + ( $p:pat, $id:ident) => ({let $p = 13; $id})) + +macro_rules! outer_bind ( + ($p:pat, $id:ident ) => (inner_bind!($p, $id))) + +fn main() { + outer_bind!(g1,g1); +} + -- cgit 1.4.1-3-g733a5 From e3361bcbc259cf25b21e9ae50701d9030ad1f513 Mon Sep 17 00:00:00 2001 From: John Clements Date: Fri, 27 Jun 2014 16:10:23 -0700 Subject: adjust fold to fold over interpolated items/exprs/etc. Closes #15221 --- src/libsyntax/ext/expand.rs | 4 +-- src/libsyntax/fold.rs | 85 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 70 insertions(+), 19 deletions(-) (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 0a8ec5bc40e..b9cedb7a779 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -1281,7 +1281,7 @@ mod test { 0) } - // FIXME #9384, match variable hygiene. Should expand into + // match variable hygiene. Should expand into // fn z() {match 8 {x_1 => {match 9 {x_2 | x_2 if x_2 == x_1 => x_2 + x_1}}}} #[test] fn issue_9384(){ run_renaming_test( @@ -1293,7 +1293,7 @@ mod test { 0) } - // FIXME #15221, somehow pats aren't getting labeled correctly? + // interpolated nodes weren't getting labeled. // should expand into // fn main(){let g1_1 = 13; g1_1}} #[test] fn pat_expand_issue_15221(){ diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index a6e4b4cae1c..c6177ce31f5 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -389,36 +389,80 @@ 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. -pub fn fold_tts(tts: &[TokenTree], fld: &mut T) -> Vec { - tts.iter().map(|tt| { - match *tt { - TTTok(span, ref tok) => - TTTok(span,maybe_fold_ident(tok,fld)), - TTDelim(ref tts) => TTDelim(Rc::new(fold_tts(tts.as_slice(), fld))), - TTSeq(span, ref pattern, ref sep, is_optional) => +pub fn fold_tt(tt: &TokenTree, fld: &mut T) -> TokenTree { + match *tt { + TTTok(span, ref tok) => + TTTok(span, fold_token(tok,fld)), + TTDelim(ref tts) => TTDelim(Rc::new(fold_tts(tts.as_slice(), fld))), + TTSeq(span, ref pattern, ref sep, is_optional) => TTSeq(span, Rc::new(fold_tts(pattern.as_slice(), fld)), - sep.as_ref().map(|tok|maybe_fold_ident(tok,fld)), + sep.as_ref().map(|tok| fold_token(tok,fld)), is_optional), - TTNonterminal(sp,ref ident) => + TTNonterminal(sp,ref ident) => TTNonterminal(sp,fld.fold_ident(*ident)) - } - }).collect() + } +} + +pub fn fold_tts(tts: &[TokenTree], fld: &mut T) -> Vec { + tts.iter().map(|tt| fold_tt(tt,fld)).collect() } -// apply ident folder if it's an ident, otherwise leave it alone -fn maybe_fold_ident(t: &token::Token, fld: &mut T) -> token::Token { + +// apply ident folder if it's an ident, apply other folds to interpolated nodes +fn fold_token(t: &token::Token, fld: &mut T) -> token::Token { match *t { token::IDENT(id, followed_by_colons) => { token::IDENT(fld.fold_ident(id), followed_by_colons) } token::LIFETIME(id) => token::LIFETIME(fld.fold_ident(id)), + token::INTERPOLATED(ref nt) => token::INTERPOLATED(fold_interpolated(nt,fld)), _ => (*t).clone() } } +// apply folder to elements of interpolated nodes +// +// NB: this can occur only when applying a fold to partially expanded code, where +// parsed pieces have gotten implanted ito *other* macro invocations. This is relevant +// for macro hygiene, but possibly not elsewhere. +// +// One problem here occurs because the types for fold_item, fold_stmt, etc. allow the +// folder to return *multiple* items; this is a problem for the nodes here, because +// they insist on having exactly one piece. One solution would be to mangle the fold +// trait to include one-to-many and one-to-one versions of these entry points, but that +// would probably confuse a lot of people and help very few. Instead, I'm just going +// to put in dynamic checks. I think the performance impact of this will be pretty much +// nonexistent. The danger is that someone will apply a fold to a partially expanded +// node, and will be confused by the fact that their "fold_item" or "fold_stmt" isn't +// getting called on NtItem or NtStmt nodes. Hopefully they'll wind up reading this +// comment, and doing something appropriate. +// +// BTW, design choice: I considered just changing the type of, e.g., NtItem to contain +// multiple items, but decided against it when I looked at parse_item_or_view_item and +// tried to figure out what I would do with multiple items there.... +fn fold_interpolated(nt : &token::Nonterminal, fld: &mut T) -> token::Nonterminal { + match *nt { + token::NtItem(item) => + token::NtItem(fld.fold_item(item) + .expect_one("expected fold to produce exactly one item")), + token::NtBlock(block) => token::NtBlock(fld.fold_block(block)), + token::NtStmt(stmt) => + token::NtStmt(fld.fold_stmt(stmt) + .expect_one("expected fold to produce exactly one statement")), + token::NtPat(pat) => token::NtPat(fld.fold_pat(pat)), + token::NtExpr(expr) => token::NtExpr(fld.fold_expr(expr)), + token::NtTy(ty) => token::NtTy(fld.fold_ty(ty)), + token::NtIdent(ref id, is_mod_name) => + token::NtIdent(box fld.fold_ident(**id),is_mod_name), + token::NtMeta(meta_item) => token::NtMeta(fold_meta_item_(meta_item,fld)), + token::NtPath(ref path) => token::NtPath(box fld.fold_path(*path)), + token::NtTT(tt) => token::NtTT(box (GC) fold_tt(tt,fld)), + // it looks to me like we can leave out the matchers: token::NtMatchers(matchers) + _ => (*nt).clone() + } +} + pub fn noop_fold_fn_decl(decl: &FnDecl, fld: &mut T) -> P { P(FnDecl { inputs: decl.inputs.iter().map(|x| fold_arg_(x, fld)).collect(), // bad copy @@ -672,8 +716,15 @@ pub fn noop_fold_crate(c: Crate, folder: &mut T) -> Crate { } } +// fold one item into possibly many items pub fn noop_fold_item(i: &Item, folder: &mut T) -> SmallVector> { + SmallVector::one(box(GC) noop_fold_item_(i,folder)) +} + + +// fold one item into exactly one item +pub fn noop_fold_item_(i: &Item, folder: &mut T) -> Item { let id = folder.new_id(i.id); // Needs to be first, for ast_map. let node = folder.fold_item_underscore(&i.node); let ident = match node { @@ -684,14 +735,14 @@ pub fn noop_fold_item(i: &Item, _ => i.ident }; - SmallVector::one(box(GC) Item { + Item { id: id, ident: folder.fold_ident(ident), attrs: i.attrs.iter().map(|e| folder.fold_attribute(*e)).collect(), node: node, vis: i.vis, span: folder.new_span(i.span) - }) + } } pub fn noop_fold_foreign_item(ni: &ForeignItem, -- cgit 1.4.1-3-g733a5 From 04ced031ad52cfa33f30dbd55f72aff1d95813a3 Mon Sep 17 00:00:00 2001 From: John Clements Date: Fri, 27 Jun 2014 16:11:18 -0700 Subject: comments only --- src/libsyntax/ext/mtwt.rs | 3 ++- src/libsyntax/parse/token.rs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'src/libsyntax/ext') diff --git a/src/libsyntax/ext/mtwt.rs b/src/libsyntax/ext/mtwt.rs index 6f6eeb3debd..48895d34022 100644 --- a/src/libsyntax/ext/mtwt.rs +++ b/src/libsyntax/ext/mtwt.rs @@ -30,6 +30,7 @@ use std::collections::HashMap; // change the semantics--everything here is immutable--but // it should cut down on memory use *a lot*; applying a mark // to a tree containing 50 identifiers would otherwise generate +// 50 new contexts pub struct SCTable { table: RefCell>, mark_memo: RefCell>, @@ -160,7 +161,7 @@ fn with_resolve_table_mut(op: |&mut ResolveTable| -> T) -> T { } // Resolve a syntax object to a name, per MTWT. -// adding memoization to possibly resolve 500+ seconds in resolve for librustc (!) +// adding memoization to resolve 500+ seconds in resolve for librustc (!) fn resolve_internal(id: Ident, table: &SCTable, resolve_table: &mut ResolveTable) -> Name { diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs index 9549d5b8389..a93e8270d98 100644 --- a/src/libsyntax/parse/token.rs +++ b/src/libsyntax/parse/token.rs @@ -114,6 +114,7 @@ pub enum Nonterminal { NtPat( Gc), NtExpr(Gc), NtTy( P), + // see IDENT, above, for meaning of bool in NtIdent: NtIdent(Box, bool), NtMeta(Gc), // stuff inside brackets for attributes NtPath(Box), -- cgit 1.4.1-3-g733a5