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/ast.rs | 1 + src/libsyntax/ext/expand.rs | 62 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 51 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index d24c2be5a74..529b460adcd 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -401,6 +401,7 @@ pub enum Decl_ { DeclItem(Gc), } +/// represents one arm of a 'match' #[deriving(Clone, PartialEq, Eq, Encodable, Decodable, Hash)] pub struct Arm { pub attrs: Vec, 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 4b833e24c39112d2e59cfc1287483c2fe61c34e4 Mon Sep 17 00:00:00 2001 From: John Clements Date: Wed, 25 Jun 2014 18:15:07 -0700 Subject: make fold_attribute part of Folder trait --- src/libsyntax/fold.rs | 48 +++++++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index 6d2b0ceed8b..01742ca131e 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -86,7 +86,7 @@ pub trait Folder { kind: sf.node.kind, id: id, ty: self.fold_ty(sf.node.ty), - attrs: sf.node.attrs.iter().map(|e| fold_attribute_(*e, self)).collect() + attrs: sf.node.attrs.iter().map(|e| self.fold_attribute(*e)).collect() }, span: self.new_span(sf.span) } @@ -118,7 +118,7 @@ pub trait Folder { fn fold_arm(&mut self, a: &Arm) -> Arm { Arm { - attrs: a.attrs.iter().map(|x| fold_attribute_(*x, self)).collect(), + 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), @@ -251,7 +251,7 @@ pub trait Folder { } } - let attrs = v.node.attrs.iter().map(|x| fold_attribute_(*x, self)).collect(); + let attrs = v.node.attrs.iter().map(|x| self.fold_attribute(*x)).collect(); let de = match v.node.disr_expr { Some(e) => Some(self.fold_expr(e)), @@ -344,6 +344,21 @@ pub trait Folder { fn fold_lifetime(&mut self, l: &Lifetime) -> Lifetime { noop_fold_lifetime(l, self) } + + //used in noop_fold_item and noop_fold_crate + fn fold_attribute(&mut self, at: Attribute) -> Attribute { + Spanned { + span: self.new_span(at.span), + node: ast::Attribute_ { + id: at.node.id, + style: at.node.style, + value: fold_meta_item_(at.node.value, self), + is_sugared_doc: at.node.is_sugared_doc + } + } + } + + } /* some little folds that probably aren't useful to have in Folder itself*/ @@ -364,19 +379,6 @@ fn fold_meta_item_(mi: Gc, fld: &mut T) -> Gc { span: fld.new_span(mi.span) } } -//used in noop_fold_item and noop_fold_crate -fn fold_attribute_(at: Attribute, fld: &mut T) -> Attribute { - Spanned { - span: fld.new_span(at.span), - node: ast::Attribute_ { - id: at.node.id, - style: at.node.style, - value: fold_meta_item_(at.node.value, fld), - is_sugared_doc: at.node.is_sugared_doc - } - } -} - //used in noop_fold_foreign_item and noop_fold_fn_decl fn fold_arg_(a: &Arg, fld: &mut T) -> Arg { let id = fld.new_id(a.id); // Needs to be first, for ast_map. @@ -526,7 +528,7 @@ fn fold_struct_field(f: &StructField, fld: &mut T) -> StructField { kind: f.node.kind, id: id, ty: fld.fold_ty(f.node.ty), - attrs: f.node.attrs.iter().map(|a| fold_attribute_(*a, fld)).collect(), + attrs: f.node.attrs.iter().map(|a| fld.fold_attribute(*a)).collect(), }, span: fld.new_span(f.span), } @@ -578,7 +580,7 @@ pub fn noop_fold_view_item(vi: &ViewItem, folder: &mut T) }; ViewItem { node: inner_view_item, - attrs: vi.attrs.iter().map(|a| fold_attribute_(*a, folder)).collect(), + attrs: vi.attrs.iter().map(|a| folder.fold_attribute(*a)).collect(), vis: vi.vis, span: folder.new_span(vi.span), } @@ -658,7 +660,7 @@ pub fn noop_fold_type_method(m: &TypeMethod, fld: &mut T) -> TypeMeth TypeMethod { id: id, ident: fld.fold_ident(m.ident), - attrs: m.attrs.iter().map(|a| fold_attribute_(*a, fld)).collect(), + attrs: m.attrs.iter().map(|a| fld.fold_attribute(*a)).collect(), fn_style: m.fn_style, decl: fld.fold_fn_decl(&*m.decl), generics: fold_generics(&m.generics, fld), @@ -681,7 +683,7 @@ pub fn noop_fold_mod(m: &Mod, folder: &mut T) -> Mod { pub fn noop_fold_crate(c: Crate, folder: &mut T) -> Crate { Crate { module: folder.fold_mod(&c.module), - attrs: c.attrs.iter().map(|x| fold_attribute_(*x, folder)).collect(), + attrs: c.attrs.iter().map(|x| folder.fold_attribute(*x)).collect(), config: c.config.iter().map(|x| fold_meta_item_(*x, folder)).collect(), span: folder.new_span(c.span), } @@ -702,7 +704,7 @@ pub fn noop_fold_item(i: &Item, SmallVector::one(box(GC) Item { id: id, ident: folder.fold_ident(ident), - attrs: i.attrs.iter().map(|e| fold_attribute_(*e, folder)).collect(), + attrs: i.attrs.iter().map(|e| folder.fold_attribute(*e)).collect(), node: node, vis: i.vis, span: folder.new_span(i.span) @@ -715,7 +717,7 @@ pub fn noop_fold_foreign_item(ni: &ForeignItem, box(GC) ForeignItem { id: id, ident: folder.fold_ident(ni.ident), - attrs: ni.attrs.iter().map(|x| fold_attribute_(*x, folder)).collect(), + attrs: ni.attrs.iter().map(|x| folder.fold_attribute(*x)).collect(), node: match ni.node { ForeignItemFn(ref fdec, ref generics) => { ForeignItemFn(P(FnDecl { @@ -739,7 +741,7 @@ pub fn noop_fold_method(m: &Method, folder: &mut T) -> Gc { box(GC) Method { id: id, ident: folder.fold_ident(m.ident), - attrs: m.attrs.iter().map(|a| fold_attribute_(*a, folder)).collect(), + attrs: m.attrs.iter().map(|a| folder.fold_attribute(*a)).collect(), generics: fold_generics(&m.generics, folder), explicit_self: folder.fold_explicit_self(&m.explicit_self), fn_style: m.fn_style, -- cgit 1.4.1-3-g733a5 From c956f76c3c8bf42aa768c80e16ed16a3d7d370ea Mon Sep 17 00:00:00 2001 From: John Clements Date: Wed, 25 Jun 2014 19:41:16 -0700 Subject: replaced ignore-pretty with no-pretty-expanded Per @acrichto's suggestion, use the more narrowly focused exclusion. --- src/test/bench/msgsend-ring-mutex-arcs.rs | 2 +- src/test/bench/msgsend-ring-rw-arcs.rs | 2 +- src/test/bench/shootout-meteor.rs | 2 +- src/test/bench/shootout-spectralnorm.rs | 2 +- src/test/run-pass/backtrace.rs | 2 +- src/test/run-pass/deriving-cmp-generic-enum.rs | 2 +- src/test/run-pass/deriving-cmp-generic-struct-enum.rs | 2 +- src/test/run-pass/deriving-cmp-generic-struct.rs | 2 +- src/test/run-pass/deriving-cmp-generic-tuple-struct.rs | 2 +- src/test/run-pass/linear-for-loop.rs | 2 +- src/test/run-pass/task-comm-3.rs | 2 +- src/test/run-pass/unfold-cross-crate.rs | 2 +- src/test/run-pass/utf8.rs | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/test/bench/msgsend-ring-mutex-arcs.rs b/src/test/bench/msgsend-ring-mutex-arcs.rs index 716646da37e..2b9abfbc350 100644 --- a/src/test/bench/msgsend-ring-mutex-arcs.rs +++ b/src/test/bench/msgsend-ring-mutex-arcs.rs @@ -15,7 +15,7 @@ // This also serves as a pipes test, because Arcs are implemented with pipes. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 extern crate time; diff --git a/src/test/bench/msgsend-ring-rw-arcs.rs b/src/test/bench/msgsend-ring-rw-arcs.rs index 2580e6cad21..afed753f455 100644 --- a/src/test/bench/msgsend-ring-rw-arcs.rs +++ b/src/test/bench/msgsend-ring-rw-arcs.rs @@ -15,7 +15,7 @@ // This also serves as a pipes test, because Arcs are implemented with pipes. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 extern crate time; diff --git a/src/test/bench/shootout-meteor.rs b/src/test/bench/shootout-meteor.rs index a0ff8e8c1f9..e13c53407e4 100644 --- a/src/test/bench/shootout-meteor.rs +++ b/src/test/bench/shootout-meteor.rs @@ -38,7 +38,7 @@ // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED // OF THE POSSIBILITY OF SUCH DAMAGE. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 #![feature(phase)] #[phase(plugin)] extern crate green; diff --git a/src/test/bench/shootout-spectralnorm.rs b/src/test/bench/shootout-spectralnorm.rs index 949cf439df1..8cec135944f 100644 --- a/src/test/bench/shootout-spectralnorm.rs +++ b/src/test/bench/shootout-spectralnorm.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 #![feature(phase)] #![allow(non_snake_case_functions)] diff --git a/src/test/run-pass/backtrace.rs b/src/test/run-pass/backtrace.rs index 3b74ec4add3..859fc62647a 100644 --- a/src/test/run-pass/backtrace.rs +++ b/src/test/run-pass/backtrace.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 // ignore-win32 FIXME #13259 extern crate native; diff --git a/src/test/run-pass/deriving-cmp-generic-enum.rs b/src/test/run-pass/deriving-cmp-generic-enum.rs index 4a9324dd201..ccd0d967a51 100644 --- a/src/test/run-pass/deriving-cmp-generic-enum.rs +++ b/src/test/run-pass/deriving-cmp-generic-enum.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 #[deriving(PartialEq, Eq, PartialOrd, Ord)] enum E { diff --git a/src/test/run-pass/deriving-cmp-generic-struct-enum.rs b/src/test/run-pass/deriving-cmp-generic-struct-enum.rs index b21c95d7b50..2abdf4c7c7e 100644 --- a/src/test/run-pass/deriving-cmp-generic-struct-enum.rs +++ b/src/test/run-pass/deriving-cmp-generic-struct-enum.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 #![feature(struct_variant)] diff --git a/src/test/run-pass/deriving-cmp-generic-struct.rs b/src/test/run-pass/deriving-cmp-generic-struct.rs index e2b8e1b6b82..d1610978e2e 100644 --- a/src/test/run-pass/deriving-cmp-generic-struct.rs +++ b/src/test/run-pass/deriving-cmp-generic-struct.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 #[deriving(PartialEq, Eq, PartialOrd, Ord)] struct S { diff --git a/src/test/run-pass/deriving-cmp-generic-tuple-struct.rs b/src/test/run-pass/deriving-cmp-generic-tuple-struct.rs index c07f124a08d..25f62e85ba6 100644 --- a/src/test/run-pass/deriving-cmp-generic-tuple-struct.rs +++ b/src/test/run-pass/deriving-cmp-generic-tuple-struct.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 #[deriving(PartialEq, Eq, PartialOrd, Ord)] struct TS(T,T); diff --git a/src/test/run-pass/linear-for-loop.rs b/src/test/run-pass/linear-for-loop.rs index 640ed3883eb..e9e2a753469 100644 --- a/src/test/run-pass/linear-for-loop.rs +++ b/src/test/run-pass/linear-for-loop.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 extern crate debug; diff --git a/src/test/run-pass/task-comm-3.rs b/src/test/run-pass/task-comm-3.rs index 1c14153a110..afeb9125fe6 100644 --- a/src/test/run-pass/task-comm-3.rs +++ b/src/test/run-pass/task-comm-3.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 extern crate debug; diff --git a/src/test/run-pass/unfold-cross-crate.rs b/src/test/run-pass/unfold-cross-crate.rs index d3e70706867..2af38047264 100644 --- a/src/test/run-pass/unfold-cross-crate.rs +++ b/src/test/run-pass/unfold-cross-crate.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 use std::iter::Unfold; diff --git a/src/test/run-pass/utf8.rs b/src/test/run-pass/utf8.rs index 6cf0d518628..a79fcfa9417 100644 --- a/src/test/run-pass/utf8.rs +++ b/src/test/run-pass/utf8.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-pretty FIXME #15189 +// no-pretty-expanded FIXME #15189 pub fn main() { let yen: char = '¥'; // 0xa5 -- 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') 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') 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') 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') 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') 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') 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') 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 ee1ee7f463240ebda33d2180dd54be7a5e7f923c Mon Sep 17 00:00:00 2001 From: John Clements Date: Thu, 26 Jun 2014 17:41:43 -0700 Subject: make tests hygienic... ... and possibly totally pointless. Specifically, fixing these to make their macros hygienic may mean that they no longer test the thing that they were supposed to test. --- src/test/run-pass/issue-8851.rs | 13 +++++++++---- src/test/run-pass/typeck-macro-interaction-issue-8852.rs | 13 +++++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/test/run-pass/issue-8851.rs b/src/test/run-pass/issue-8851.rs index 62970369579..21037120569 100644 --- a/src/test/run-pass/issue-8851.rs +++ b/src/test/run-pass/issue-8851.rs @@ -10,23 +10,28 @@ #![feature(macro_rules)] +// after fixing #9384 and implementing hygiene for match bindings, +// this now fails because the insertion of the 'y' into the match +// doesn't cause capture. Making this macro hygienic (as I've done) +// could very well make this test case completely pointless.... + enum T { A(int), B(uint) } macro_rules! test( - ($e:expr) => ( + ($id:ident, $e:expr) => ( fn foo(t: T) -> int { match t { - A(y) => $e, - B(y) => $e + A($id) => $e, + B($id) => $e } } ) ) -test!(10 + (y as int)) +test!(y, 10 + (y as int)) pub fn main() { foo(A(20)); diff --git a/src/test/run-pass/typeck-macro-interaction-issue-8852.rs b/src/test/run-pass/typeck-macro-interaction-issue-8852.rs index 50ef1922c8f..6be79cb62dd 100644 --- a/src/test/run-pass/typeck-macro-interaction-issue-8852.rs +++ b/src/test/run-pass/typeck-macro-interaction-issue-8852.rs @@ -15,19 +15,24 @@ enum T { B(f64) } +// after fixing #9384 and implementing hygiene for match bindings, +// this now fails because the insertion of the 'y' into the match +// doesn't cause capture. Making this macro hygienic (as I've done) +// could very well make this test case completely pointless.... + macro_rules! test( - ($e:expr) => ( + ($id1:ident, $id2:ident, $e:expr) => ( fn foo(a:T, b:T) -> T { match (a, b) { - (A(x), A(y)) => A($e), - (B(x), B(y)) => B($e), + (A($id1), A($id2)) => A($e), + (B($id1), B($id2)) => B($e), _ => fail!() } } ) ) -test!(x + y) +test!(x,y,x + y) pub fn main() { foo(A(1), A(2)); -- 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') 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') 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 268f6c56c219e97b444d78dadc387cc549e27aa6 Mon Sep 17 00:00:00 2001 From: John Clements Date: Fri, 27 Jun 2014 14:12:13 -0700 Subject: removed incomplete comment as written, I don't believe this comment was helpful; I think it's better just to steer the reader toward a general understanding of hygiene. --- src/libsyntax/fold.rs | 17 ----------------- 1 file changed, 17 deletions(-) (limited to 'src') diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index 01742ca131e..a6e4b4cae1c 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -391,23 +391,6 @@ 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) -> Vec { tts.iter().map(|tt| { match *tt { -- cgit 1.4.1-3-g733a5 From 764c2fe2d582c36ad00ef67b5553f7777447e415 Mon Sep 17 00:00:00 2001 From: John Clements Date: Fri, 27 Jun 2014 14:41:32 -0700 Subject: simplified test case --- src/test/run-pass/issue-15221.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/test/run-pass/issue-15221.rs b/src/test/run-pass/issue-15221.rs index e74ba9b85ee..e5cfccac13a 100644 --- a/src/test/run-pass/issue-15221.rs +++ b/src/test/run-pass/issue-15221.rs @@ -10,13 +10,14 @@ #![feature(macro_rules)] -macro_rules! inner_bind ( - ( $p:pat, $id:ident) => ({let $p = 13; $id})) +macro_rules! inner ( + ($e:pat ) => ($e)) -macro_rules! outer_bind ( - ($p:pat, $id:ident ) => (inner_bind!($p, $id))) +macro_rules! outer ( + ($e:pat ) => (inner!($e))) fn main() { - outer_bind!(g1,g1); + let outer!(g1) = 13; + g1; } -- cgit 1.4.1-3-g733a5 From 2f73b7874ed2ba3be02cbf8c767aaecd6840bbe9 Mon Sep 17 00:00:00 2001 From: John Clements Date: Fri, 27 Jun 2014 16:07:25 -0700 Subject: looks like a cut-n-paste error in unused code --- src/doc/guide-macros.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/doc/guide-macros.md b/src/doc/guide-macros.md index 45745c7b7bc..6d3825f8ecf 100644 --- a/src/doc/guide-macros.md +++ b/src/doc/guide-macros.md @@ -355,6 +355,7 @@ macro_rules! biased_match_rec ( _ => { $err } } ); + // Produce the requested values ( binds $( $bind_res:ident ),* ) => ( ($( $bind_res ),*) ) ) @@ -364,7 +365,7 @@ macro_rules! biased_match ( ( $( ($e:expr) ~ ($p:pat) else $err:stmt ; )* binds $bind_res:ident ) => ( - let ( $( $bind_res ),* ) = biased_match_rec!( + let $bind_res = biased_match_rec!( $( ($e) ~ ($p) else $err ; )* binds $bind_res ); -- 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') 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') 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