implement hygiene for arguments of item functions

jbclements · jbclements · commit 2f129b21cd36 · 2014-07-03T11:51:46.000-07:00
Closes rust-lang#9384
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
@@ -344,15 +344,16 @@ fn expand_item(it: Gc<ast::Item>, fld: &mut MacroExpander)
 
 fn expand_item_modifiers(mut it: Gc<ast::Item>, fld: &mut MacroExpander)
                          -> Gc<ast::Item> {
-    let (modifiers, attrs) = it.attrs.partitioned(|attr| {
+    // partition the attributes into ItemModifiers and others
+    let (modifiers, other_attrs) = it.attrs.partitioned(|attr| {
         match fld.extsbox.find(&intern(attr.name().get())) {
             Some(&ItemModifier(_)) => true,
             _ => false
         }
     });
-
+    // update the attrs, leave everything else alone. Is this mutation really a good idea?
     it = box(GC) ast::Item {
-        attrs: attrs,
+        attrs: other_attrs,
         ..(*it).clone()
     };
 
@@ -385,6 +386,34 @@ fn expand_item_modifiers(mut it: Gc<ast::Item>, fld: &mut MacroExpander)
     expand_item_modifiers(it, fld)
 }
 
+/// Expand item_underscore
+fn expand_item_underscore(item: &ast::Item_, fld: &mut MacroExpander) -> ast::Item_ {
+    match *item {
+        ast::ItemFn(decl, fn_style, abi, ref generics, body) => {
+            let expanded_decl = fld.fold_fn_decl(decl);
+            // avoiding the pattern_bindings abstraction here
+            // for efficiency's sake, to re-use one mutable vector
+            // in the collection.
+            let mut pat_idents = PatIdentFinder{ident_accumulator:Vec::new()};
+            //pat_idents.visit_fn_decl(expanded_decl,());
+            for arg in expanded_decl.inputs.iter() {
+                pat_idents.visit_pat(arg.pat,());
+            }
+            let idents = pat_idents.ident_accumulator;
+            let new_pending_renames =
+                idents.iter().map(|id : &ast::Ident| (*id,fresh_name(id))).collect();
+            let mut rename_pat_fld = PatIdentRenamer{renames:&new_pending_renames};
+            let rewritten_fn_decl = rename_pat_fld.fold_fn_decl(expanded_decl);
+            // now, a renamer for *all* idents:
+            let mut rename_fld = IdentRenamer{renames:&new_pending_renames};
+            let rewritten_body = fld.fold_block(rename_fld.fold_block(body));
+            let expanded_generics = fold::fold_generics(generics,fld);
+            ast::ItemFn(rewritten_fn_decl, fn_style, abi, expanded_generics, rewritten_body)
+        }
+        _ => noop_fold_item_underscore(&*item, fld)
+    }
+}
+
 // does this attribute list contain "macro_escape" ?
 fn contains_macro_escape(attrs: &[ast::Attribute]) -> bool {
     attr::contains_name(attrs, "macro_escape")
@@ -667,19 +696,14 @@ 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 idents = pattern_bindings(*first_pat);
-    let mut new_pending_renames =
+    let 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};
+    // apply the renaming, but only to the PatIdents:
+    let mut rename_pats_fld = PatIdentRenamer{renames:&new_pending_renames};
     let rewritten_pats =
-        expanded_pats.iter().map(|pat| rename_fld.fold_pat(*pat)).collect();
+        expanded_pats.iter().map(|pat| rename_pats_fld.fold_pat(*pat)).collect();
     // apply renaming and then expansion to the guard and the body:
+    let mut rename_fld = IdentRenamer{renames:&new_pending_renames};
     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));
@@ -905,6 +929,10 @@ impl<'a, 'b> Folder for MacroExpander<'a, 'b> {
         expand_item(item, self)
     }
 
+    fn fold_item_underscore(&mut self, item: &ast::Item_) -> ast::Item_ {
+        expand_item_underscore(item, self)
+    }
+
     fn fold_stmt(&mut self, stmt: &ast::Stmt) -> SmallVector<Gc<ast::Stmt>> {
         expand_stmt(stmt, self)
     }
