auto merge of #15233 : jbclements/rust/match-var-hygiene-etc, r=cmr

This PR includes two big things and a bunch of little ones.

1) It enables hygiene for variables bound by 'match' expressions.
2) It fixes a bug discovered indirectly (#15221), wherein fold traversal failed to visit nonterminal nodes.
3) It fixes a small bug in the macro tutorial.

It also adds tests for the first two, and makes a bunch of small comment improvements and cleanup.

Author: bors

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
         );

src/libsyntax/ast.rs

@@ -401,6 +401,7 @@ pub enum Decl_ {
     DeclItem(Gc<Item>),
 }
 
+/// represents one arm of a 'match'
 #[deriving(Clone, PartialEq, Eq, Encodable, Decodable, Hash)]
 pub struct Arm {
     pub attrs: Vec<Attribute>,

src/libsyntax/ext/expand.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<Vec<SyntaxContext_>>,
     mark_memo: RefCell<HashMap<(SyntaxContext,Mrk),SyntaxContext>>,
@@ -160,7 +161,7 @@ fn with_resolve_table_mut<T>(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 resolve 500+ seconds in resolve for librustc (!)
 fn resolve_internal(id: Ident,
                     table: &SCTable,
                     resolve_table: &mut ResolveTable) -> Name {

src/libsyntax/ext/mtwt.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_<T: Folder>(mi: Gc<MetaItem>, fld: &mut T) -> Gc<MetaItem> {
         span: fld.new_span(mi.span) }
 }
 
-//used in noop_fold_item and noop_fold_crate
-fn fold_attribute_<T: Folder>(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_<T: Folder>(a: &Arg, fld: &mut T) -> Arg {
     let id = fld.new_id(a.id); // Needs to be first, for ast_map.
@@ -387,53 +389,80 @@ fn fold_arg_<T: Folder>(a: &Arg, fld: &mut T) -> Arg {
     }
 }
 
-// build a new vector of tts by appling the Folder's fold_ident to
-// all of the identifiers in the token trees.
-//
-// This is part of hygiene magic. As far as hygiene is concerned, there
-// are three types of let pattern bindings or loop labels:
-//      - those defined and used in non-macro part of the program
-//      - those used as part of macro invocation arguments
-//      - those defined and used inside macro definitions
-// Lexically, type 1 and 2 are in one group and type 3 the other. If they
-// clash, in order for let and loop label to work hygienically, one group
-// or the other needs to be renamed. The problem is that type 2 and 3 are
-// parsed together (inside the macro expand function). After being parsed and
-// AST being constructed, they can no longer be distinguished from each other.
-//
-// For that reason, type 2 let bindings and loop labels are actually renamed
-// in the form of tokens instead of AST nodes, here. There are wasted effort
-// since many token::IDENT are not necessary part of let bindings and most
-// token::LIFETIME are certainly not loop labels. But we can't tell in their
-// token form. So this is less ideal and hacky but it works.
-pub fn fold_tts<T: Folder>(tts: &[TokenTree], fld: &mut T) -> Vec<TokenTree> {
-    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<T: Folder>(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<T: Folder>(tts: &[TokenTree], fld: &mut T) -> Vec<TokenTree> {
+    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: Folder>(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: Folder>(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<T: Folder>(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<T: Folder>(decl: &FnDecl, fld: &mut T) -> P<FnDecl> {
     P(FnDecl {
         inputs: decl.inputs.iter().map(|x| fold_arg_(x, fld)).collect(), // bad copy
@@ -526,7 +555,7 @@ fn fold_struct_field<T: Folder>(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 +607,7 @@ pub fn noop_fold_view_item<T: Folder>(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 +687,7 @@ pub fn noop_fold_type_method<T: Folder>(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,14 +710,21 @@ pub fn noop_fold_mod<T: Folder>(m: &Mod, folder: &mut T) -> Mod {
 pub fn noop_fold_crate<T: Folder>(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),
     }
 }
 
+// fold one item into possibly many items
 pub fn noop_fold_item<T: Folder>(i: &Item,
                                  folder: &mut T) -> SmallVector<Gc<Item>> {
+    SmallVector::one(box(GC) noop_fold_item_(i,folder))
+}
+
+
+// fold one item into exactly one item
+pub fn noop_fold_item_<T: Folder>(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 {
@@ -699,14 +735,14 @@ pub fn noop_fold_item<T: Folder>(i: &Item,
         _ => i.ident
     };
 
-    SmallVector::one(box(GC) Item {
+    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)
-    })
+    }
 }
 
 pub fn noop_fold_foreign_item<T: Folder>(ni: &ForeignItem,
@@ -715,7 +751,7 @@ pub fn noop_fold_foreign_item<T: Folder>(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 +775,7 @@ pub fn noop_fold_method<T: Folder>(m: &Method, folder: &mut T) -> Gc<Method> {
     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,

src/libsyntax/fold.rs

@@ -114,6 +114,7 @@ pub enum Nonterminal {
     NtPat( Gc<ast::Pat>),
     NtExpr(Gc<ast::Expr>),
     NtTy(  P<ast::Ty>),
+    // see IDENT, above, for meaning of bool in NtIdent:
     NtIdent(Box<ast::Ident>, bool),
     NtMeta(Gc<ast::MetaItem>), // stuff inside brackets for attributes
     NtPath(Box<ast::Path>),

src/libsyntax/parse/token.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;
 

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;
 

src/test/bench/msgsend-ring-rw-arcs.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;

src/test/bench/shootout-meteor.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)]