Auto merge of #34374 - jseyfried:fix_hygiene_bug, r=nrc

Fix macro hygiene regression

The regression was caused by #32923, which is currently in beta.

The following is an example of regressed code:
```rust
fn main() {
    let x = 0;
    macro_rules! foo { () => {
        println!("{}", x); // prints `0` on stable and after this PR, prints `1` on beta and nightly
    } }

    let x = 1;
    foo!();
}
```

For code to regress, the following is necessary (but not sufficient):
 - There must be a local variable before a macro in a block, and the macro must use the variable.
 - There must be a second local variable with the same name after the macro.
 - The macro must be invoked in a statement position after the second local variable.

For example, if the `let x = 0;` from the breaking example were commented out, it would (correctly) not compile on beta/nightly. If the semicolon were removed from `foo!();`, it would (correctly) print `0` on beta and nightly.

r? @nrc

3 files changed, 163 insertions, 596 deletions

diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index d63411568dc..73acbb2aa0a 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -681,7 +681,7 @@ pub struct IdentRenamer<'a> {
 
 impl<'a> Folder for IdentRenamer<'a> {
     fn fold_ident(&mut self, id: Ident) -> Ident {
-        Ident::new(id.name, mtwt::apply_renames(self.renames, id.ctxt))
+        mtwt::apply_renames(self.renames, id)
     }
     fn fold_mac(&mut self, mac: ast::Mac) -> ast::Mac {
         fold::noop_fold_mac(mac, self)
@@ -705,8 +705,7 @@ impl<'a> Folder for PatIdentRenamer<'a> {
 
         pat.map(|ast::Pat {id, node, span}| match node {
             PatKind::Ident(binding_mode, Spanned{span: sp, node: ident}, sub) => {
-                let new_ident = Ident::new(ident.name,
-                                           mtwt::apply_renames(self.renames, ident.ctxt));
+                let new_ident = mtwt::apply_renames(self.renames, ident);
                 let new_node =
                     PatKind::Ident(binding_mode,
                                   Spanned{span: sp, node: new_ident},
@@ -1266,7 +1265,7 @@ mod tests {
     use ext::mtwt;
     use fold::Folder;
     use parse;
-    use parse::token::{self, keywords};
+    use parse::token;
     use util::parser_testing::{string_to_parser};
     use util::parser_testing::{string_to_pat, string_to_crate, strs_to_idents};
     use visit;
@@ -1396,267 +1395,10 @@ mod tests {
             );
     }
 
-    // renaming tests expand a crate and then check that the bindings match
-    // the right varrefs. The specification of the test case includes the
-    // text of the crate, and also an array of arrays.  Each element in the
-    // outer array corresponds to a binding in the traversal of the AST
-    // induced by visit.  Each of these arrays contains a list of indexes,
-    // interpreted as the varrefs in the varref traversal that this binding
-    // should match.  So, for instance, in a program with two bindings and
-    // three varrefs, the array [[1, 2], [0]] would indicate that the first
-    // binding should match the second two varrefs, and the second binding
-    // should match the first varref.
-    //
-    // Put differently; this is a sparse representation of a boolean matrix
-    // indicating which bindings capture which identifiers.
-    //
-    // Note also that this matrix is dependent on the implicit ordering of
-    // the bindings and the varrefs discovered by the name-finder and the path-finder.
-    //
-    // The comparisons are done post-mtwt-resolve, so we're comparing renamed
-    // names; differences in marks don't matter any more.
-    //
-    // oog... I also want tests that check "bound-identifier-=?". That is,
-    // not just "do these have the same name", but "do they have the same
-    // name *and* the same marks"? Understanding this is really pretty painful.
-    // in principle, you might want to control this boolean on a per-varref basis,
-    // but that would make things even harder to understand, and might not be
-    // necessary for thorough testing.
-    type RenamingTest = (&'static str, Vec<Vec<usize>>, bool);
-
-    #[test]
-    fn automatic_renaming () {
-        let tests: Vec<RenamingTest> =
-            vec!(// b & c should get new names throughout, in the expr too:
-                ("fn a() -> i32 { let b = 13; let c = b; b+c }",
-                 vec!(vec!(0,1),vec!(2)), false),
-                // both x's should be renamed (how is this causing a bug?)
-                ("fn main () {let x: i32 = 13;x;}",
-                 vec!(vec!(0)), false),
-                // the use of b after the + should be renamed, the other one not:
-                ("macro_rules! f (($x:ident) => (b + $x)); fn a() -> i32 { let b = 13; f!(b)}",
-                 vec!(vec!(1)), false),
-                // the b before the plus should not be renamed (requires marks)
-                ("macro_rules! f (($x:ident) => ({let b=9; ($x + b)})); fn a() -> i32 { f!(b)}",
-                 vec!(vec!(1)), false),
-                // the marks going in and out of letty should cancel, allowing that $x to
-                // capture the one following the semicolon.
-                // this was an awesome test case, and caught a *lot* of bugs.
-                ("macro_rules! letty(($x:ident) => (let $x = 15;));
-                  macro_rules! user(($x:ident) => ({letty!($x); $x}));
-                  fn main() -> i32 {user!(z)}",
-                 vec!(vec!(0)), false)
-                );
-        for (idx,s) in tests.iter().enumerate() {
-            run_renaming_test(s,idx);
-        }
-    }
-
-    // no longer a fixme #8062: this test exposes a *potential* bug; our system does
-    // not behave exactly like MTWT, but a conversation with Matthew Flatt
-    // suggests that this can only occur in the presence of local-expand, which
-    // we have no plans to support. ... unless it's needed for item hygiene....
-    #[ignore]
-    #[test]
-    fn issue_8062(){
-        run_renaming_test(
-            &("fn main() {let hrcoo = 19; macro_rules! getx(()=>(hrcoo)); getx!();}",
-              vec!(vec!(0)), true), 0)
-    }
-
-    // FIXME #6994:
-    // the z flows into and out of two macros (g & f) along one path, and one
-    // (just g) along the other, so the result of the whole thing should
-    // be "let z_123 = 3; z_123"
-    #[ignore]
-    #[test]
-    fn issue_6994(){
-        run_renaming_test(
-            &("macro_rules! g (($x:ident) =>
-              ({macro_rules! f(($y:ident)=>({let $y=3;$x}));f!($x)}));
-              fn a(){g!(z)}",
-              vec!(vec!(0)),false),
-            0)
-    }
-
-    // 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(
-            &("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,3),vec!(0,2),vec!(0,2)),
-              true),
-            0)
-    }
-
-    // interpolated nodes weren't getting labeled.
-    // 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 *shouldn't* bind because it was inserted by a different macro....
     // can't write this test case until we have macro-generating macros.
 
-    // method arg hygiene
-    // method expands to fn get_x(&self_0, x_1: i32) {self_0 + self_2 + x_3 + x_1}
-    #[test]
-    fn method_arg_hygiene(){
-        run_renaming_test(
-            &("macro_rules! inject_x (()=>(x));
-              macro_rules! inject_self (()=>(self));
-              struct A;
-              impl A{fn get_x(&self, x: i32) {self + inject_self!() + inject_x!() + x;} }",
-              vec!(vec!(0),vec!(3)),
-              true),
-            0)
-    }
-
-    // ooh, got another bite?
-    // expands to struct A; impl A {fn thingy(&self_1) {self_1;}}
-    #[test]
-    fn method_arg_hygiene_2(){
-        run_renaming_test(
-            &("struct A;
-              macro_rules! add_method (($T:ty) =>
-              (impl $T {  fn thingy(&self) {self;} }));
-              add_method!(A);",
-              vec!(vec!(0)),
-              true),
-            0)
-    }
-
-    // item fn hygiene
-    // expands to fn q(x_1: i32){fn g(x_2: i32){x_2 + x_1};}
-    #[test]
-    fn issue_9383(){
-        run_renaming_test(
-            &("macro_rules! bad_macro (($ex:expr) => (fn g(x: i32){ x + $ex }));
-              fn q(x: i32) { bad_macro!(x); }",
-              vec!(vec!(1),vec!(0)),true),
-            0)
-    }
-
-    // closure arg hygiene (ExprKind::Closure)
-    // expands to fn f(){(|x_1 : i32| {(x_2 + x_1)})(3);}
-    #[test]
-    fn closure_arg_hygiene(){
-        run_renaming_test(
-            &("macro_rules! inject_x (()=>(x));
-            fn f(){(|x : i32| {(inject_x!() + x)})(3);}",
-              vec!(vec!(1)),
-              true),
-            0)
-    }
-
-    // macro_rules in method position. Sadly, unimplemented.
-    #[test]
-    fn macro_in_method_posn(){
-        expand_crate_str(
-            "macro_rules! my_method (() => (fn thirteen(&self) -> i32 {13}));
-            struct A;
-            impl A{ my_method!(); }
-            fn f(){A.thirteen;}".to_string());
-    }
-
-    // another nested macro
-    // expands to impl Entries {fn size_hint(&self_1) {self_1;}
-    #[test]
-    fn item_macro_workaround(){
-        run_renaming_test(
-            &("macro_rules! item { ($i:item) => {$i}}
-              struct Entries;
-              macro_rules! iterator_impl {
-              () => { item!( impl Entries { fn size_hint(&self) { self;}});}}
-              iterator_impl! { }",
-              vec!(vec!(0)), true),
-            0)
-    }
-
-    // run one of the renaming tests
-    fn run_renaming_test(t: &RenamingTest, test_idx: usize) {
-        let invalid_name = keywords::Invalid.name();
-        let (teststr, bound_connections, bound_ident_check) = match *t {
-            (ref str,ref conns, bic) => (str.to_string(), conns.clone(), bic)
-        };
-        let cr = expand_crate_str(teststr.to_string());
-        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());
-        for (binding_idx,shouldmatch) in bound_connections.iter().enumerate() {
-            let binding_name = mtwt::resolve(bindings[binding_idx]);
-            let binding_marks = mtwt::marksof(bindings[binding_idx].ctxt, invalid_name);
-            // shouldmatch can't name varrefs that don't exist:
-            assert!((shouldmatch.is_empty()) ||
-                    (varrefs.len() > *shouldmatch.iter().max().unwrap()));
-            for (idx,varref) in varrefs.iter().enumerate() {
-                let print_hygiene_debug_info = || {
-                    // good lord, you can't make a path with 0 segments, can you?
-                    let final_varref_ident = match varref.segments.last() {
-                        Some(pathsegment) => pathsegment.identifier,
-                        None => panic!("varref with 0 path segments?")
-                    };
-                    let varref_name = mtwt::resolve(final_varref_ident);
-                    let varref_idents : Vec<ast::Ident>
-                        = varref.segments.iter().map(|s| s.identifier)
-                        .collect();
-                    println!("varref #{}: {:?}, resolves to {}",idx, varref_idents, varref_name);
-                    println!("varref's first segment's string: \"{}\"", final_varref_ident);
-                    println!("binding #{}: {}, resolves to {}",
-                             binding_idx, bindings[binding_idx], binding_name);
-                    mtwt::with_sctable(|x| mtwt::display_sctable(x));
-                };
-                if shouldmatch.contains(&idx) {
-                    // it should be a path of length 1, and it should
-                    // be free-identifier=? or bound-identifier=? to the given binding
-                    assert_eq!(varref.segments.len(),1);
-                    let varref_name = mtwt::resolve(varref.segments[0].identifier);
-                    let varref_marks = mtwt::marksof(varref.segments[0]
-                                                           .identifier
-                                                           .ctxt,
-                                                     invalid_name);
-                    if !(varref_name==binding_name) {
-                        println!("uh oh, should match but doesn't:");
-                        print_hygiene_debug_info();
-                    }
-                    assert_eq!(varref_name,binding_name);
-                    if bound_ident_check {
-                        // we're checking bound-identifier=?, and the marks
-                        // should be the same, too:
-                        assert_eq!(varref_marks,binding_marks.clone());
-                    }
-                } else {
-                    let varref_name = mtwt::resolve(varref.segments[0].identifier);
-                    let fail = (varref.segments.len() == 1)
-                        && (varref_name == binding_name);
-                    // temp debugging:
-                    if fail {
-                        println!("failure on test {}",test_idx);
-                        println!("text of test case: \"{}\"", teststr);
-                        println!("");
-                        println!("uh oh, matches but shouldn't:");
-                        print_hygiene_debug_info();
-                    }
-                    assert!(!fail);
-                }
-            }
-        }
-    }
-
     #[test]
     fn fmt_in_macro_used_inside_module_macro() {
         let crate_str = "macro_rules! fmt_wrap(($b:expr)=>($b.to_string()));
diff --git a/src/libsyntax/ext/mtwt.rs b/src/libsyntax/ext/mtwt.rs
index 7ac0e8c64c2..c9e8715dda6 100644
--- a/src/libsyntax/ext/mtwt.rs
+++ b/src/libsyntax/ext/mtwt.rs
@@ -25,34 +25,22 @@ use std::collections::HashMap;
 /// The SCTable contains a table of SyntaxContext_'s. It
 /// represents a flattened tree structure, to avoid having
 /// managed pointers everywhere (that caused an ICE).
-/// the mark_memo and rename_memo fields are side-tables
+/// the `marks` and `renames` fields are side-tables
 /// that ensure that adding the same mark to the same context
-/// gives you back the same context as before. This shouldn't
-/// 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
+/// gives you back the same context as before. This 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<Vec<SyntaxContext_>>,
-    mark_memo: RefCell<HashMap<(SyntaxContext,Mrk),SyntaxContext>>,
-    // The pair (Name,SyntaxContext) is actually one Ident, but it needs to be hashed and
-    // compared as pair (name, ctxt) and not as an Ident
-    rename_memo: RefCell<HashMap<(SyntaxContext,(Name,SyntaxContext),Name),SyntaxContext>>,
+    marks: RefCell<HashMap<(SyntaxContext,Mrk),SyntaxContext>>,
+    renames: RefCell<HashMap<Name,SyntaxContext>>,
 }
 
 #[derive(PartialEq, RustcEncodable, RustcDecodable, Hash, Debug, Copy, Clone)]
 pub enum SyntaxContext_ {
     EmptyCtxt,
     Mark (Mrk,SyntaxContext),
-    /// flattening the name and syntaxcontext into the rename...
-    /// HIDDEN INVARIANTS:
-    /// 1) the first name in a Rename node
-    /// can only be a programmer-supplied name.
-    /// 2) Every Rename node with a given Name in the
-    /// "to" slot must have the same name and context
-    /// in the "from" slot. In essence, they're all
-    /// pointers to a single "rename" event node.
-    Rename (Ident,Name,SyntaxContext),
+    Rename (Name),
     /// actually, IllegalCtxt may not be necessary.
     IllegalCtxt
 }
@@ -67,37 +55,39 @@ pub fn apply_mark(m: Mrk, ctxt: SyntaxContext) -> SyntaxContext {
 
 /// Extend a syntax context with a given mark and sctable (explicit memoization)
 fn apply_mark_internal(m: Mrk, ctxt: SyntaxContext, table: &SCTable) -> SyntaxContext {
-    let key = (ctxt, m);
-    *table.mark_memo.borrow_mut().entry(key).or_insert_with(|| {
-        SyntaxContext(idx_push(&mut *table.table.borrow_mut(), Mark(m, ctxt)))
-    })
+    let ctxts = &mut *table.table.borrow_mut();
+    match ctxts[ctxt.0 as usize] {
+        // Applying the same mark twice is a no-op.
+        Mark(outer_mark, prev_ctxt) if outer_mark == m => return prev_ctxt,
+        _ => *table.marks.borrow_mut().entry((ctxt, m)).or_insert_with(|| {
+            SyntaxContext(idx_push(ctxts, Mark(m, ctxt)))
+        }),
+    }
 }
 
 /// Extend a syntax context with a given rename
-pub fn apply_rename(id: Ident, to:Name,
-                  ctxt: SyntaxContext) -> SyntaxContext {
-    with_sctable(|table| apply_rename_internal(id, to, ctxt, table))
+pub fn apply_rename(from: Ident, to: Name, ident: Ident) -> Ident {
+    with_sctable(|table| apply_rename_internal(from, to, ident, table))
 }
 
 /// Extend a syntax context with a given rename and sctable (explicit memoization)
-fn apply_rename_internal(id: Ident,
-                       to: Name,
-                       ctxt: SyntaxContext,
-                       table: &SCTable) -> SyntaxContext {
-    let key = (ctxt, (id.name, id.ctxt), to);
-
-    *table.rename_memo.borrow_mut().entry(key).or_insert_with(|| {
-            SyntaxContext(idx_push(&mut *table.table.borrow_mut(), Rename(id, to, ctxt)))
-    })
+fn apply_rename_internal(from: Ident, to: Name, ident: Ident, table: &SCTable) -> Ident {
+    if (ident.name, ident.ctxt) != (from.name, from.ctxt) {
+        return ident;
+    }
+    let ctxt = *table.renames.borrow_mut().entry(to).or_insert_with(|| {
+        SyntaxContext(idx_push(&mut *table.table.borrow_mut(), Rename(to)))
+    });
+    Ident { ctxt: ctxt, ..ident }
 }
 
 /// Apply a list of renamings to a context
 // if these rename lists get long, it would make sense
 // to consider memoizing this fold. This may come up
 // when we add hygiene to item names.
-pub fn apply_renames(renames: &RenameList, ctxt: SyntaxContext) -> SyntaxContext {
-    renames.iter().fold(ctxt, |ctxt, &(from, to)| {
-        apply_rename(from, to, ctxt)
+pub fn apply_renames(renames: &RenameList, ident: Ident) -> Ident {
+    renames.iter().fold(ident, |ident, &(from, to)| {
+        apply_rename(from, to, ident)
     })
 }
 
@@ -114,8 +104,8 @@ pub fn with_sctable<T, F>(op: F) -> T where
 fn new_sctable_internal() -> SCTable {
     SCTable {
         table: RefCell::new(vec!(EmptyCtxt, IllegalCtxt)),
-        mark_memo: RefCell::new(HashMap::new()),
-        rename_memo: RefCell::new(HashMap::new()),
+        marks: RefCell::new(HashMap::new()),
+        renames: RefCell::new(HashMap::new()),
     }
 }
 
@@ -131,20 +121,18 @@ pub fn display_sctable(table: &SCTable) {
 pub fn clear_tables() {
     with_sctable(|table| {
         *table.table.borrow_mut() = Vec::new();
-        *table.mark_memo.borrow_mut() = HashMap::new();
-        *table.rename_memo.borrow_mut() = HashMap::new();
+        *table.marks.borrow_mut() = HashMap::new();
+        *table.renames.borrow_mut() = HashMap::new();
     });
-    with_resolve_table_mut(|table| *table = HashMap::new());
 }
 
 /// Reset the tables to their initial state
 pub fn reset_tables() {
     with_sctable(|table| {
         *table.table.borrow_mut() = vec!(EmptyCtxt, IllegalCtxt);
-        *table.mark_memo.borrow_mut() = HashMap::new();
-        *table.rename_memo.borrow_mut() = HashMap::new();
+        *table.marks.borrow_mut() = HashMap::new();
+        *table.renames.borrow_mut() = HashMap::new();
     });
-    with_resolve_table_mut(|table| *table = HashMap::new());
 }
 
 /// Add a value to the end of a vec, return its index
@@ -156,103 +144,19 @@ fn idx_push<T>(vec: &mut Vec<T>, val: T) -> u32 {
 /// Resolve a syntax object to a name, per MTWT.
 pub fn resolve(id: Ident) -> Name {
     with_sctable(|sctable| {
-        with_resolve_table_mut(|resolve_table| {
-            resolve_internal(id, sctable, resolve_table)
-        })
+        resolve_internal(id, sctable)
     })
 }
 
-type ResolveTable = HashMap<(Name,SyntaxContext),Name>;
-
-// okay, I admit, putting this in TLS is not so nice:
-// fetch the SCTable from TLS, create one if it doesn't yet exist.
-fn with_resolve_table_mut<T, F>(op: F) -> T where
-    F: FnOnce(&mut ResolveTable) -> T,
-{
-    thread_local!(static RESOLVE_TABLE_KEY: RefCell<ResolveTable> = {
-        RefCell::new(HashMap::new())
-    });
-
-    RESOLVE_TABLE_KEY.with(move |slot| op(&mut *slot.borrow_mut()))
-}
-
 /// Resolve a syntax object to a name, per MTWT.
 /// adding memoization to resolve 500+ seconds in resolve for librustc (!)
-fn resolve_internal(id: Ident,
-                    table: &SCTable,
-                    resolve_table: &mut ResolveTable) -> Name {
-    let key = (id.name, id.ctxt);
-
-    match resolve_table.get(&key) {
-        Some(&name) => return name,
-        None => {}
-    }
-
-    let resolved = {
-        let result = (*table.table.borrow())[id.ctxt.0 as usize];
-        match result {
-            EmptyCtxt => id.name,
-            // ignore marks here:
-            Mark(_,subctxt) =>
-                resolve_internal(Ident::new(id.name, subctxt),
-                                 table, resolve_table),
-            // do the rename if necessary:
-            Rename(Ident{name, ctxt}, toname, subctxt) => {
-                let resolvedfrom =
-                    resolve_internal(Ident::new(name, ctxt),
-                                     table, resolve_table);
-                let resolvedthis =
-                    resolve_internal(Ident::new(id.name, subctxt),
-                                     table, resolve_table);
-                if (resolvedthis == resolvedfrom)
-                    && (marksof_internal(ctxt, resolvedthis, table)
-                        == marksof_internal(subctxt, resolvedthis, table)) {
-                    toname
-                } else {
-                    resolvedthis
-                }
-            }
-            IllegalCtxt => panic!("expected resolvable context, got IllegalCtxt")
-        }
-    };
-    resolve_table.insert(key, resolved);
-    resolved
-}
-
-/// Compute the marks associated with a syntax context.
-pub fn marksof(ctxt: SyntaxContext, stopname: Name) -> Vec<Mrk> {
-    with_sctable(|table| marksof_internal(ctxt, stopname, table))
-}
-
-// the internal function for computing marks
-// it's not clear to me whether it's better to use a [] mutable
-// vector or a cons-list for this.
-fn marksof_internal(ctxt: SyntaxContext,
-                    stopname: Name,
-                    table: &SCTable) -> Vec<Mrk> {
-    let mut result = Vec::new();
-    let mut loopvar = ctxt;
-    loop {
-        let table_entry = (*table.table.borrow())[loopvar.0 as usize];
-        match table_entry {
-            EmptyCtxt => {
-                return result;
-            },
-            Mark(mark, tl) => {
-                xor_push(&mut result, mark);
-                loopvar = tl;
-            },
-            Rename(_,name,tl) => {
-                // see MTWT for details on the purpose of the stopname.
-                // short version: it prevents duplication of effort.
-                if name == stopname {
-                    return result;
-                } else {
-                    loopvar = tl;
-                }
-            }
-            IllegalCtxt => panic!("expected resolvable context, got IllegalCtxt")
-        }
+fn resolve_internal(id: Ident, table: &SCTable) -> Name {
+    match table.table.borrow()[id.ctxt.0 as usize] {
+        EmptyCtxt => id.name,
+        // ignore marks here:
+        Mark(_, subctxt) => resolve_internal(Ident::new(id.name, subctxt), table),
+        Rename(name) => name,
+        IllegalCtxt => panic!("expected resolvable context, got IllegalCtxt")
     }
 }
 
@@ -267,103 +171,16 @@ pub fn outer_mark(ctxt: SyntaxContext) -> Mrk {
     })
 }
 
-/// Push a name... unless it matches the one on top, in which
-/// case pop and discard (so two of the same marks cancel)
-fn xor_push(marks: &mut Vec<Mrk>, mark: Mrk) {
-    if (!marks.is_empty()) && (*marks.last().unwrap() == mark) {
-        marks.pop().unwrap();
-    } else {
-        marks.push(mark);
-    }
-}
-
 #[cfg(test)]
 mod tests {
-    use self::TestSC::*;
     use ast::{EMPTY_CTXT, Ident, Mrk, Name, SyntaxContext};
-    use super::{resolve, xor_push, apply_mark_internal, new_sctable_internal};
-    use super::{apply_rename_internal, apply_renames, marksof_internal, resolve_internal};
-    use super::{SCTable, EmptyCtxt, Mark, Rename, IllegalCtxt};
-    use std::collections::HashMap;
-
-    #[test]
-    fn xorpush_test () {
-        let mut s = Vec::new();
-        xor_push(&mut s, 14);
-        assert_eq!(s.clone(), [14]);
-        xor_push(&mut s, 14);
-        assert_eq!(s.clone(), []);
-        xor_push(&mut s, 14);
-        assert_eq!(s.clone(), [14]);
-        xor_push(&mut s, 15);
-        assert_eq!(s.clone(), [14, 15]);
-        xor_push(&mut s, 16);
-        assert_eq!(s.clone(), [14, 15, 16]);
-        xor_push(&mut s, 16);
-        assert_eq!(s.clone(), [14, 15]);
-        xor_push(&mut s, 15);
-        assert_eq!(s.clone(), [14]);
-    }
+    use super::{resolve, apply_mark_internal, new_sctable_internal};
+    use super::{SCTable, Mark};
 
     fn id(n: u32, s: SyntaxContext) -> Ident {
         Ident::new(Name(n), s)
     }
 
-    // because of the SCTable, I now need a tidy way of
-    // creating syntax objects. Sigh.
-    #[derive(Clone, PartialEq, Debug)]
-    enum TestSC {
-        M(Mrk),
-        R(Ident,Name)
-    }
-
-    // unfold a vector of TestSC values into a SCTable,
-    // returning the resulting index
-    fn unfold_test_sc(tscs : Vec<TestSC> , tail: SyntaxContext, table: &SCTable)
-        -> SyntaxContext {
-        tscs.iter().rev().fold(tail, |tail : SyntaxContext, tsc : &TestSC|
-                  {match *tsc {
-                      M(mrk) => apply_mark_internal(mrk,tail,table),
-                      R(ident,name) => apply_rename_internal(ident,name,tail,table)}})
-    }
-
-    // gather a SyntaxContext back into a vector of TestSCs
-    fn refold_test_sc(mut sc: SyntaxContext, table : &SCTable) -> Vec<TestSC> {
-        let mut result = Vec::new();
-        loop {
-            let table = table.table.borrow();
-            match (*table)[sc.0 as usize] {
-                EmptyCtxt => {return result;},
-                Mark(mrk,tail) => {
-                    result.push(M(mrk));
-                    sc = tail;
-                    continue;
-                },
-                Rename(id,name,tail) => {
-                    result.push(R(id,name));
-                    sc = tail;
-                    continue;
-                }
-                IllegalCtxt => panic!("expected resolvable context, got IllegalCtxt")
-            }
-        }
-    }
-
-    #[test]
-    fn test_unfold_refold(){
-        let mut t = new_sctable_internal();
-
-        let test_sc = vec!(M(3),R(id(101,EMPTY_CTXT),Name(14)),M(9));
-        assert_eq!(unfold_test_sc(test_sc.clone(),EMPTY_CTXT,&mut t),SyntaxContext(4));
-        {
-            let table = t.table.borrow();
-            assert!((*table)[2] == Mark(9,EMPTY_CTXT));
-            assert!((*table)[3] == Rename(id(101,EMPTY_CTXT),Name(14),SyntaxContext(2)));
-            assert!((*table)[4] == Mark(3,SyntaxContext(3)));
-        }
-        assert_eq!(refold_test_sc(SyntaxContext(4),&t),test_sc);
-    }
-
     // extend a syntax context with a sequence of marks given
     // in a vector. v[0] will be the outermost mark.
     fn unfold_marks(mrks: Vec<Mrk> , tail: SyntaxContext, table: &SCTable)
@@ -384,97 +201,11 @@ mod tests {
     }
 
     #[test]
-    fn test_marksof () {
-        let stopname = Name(242);
-        let name1 = Name(243);
-        let mut t = new_sctable_internal();
-        assert_eq!(marksof_internal (EMPTY_CTXT,stopname,&t),Vec::new());
-        // FIXME #5074: ANF'd to dodge nested calls
-        { let ans = unfold_marks(vec!(4,98),EMPTY_CTXT,&mut t);
-         assert_eq! (marksof_internal (ans,stopname,&t), [4, 98]);}
-        // does xoring work?
-        { let ans = unfold_marks(vec!(5,5,16),EMPTY_CTXT,&mut t);
-         assert_eq! (marksof_internal (ans,stopname,&t), [16]);}
-        // does nested xoring work?
-        { let ans = unfold_marks(vec!(5,10,10,5,16),EMPTY_CTXT,&mut t);
-         assert_eq! (marksof_internal (ans, stopname,&t), [16]);}
-        // rename where stop doesn't match:
-        { let chain = vec!(M(9),
-                        R(id(name1.0,
-                             apply_mark_internal (4, EMPTY_CTXT,&mut t)),
-                          Name(100101102)),
-                        M(14));
-         let ans = unfold_test_sc(chain,EMPTY_CTXT,&mut t);
-         assert_eq! (marksof_internal (ans, stopname, &t), [9, 14]);}
-        // rename where stop does match
-        { let name1sc = apply_mark_internal(4, EMPTY_CTXT, &mut t);
-         let chain = vec!(M(9),
-                       R(id(name1.0, name1sc),
-                         stopname),
-                       M(14));
-         let ans = unfold_test_sc(chain,EMPTY_CTXT,&mut t);
-         assert_eq! (marksof_internal (ans, stopname, &t), [9]); }
-    }
-
-
-    #[test]
-    fn resolve_tests () {
-        let a = 40;
-        let mut t = new_sctable_internal();
-        let mut rt = HashMap::new();
-        // - ctxt is MT
-        assert_eq!(resolve_internal(id(a,EMPTY_CTXT),&mut t, &mut rt),Name(a));
-        // - simple ignored marks
-        { let sc = unfold_marks(vec!(1,2,3),EMPTY_CTXT,&mut t);
-         assert_eq!(resolve_internal(id(a,sc),&mut t, &mut rt),Name(a));}
-        // - orthogonal rename where names don't match
-        { let sc = unfold_test_sc(vec!(R(id(50,EMPTY_CTXT),Name(51)),M(12)),EMPTY_CTXT,&mut t);
-         assert_eq!(resolve_internal(id(a,sc),&mut t, &mut rt),Name(a));}
-        // - rename where names do match, but marks don't
-        { let sc1 = apply_mark_internal(1,EMPTY_CTXT,&mut t);
-         let sc = unfold_test_sc(vec!(R(id(a,sc1),Name(50)),
-                                   M(1),
-                                   M(2)),
-                                 EMPTY_CTXT,&mut t);
-        assert_eq!(resolve_internal(id(a,sc),&mut t, &mut rt), Name(a));}
-        // - rename where names and marks match
-        { let sc1 = unfold_test_sc(vec!(M(1),M(2)),EMPTY_CTXT,&mut t);
-         let sc = unfold_test_sc(vec!(R(id(a,sc1),Name(50)),M(1),M(2)),EMPTY_CTXT,&mut t);
-         assert_eq!(resolve_internal(id(a,sc),&mut t, &mut rt), Name(50)); }
-        // - rename where names and marks match by literal sharing
-        { let sc1 = unfold_test_sc(vec!(M(1),M(2)),EMPTY_CTXT,&mut t);
-         let sc = unfold_test_sc(vec!(R(id(a,sc1),Name(50))),sc1,&mut t);
-         assert_eq!(resolve_internal(id(a,sc),&mut t, &mut rt), Name(50)); }
-        // - two renames of the same var.. can only happen if you use
-        // local-expand to prevent the inner binding from being renamed
-        // during the rename-pass caused by the first:
-        println!("about to run bad test");
-        { let sc = unfold_test_sc(vec!(R(id(a,EMPTY_CTXT),Name(50)),
-                                    R(id(a,EMPTY_CTXT),Name(51))),
-                                  EMPTY_CTXT,&mut t);
-         assert_eq!(resolve_internal(id(a,sc),&mut t, &mut rt), Name(51)); }
-        // the simplest double-rename:
-        { let a_to_a50 = apply_rename_internal(id(a,EMPTY_CTXT),Name(50),EMPTY_CTXT,&mut t);
-         let a50_to_a51 = apply_rename_internal(id(a,a_to_a50),Name(51),a_to_a50,&mut t);
-         assert_eq!(resolve_internal(id(a,a50_to_a51),&mut t, &mut rt),Name(51));
-         // mark on the outside doesn't stop rename:
-         let sc = apply_mark_internal(9,a50_to_a51,&mut t);
-         assert_eq!(resolve_internal(id(a,sc),&mut t, &mut rt),Name(51));
-         // but mark on the inside does:
-         let a50_to_a51_b = unfold_test_sc(vec!(R(id(a,a_to_a50),Name(51)),
-                                              M(9)),
-                                           a_to_a50,
-                                           &mut t);
-         assert_eq!(resolve_internal(id(a,a50_to_a51_b),&mut t, &mut rt),Name(50));}
-    }
-
-    #[test]
     fn mtwt_resolve_test(){
         let a = 40;
         assert_eq!(resolve(id(a,EMPTY_CTXT)),Name(a));
     }
 
-
     #[test]
     fn hashing_tests () {
         let mut t = new_sctable_internal();
@@ -484,26 +215,4 @@ mod tests {
         assert_eq!(apply_mark_internal(12,EMPTY_CTXT,&mut t),SyntaxContext(2));
         // I'm assuming that the rename table will behave the same....
     }
-
-    #[test]
-    fn resolve_table_hashing_tests() {
-        let mut t = new_sctable_internal();
-        let mut rt = HashMap::new();
-        assert_eq!(rt.len(),0);
-        resolve_internal(id(30,EMPTY_CTXT),&mut t, &mut rt);
-        assert_eq!(rt.len(),1);
-        resolve_internal(id(39,EMPTY_CTXT),&mut t, &mut rt);
-        assert_eq!(rt.len(),2);
-        resolve_internal(id(30,EMPTY_CTXT),&mut t, &mut rt);
-        assert_eq!(rt.len(),2);
-    }
-
-    #[test]
-    fn new_resolves_test() {
-        let renames = vec!((Ident::with_empty_ctxt(Name(23)),Name(24)),
-                           (Ident::with_empty_ctxt(Name(29)),Name(29)));
-        let new_ctxt1 = apply_renames(&renames,EMPTY_CTXT);
-        assert_eq!(resolve(Ident::new(Name(23),new_ctxt1)),Name(24));
-        assert_eq!(resolve(Ident::new(Name(29),new_ctxt1)),Name(29));
-    }
 }
diff --git a/src/test/run-pass/hygiene.rs b/src/test/run-pass/hygiene.rs
new file mode 100644
index 00000000000..4507ba50192
--- /dev/null
+++ b/src/test/run-pass/hygiene.rs
@@ -0,0 +1,116 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![allow(unused)]
+
+fn f() {
+    let x = 0;
+    macro_rules! foo { () => {
+        assert_eq!(x, 0);
+    } }
+
+    let x = 1;
+    foo!();
+}
+
+fn g() {
+    let x = 0;
+    macro_rules! m { ($x:ident) => {
+        macro_rules! m2 { () => { ($x, x) } }
+        let x = 1;
+        macro_rules! m3 { () => { ($x, x) } }
+    } }
+
+    let x = 2;
+    m!(x);
+
+    let x = 3;
+    assert_eq!(m2!(), (2, 0));
+    assert_eq!(m3!(), (2, 1));
+
+    let x = 4;
+    m!(x);
+    assert_eq!(m2!(), (4, 0));
+    assert_eq!(m3!(), (4, 1));
+}
+
+mod foo {
+    macro_rules! m {
+        ($f:ident : |$x:ident| $e:expr) => {
+            pub fn $f() -> (i32, i32) {
+                let x = 0;
+                let $x = 1;
+                (x, $e)
+            }
+        }
+    }
+
+    m!(f: |x| x + 10);
+}
+
+fn interpolated_pattern() {
+    let x = 0;
+    macro_rules! m {
+        ($p:pat, $e:expr) => {
+            let $p = 1;
+            assert_eq!((x, $e), (0, 1));
+        }
+    }
+
+    m!(x, x);
+}
+
+fn patterns_in_macro_generated_macros() {
+    let x = 0;
+    macro_rules! m {
+        ($a:expr, $b:expr) => {
+            assert_eq!(x, 0);
+            let x = $a;
+            macro_rules! n {
+                () => {
+                    (x, $b)
+                }
+            }
+        }
+    }
+
+    let x = 1;
+    m!(2, x);
+
+    let x = 3;
+    assert_eq!(n!(), (2, 1));
+}
+
+fn match_hygiene() {
+    let x = 0;
+
+    macro_rules! m {
+        ($p:pat, $e:expr) => {
+            for result in &[Ok(1), Err(1)] {
+                match *result {
+                    $p => { assert_eq!(($e, x), (1, 0)); }
+                    Err(x) => { assert_eq!(($e, x), (2, 1)); }
+                }
+            }
+        }
+    }
+
+    let x = 2;
+    m!(Ok(x), x);
+}
+
+fn main() {
+    f();
+    g();
+    assert_eq!(foo::f(), (0, 11));
+    interpolated_pattern();
+    patterns_in_macro_generated_macros();
+    match_hygiene();
+}