@@ -1052,13 +1080,14 @@ fn original_span(cx: &ExtCtxt) -> Gc<codemap::ExpnInfo> {
 #[cfg(test)]
 mod test {
     use super::{pattern_bindings, expand_crate, contains_macro_escape};
-    use super::{PatIdentFinder};
+    use super::{PatIdentFinder, IdentRenamer, PatIdentRenamer};
     use ast;
     use ast::{Attribute_, AttrOuter, MetaWord};
     use attr;
     use codemap;
     use codemap::Spanned;
     use ext::mtwt;
+    use fold::Folder;
     use parse;
     use parse::token;
     use util::parser_testing::{string_to_parser};
@@ -1096,7 +1125,24 @@ mod test {
         path_finder.path_accumulator
     }
 
+    /// A Visitor that extracts the identifiers from a thingy.
+    // as a side note, I'm starting to want to abstract over these....
+    struct IdentFinder{
+        ident_accumulator: Vec<ast::Ident>
+    }
 
+    impl Visitor<()> for IdentFinder {
+        fn visit_ident(&mut self, _: codemap::Span, id: ast::Ident, _: ()){
+            self.ident_accumulator.push(id);
+        }
+    }
+
+    /// Find the idents in a crate
+    fn crate_idents(the_crate: &ast::Crate) -> Vec<ast::Ident> {
+        let mut ident_finder = IdentFinder{ident_accumulator: Vec::new()};
+        visit::walk_crate(&mut ident_finder, the_crate, ());
+        ident_finder.ident_accumulator
+    }
 
     // these following tests are quite fragile, in that they don't test what
     // *kind* of failure occurs.
@@ -1196,6 +1242,7 @@ mod test {
         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);
@@ -1321,7 +1368,7 @@ mod test {
     // but *shouldnt* bind because it was inserted by a different macro....
     // can't write this test case until we have macro-generating macros.
 
-    // FIXME #9383 : lambda var hygiene
+    // lambda var hygiene
     // expands to fn q(x_1:int){fn g(x_2:int){x_2 + x_1};}
     #[test]
     fn issue_9383(){
@@ -1380,9 +1427,9 @@ mod test {
                         assert_eq!(varref_marks,binding_marks.clone());
                     }
                 } else {
+                    let varref_name = mtwt::resolve(varref.segments.get(0).identifier);
                     let fail = (varref.segments.len() == 1)
-                        && (mtwt::resolve(varref.segments.get(0).identifier)
-                            == binding_name);
+                        && (varref_name == binding_name);
                     // temp debugging:
                     if fail {
                         let varref_idents : Vec<ast::Ident>
@@ -1393,15 +1440,18 @@ mod test {
                         println!("text of test case: \"{}\"", teststr);
                         println!("");
                         println!("uh oh, matches but shouldn't:");
-                        println!("varref: {}",varref_idents);
+                        println!("varref #{}: {}, resolves to {}",idx, varref_idents,
+                                 varref_name);
                         // good lord, you can't make a path with 0 segments, can you?
                         let string = token::get_ident(varref.segments
                                                             .get(0)
                                                             .identifier);
                         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 #{}: {}, resolves to {}",
+                                 binding_idx, *bindings.get(binding_idx),
+                                 binding_name);
                         mtwt::with_sctable(|x| mtwt::display_sctable(x));
                     }
                     assert!(!fail);
@@ -1464,13 +1514,43 @@ foo_module!()
     // 'None' is listed as an identifier pattern because we don't yet know that
     // it's the name of a 0-ary variant, and that 'i' appears twice in succession.
     #[test]
-    fn crate_idents(){
+    fn crate_bindings_test(){
         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 idents = crate_bindings(&the_crate);
         assert_eq!(idents, strs_to_idents(vec!("a","b","None","i","i","z","y")));
     }
 
-    //
+    // test the IdentRenamer directly
+    #[test]
+    fn ident_renamer_test () {
+        let the_crate = string_to_crate("fn f(x : int){let x = x; x}".to_string());
+        let f_ident = token::str_to_ident("f");
+        let x_ident = token::str_to_ident("x");
+        let int_ident = token::str_to_ident("int");
+        let renames = vec!((x_ident,16));
+        let mut renamer = IdentRenamer{renames: &renames};
+        let renamed_crate = renamer.fold_crate(the_crate);
+        let idents = crate_idents(&renamed_crate);
+        let resolved : Vec<ast::Name> = idents.iter().map(|id| mtwt::resolve(*id)).collect();
+        assert_eq!(resolved,vec!(f_ident.name,16,int_ident.name,16,16,16));
+    }
+
+    // test the PatIdentRenamer; only PatIdents get renamed
+    #[test]
+    fn pat_ident_renamer_test () {
+        let the_crate = string_to_crate("fn f(x : int){let x = x; x}".to_string());
+        let f_ident = token::str_to_ident("f");
+        let x_ident = token::str_to_ident("x");
+        let int_ident = token::str_to_ident("int");
+        let renames = vec!((x_ident,16));
+        let mut renamer = PatIdentRenamer{renames: &renames};
+        let renamed_crate = renamer.fold_crate(the_crate);
+        let idents = crate_idents(&renamed_crate);
+        let resolved : Vec<ast::Name> = idents.iter().map(|id| mtwt::resolve(*id)).collect();
+        let x_name = x_ident.name;
+        assert_eq!(resolved,vec!(f_ident.name,16,int_ident.name,16,x_name,x_name));
+    }
+
 
 }
