make walk/visit_mac opt-in only

macros can expand into arbitrary items, exprs, etc. This
means that using a default walker or folder on an AST before
macro expansion is complete will miss things (the things that
the macros expand into). As a partial fence against this, this
commit moves the default traversal of macros into a separate
procedure, and makes the default trait implementation signal
an error. This means that Folders and Visitors can traverse
macros if they want to, but they need to explicitly add an
impl that calls the walk_mac or fold_mac procedure

This should prevent problems down the road.

6 files changed, 71 insertions, 14 deletions

diff --git a/src/librustc/front/config.rs b/src/librustc/front/config.rs
index 9bff6620aaa..0c39cf350a6 100644
--- a/src/librustc/front/config.rs
+++ b/src/librustc/front/config.rs
@@ -14,6 +14,8 @@ use syntax::codemap;
 
 use std::gc::{Gc, GC};
 
+/// A folder that strips out items that do not belong in the current
+/// configuration.
 struct Context<'a> {
     in_cfg: |attrs: &[ast::Attribute]|: 'a -> bool,
 }
@@ -41,6 +43,9 @@ impl<'a> fold::Folder for Context<'a> {
     fn fold_expr(&mut self, expr: Gc<ast::Expr>) -> Gc<ast::Expr> {
         fold_expr(self, expr)
     }
+    fn fold_mac(&mut self, mac: &ast::Mac) -> ast::Mac {
+        fold::fold_mac(mac, self)
+    }
 }
 
 pub fn strip_items(krate: ast::Crate,
diff --git a/src/librustc/plugin/load.rs b/src/librustc/plugin/load.rs
index 79d0690653f..499cffa42aa 100644
--- a/src/librustc/plugin/load.rs
+++ b/src/librustc/plugin/load.rs
@@ -72,6 +72,7 @@ pub fn load_plugins(sess: &Session, krate: &ast::Crate) -> Plugins {
     loader.plugins
 }
 
+// note that macros aren't expanded yet, and therefore macros can't add plugins.
 impl<'a> Visitor<()> for PluginLoader<'a> {
     fn visit_view_item(&mut self, vi: &ast::ViewItem, _: ()) {
         match vi.node {
@@ -109,6 +110,10 @@ impl<'a> Visitor<()> for PluginLoader<'a> {
             _ => (),
         }
     }
+    fn visit_mac(&mut self, _: &ast::Mac, _:()) {
+        // bummer... can't see plugins inside macros.
+        // do nothing.
+    }
 }
 
 impl<'a> PluginLoader<'a> {
diff --git a/src/libsyntax/ast_map.rs b/src/libsyntax/ast_map.rs
index 25c8e81bdbc..de2ecd9a264 100644
--- a/src/libsyntax/ast_map.rs
+++ b/src/libsyntax/ast_map.rs
@@ -112,6 +112,7 @@ pub enum Node {
     NodeLifetime(Gc<Lifetime>),
 }
 
+/// Represents an entry and its parent Node ID
 /// The odd layout is to bring down the total size.
 #[deriving(Clone)]
 enum MapEntry {
@@ -184,6 +185,8 @@ impl MapEntry {
     }
 }
 
+/// Represents a mapping from Node IDs to AST elements and their parent
+/// Node IDs
 pub struct Map {
     /// NodeIds are sequential integers from 0, so we can be
     /// super-compact by storing them in a vector. Not everything with
@@ -430,6 +433,8 @@ pub trait FoldOps {
     }
 }
 
+/// A Folder that walks over an AST and constructs a Node ID Map. Its
+/// fold_ops argument has the opportunity to replace Node IDs and spans.
 pub struct Ctx<'a, F> {
     map: &'a Map,
     /// The node in which we are currently mapping (an item or a method).
@@ -584,6 +589,10 @@ impl<'a, F: FoldOps> Folder for Ctx<'a, F> {
         self.insert(lifetime.id, EntryLifetime(self.parent, box(GC) lifetime));
         lifetime
     }
+
+    fn fold_mac(&mut self, mac: &Mac) -> Mac {
+        fold::fold_mac(mac, self)
+    }
 }
 
 pub fn map_crate<F: FoldOps>(krate: Crate, fold_ops: F) -> (Crate, Map) {
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index e8a78e85d89..084faca02c5 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -753,7 +753,6 @@ impl Visitor<()> for PatIdentFinder {
             _ => visit::walk_pat(self, pattern, ())
         }
     }
-
 }
 
 /// find the PatIdent paths in a pattern
@@ -902,6 +901,9 @@ impl<'a> Folder for IdentRenamer<'a> {
             ctxt: mtwt::apply_renames(self.renames, id.ctxt),
         }
     }
+    fn fold_mac(&mut self, macro: &ast::Mac) -> ast::Mac {
+        fold::fold_mac(macro, self)
+    }
 }
 
 /// A tree-folder that applies every rename in its list to
@@ -931,6 +933,9 @@ impl<'a> Folder for PatIdentRenamer<'a> {
             _ => noop_fold_pat(pat, self)
         }
     }
+    fn fold_mac(&mut self, macro: &ast::Mac) -> ast::Mac {
+        fold::fold_mac(macro, self)
+    }
 }
 
 // expand a method
diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs
index f7cb1ae1934..3e3b57be6e4 100644
--- a/src/libsyntax/fold.rs
+++ b/src/libsyntax/fold.rs
@@ -8,6 +8,16 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+//! A Folder represents an AST->AST fold; it accepts an AST piece,
+//! and returns a piece of the same type. So, for instance, macro
+//! expansion is a Folder that walks over an AST and produces another
+//! AST.
+//!
+//! Note: using a Folder (other than the MacroExpander Folder) on
+//! an AST before macro expansion is probably a bad idea. For instance,
+//! a folder renaming item names in a module will miss all of those
+//! that are created by the expansion of a macro.
+
 use ast::*;
 use ast;
 use ast_util;
@@ -299,17 +309,13 @@ pub trait Folder {
         }
     }
 
-    fn fold_mac(&mut self, macro: &Mac) -> Mac {
-        Spanned {
-            node: match macro.node {
-                MacInvocTT(ref p, ref tts, ctxt) => {
-                    MacInvocTT(self.fold_path(p),
-                               fold_tts(tts.as_slice(), self),
-                               ctxt)
-                }
-            },
-            span: self.new_span(macro.span)
-        }
+    fn fold_mac(&mut self, _macro: &Mac) -> Mac {
+        fail!("fold_mac disabled by default");
+        // NB: see note about macros above.
+        // if you really want a folder that
+        // works on macros, use this
+        // definition in your trait impl:
+        // fold::fold_mac(_macro, self)
     }
 
     fn map_exprs(&self, f: |Gc<Expr>| -> Gc<Expr>,
@@ -361,6 +367,20 @@ pub trait Folder {
 
 }
 
+
+pub fn fold_mac<T: Folder>(macro: &Mac, fld: &mut T) -> Mac {
+    Spanned {
+        node: match macro.node {
+            MacInvocTT(ref p, ref tts, ctxt) => {
+                MacInvocTT(fld.fold_path(p),
+                           fold_tts(tts.as_slice(), fld),
+                           ctxt)
+            }
+        },
+        span: fld.new_span(macro.span)
+    }
+}
+
 /* some little folds that probably aren't useful to have in Folder itself*/
 
 //used in noop_fold_item and noop_fold_crate and noop_fold_crate_directive
@@ -986,6 +1006,7 @@ mod test {
     use util::parser_testing::{string_to_crate, matches_codepattern};
     use parse::token;
     use print::pprust;
+    use fold;
     use super::*;
 
     // this version doesn't care about getting comments or docstrings in.
@@ -1001,6 +1022,9 @@ mod test {
         fn fold_ident(&mut self, _: ast::Ident) -> ast::Ident {
             token::str_to_ident("zz")
         }
+        fn fold_mac(&mut self, macro: &ast::Mac) -> ast::Mac {
+            fold::fold_mac(macro, self)
+        }
     }
 
     // maybe add to expand.rs...
diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs
index 9298b58c426..7caaf2f6cc1 100644
--- a/src/libsyntax/visit.rs
+++ b/src/libsyntax/visit.rs
@@ -20,6 +20,10 @@
 //! execute before AST node B, then A is visited first.  The borrow checker in
 //! particular relies on this property.
 //!
+//! Note: walking an AST before macro expansion is probably a bad idea. For
+//! instance, a walker looking for item names in a module will miss all of
+//! those that are created by the expansion of a macro.
+
 use abi::Abi;
 use ast::*;
 use ast;
@@ -124,8 +128,13 @@ pub trait Visitor<E: Clone> {
     fn visit_explicit_self(&mut self, es: &ExplicitSelf, e: E) {
         walk_explicit_self(self, es, e)
     }
-    fn visit_mac(&mut self, macro: &Mac, e: E) {
-        walk_mac(self, macro, e)
+    fn visit_mac(&mut self, _macro: &Mac, _e: E) {
+        fail!("visit_mac disabled by default");
+        // NB: see note about macros above.
+        // if you really want a visitor that
+        // works on macros, use this
+        // definition in your trait impl:
+        // visit::walk_mac(self, _macro, _e)
     }
     fn visit_path(&mut self, path: &Path, _id: ast::NodeId, e: E) {
         walk_path(self, path, e)