Auto merge of #36601 - jseyfried:build_reduced_graph_in_expansion, r=nrc

Assign def ids and build the module graph during expansion

r? @nrc

3 files changed, 171 insertions, 158 deletions

diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 9202f8c0946..55bf5dcf1cf 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -20,21 +20,25 @@ use {NameBinding, NameBindingKind, ToNameBinding};
 use Resolver;
 use {resolve_error, resolve_struct_error, ResolutionError};
 
+use rustc::middle::cstore::LoadedMacro;
 use rustc::hir::def::*;
 use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
 use rustc::hir::map::DefPathData;
 use rustc::ty;
 
 use std::cell::Cell;
+use std::rc::Rc;
 
 use syntax::ast::Name;
 use syntax::attr;
 use syntax::parse::token;
 
-use syntax::ast::{Block, Crate};
-use syntax::ast::{ForeignItem, ForeignItemKind, Item, ItemKind};
+use syntax::ast::{self, Block, ForeignItem, ForeignItemKind, Item, ItemKind};
 use syntax::ast::{Mutability, StmtKind, TraitItem, TraitItemKind};
 use syntax::ast::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple};
+use syntax::ext::base::{MultiItemModifier, Resolver as SyntaxResolver};
+use syntax::ext::hygiene::Mark;
+use syntax::feature_gate::{self, emit_feature_err};
 use syntax::parse::token::keywords;
 use syntax::visit::{self, Visitor};
 
@@ -53,11 +57,6 @@ impl<'a> ToNameBinding<'a> for (Def, Span, ty::Visibility) {
 }
 
 impl<'b> Resolver<'b> {
-    /// Constructs the reduced graph for the entire crate.
-    pub fn build_reduced_graph(&mut self, krate: &Crate) {
-        visit::walk_crate(&mut BuildReducedGraphVisitor { resolver: self }, krate);
-    }
-
     /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined;
     /// otherwise, reports an error.
     fn define<T>(&mut self, parent: Module<'b>, name: Name, ns: Namespace, def: T)
@@ -72,15 +71,13 @@ impl<'b> Resolver<'b> {
     fn block_needs_anonymous_module(&mut self, block: &Block) -> bool {
         // If any statements are items, we need to create an anonymous module
         block.stmts.iter().any(|statement| match statement.node {
-            StmtKind::Item(_) => true,
+            StmtKind::Item(_) | StmtKind::Mac(_) => true,
             _ => false,
         })
     }
 
     /// Constructs the reduced graph for one item.
     fn build_reduced_graph_for_item(&mut self, item: &Item) {
-        self.crate_loader.process_item(item, &self.definitions);
-
         let parent = self.current_module;
         let name = item.ident.name;
         let sp = item.span;
@@ -188,8 +185,20 @@ impl<'b> Resolver<'b> {
             }
 
             ItemKind::ExternCrate(_) => {
-                // n.b. we don't need to look at the path option here, because cstore already
-                // did
+                // We need to error on `#[macro_use] extern crate` when it isn't at the
+                // crate root, because `$crate` won't work properly.
+                let is_crate_root = self.current_module.parent.is_none();
+                for def in self.crate_loader.load_macros(item, is_crate_root) {
+                    match def {
+                        LoadedMacro::Def(def) => self.add_macro(Mark::root(), def),
+                        LoadedMacro::CustomDerive(name, ext) => {
+                            self.insert_custom_derive(&name, ext, item.span);
+                        }
+                    }
+                }
+                self.crate_loader.process_item(item, &self.definitions);
+
+                // n.b. we don't need to look at the path option here, because cstore already did
                 if let Some(crate_id) = self.session.cstore.extern_mod_stmt_cnum(item.id) {
                     let def_id = DefId {
                         krate: crate_id,
@@ -206,6 +215,8 @@ impl<'b> Resolver<'b> {
                 }
             }
 
+            ItemKind::Mod(..) if item.ident == keywords::Invalid.ident() => {} // Crate root
+
             ItemKind::Mod(..) => {
                 let def = Def::Mod(self.definitions.local_def_id(item.id));
                 let module = self.arenas.alloc_module(ModuleS {
@@ -213,6 +224,7 @@ impl<'b> Resolver<'b> {
                         attr::contains_name(&item.attrs, "no_implicit_prelude")
                     },
                     normal_ancestor_id: Some(item.id),
+                    macros_escape: self.contains_macro_use(&item.attrs),
                     ..ModuleS::new(Some(parent), ModuleKind::Def(def, name))
                 });
                 self.define(parent, name, TypeNS, (module, sp, vis));
@@ -222,7 +234,7 @@ impl<'b> Resolver<'b> {
                 self.current_module = module;
             }
 
-            ItemKind::ForeignMod(..) => {}
+            ItemKind::ForeignMod(..) => self.crate_loader.process_item(item, &self.definitions),
 
             // These items live in the value namespace.
             ItemKind::Static(_, m, _) => {
@@ -476,14 +488,79 @@ impl<'b> Resolver<'b> {
         }
         module.populated.set(true)
     }
+
+    // does this attribute list contain "macro_use"?
+    fn contains_macro_use(&mut self, attrs: &[ast::Attribute]) -> bool {
+        for attr in attrs {
+            if attr.check_name("macro_escape") {
+                let msg = "macro_escape is a deprecated synonym for macro_use";
+                let mut err = self.session.struct_span_warn(attr.span, msg);
+                if let ast::AttrStyle::Inner = attr.node.style {
+                    err.help("consider an outer attribute, #[macro_use] mod ...").emit();
+                } else {
+                    err.emit();
+                }
+            } else if !attr.check_name("macro_use") {
+                continue;
+            }
+
+            if !attr.is_word() {
+                self.session.span_err(attr.span, "arguments to macro_use are not allowed here");
+            }
+            return true;
+        }
+
+        false
+    }
+
+    fn insert_custom_derive(&mut self, name: &str, ext: Rc<MultiItemModifier>, sp: Span) {
+        if !self.session.features.borrow().rustc_macro {
+            let sess = &self.session.parse_sess;
+            let msg = "loading custom derive macro crates is experimentally supported";
+            emit_feature_err(sess, "rustc_macro", sp, feature_gate::GateIssue::Language, msg);
+        }
+        if self.derive_modes.insert(token::intern(name), ext).is_some() {
+            self.session.span_err(sp, &format!("cannot shadow existing derive mode `{}`", name));
+        }
+    }
+}
+
+pub struct BuildReducedGraphVisitor<'a, 'b: 'a> {
+    pub resolver: &'a mut Resolver<'b>,
+}
+
+impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
+    fn visit_invoc(&mut self, id: ast::NodeId) {
+        self.resolver.expansion_data.get_mut(&id.as_u32()).unwrap().module =
+            self.resolver.current_module;
+    }
 }
 
-struct BuildReducedGraphVisitor<'a, 'b: 'a> {
-    resolver: &'a mut Resolver<'b>,
+macro_rules! method {
+    ($visit:ident: $ty:ty, $invoc:path, $walk:ident) => {
+        fn $visit(&mut self, node: &$ty) {
+            match node.node {
+                $invoc(..) => self.visit_invoc(node.id),
+                _ => visit::$walk(self, node),
+            }
+        }
+    }
 }
 
 impl<'a, 'b> Visitor for BuildReducedGraphVisitor<'a, 'b> {
+    method!(visit_impl_item: ast::ImplItem, ast::ImplItemKind::Macro, walk_impl_item);
+    method!(visit_stmt:      ast::Stmt,     ast::StmtKind::Mac,       walk_stmt);
+    method!(visit_expr:      ast::Expr,     ast::ExprKind::Mac,       walk_expr);
+    method!(visit_pat:       ast::Pat,      ast::PatKind::Mac,        walk_pat);
+    method!(visit_ty:        ast::Ty,       ast::TyKind::Mac,         walk_ty);
+
     fn visit_item(&mut self, item: &Item) {
+        match item.node {
+            ItemKind::Mac(..) if item.id == ast::DUMMY_NODE_ID => return, // Scope placeholder
+            ItemKind::Mac(..) => return self.visit_invoc(item.id),
+            _ => {}
+        }
+
         let parent = self.resolver.current_module;
         self.resolver.build_reduced_graph_for_item(item);
         visit::walk_item(self, item);
@@ -492,6 +569,7 @@ impl<'a, 'b> Visitor for BuildReducedGraphVisitor<'a, 'b> {
 
     fn visit_foreign_item(&mut self, foreign_item: &ForeignItem) {
         self.resolver.build_reduced_graph_for_foreign_item(foreign_item);
+        visit::walk_foreign_item(self, foreign_item);
     }
 
     fn visit_block(&mut self, block: &Block) {
@@ -515,7 +593,7 @@ impl<'a, 'b> Visitor for BuildReducedGraphVisitor<'a, 'b> {
                 (Def::Method(item_def_id), ValueNS)
             }
             TraitItemKind::Type(..) => (Def::AssociatedTy(item_def_id), TypeNS),
-            TraitItemKind::Macro(_) => panic!("unexpanded macro in resolve!"),
+            TraitItemKind::Macro(_) => return self.visit_invoc(item.id),
         };
 
         self.resolver.trait_item_map.insert((item.ident.name, def_id), is_static_method);
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 38b31db6781..0694edd2357 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -42,7 +42,7 @@ use self::RibKind::*;
 use self::UseLexicalScopeFlag::*;
 use self::ModulePrefixResult::*;
 
-use rustc::hir::map::Definitions;
+use rustc::hir::map::{Definitions, DefCollector};
 use rustc::hir::{self, PrimTy, TyBool, TyChar, TyFloat, TyInt, TyUint, TyStr};
 use rustc::middle::cstore::CrateLoader;
 use rustc::session::Session;
@@ -57,6 +57,7 @@ use syntax::ext::base::MultiItemModifier;
 use syntax::ext::hygiene::Mark;
 use syntax::ast::{self, FloatTy};
 use syntax::ast::{CRATE_NODE_ID, Name, NodeId, IntTy, UintTy};
+use syntax::ext::base::SyntaxExtension;
 use syntax::parse::token::{self, keywords};
 use syntax::util::lev_distance::find_best_match_for_name;
 
@@ -72,9 +73,9 @@ use syntax_pos::{Span, DUMMY_SP};
 use errors::DiagnosticBuilder;
 
 use std::cell::{Cell, RefCell};
-use std::rc::Rc;
 use std::fmt;
 use std::mem::replace;
+use std::rc::Rc;
 
 use resolve_imports::{ImportDirective, NameResolution};
 
@@ -791,6 +792,9 @@ pub struct ModuleS<'a> {
     // access the children must be preceded with a
     // `populate_module_if_necessary` call.
     populated: Cell<bool>,
+
+    macros: RefCell<FnvHashMap<Name, Rc<SyntaxExtension>>>,
+    macros_escape: bool,
 }
 
 pub type Module<'a> = &'a ModuleS<'a>;
@@ -808,6 +812,8 @@ impl<'a> ModuleS<'a> {
             globs: RefCell::new((Vec::new())),
             traits: RefCell::new(None),
             populated: Cell::new(true),
+            macros: RefCell::new(FnvHashMap()),
+            macros_escape: false,
         }
     }
 
@@ -1079,7 +1085,7 @@ pub struct Resolver<'a> {
     macro_names: FnvHashSet<Name>,
 
     // Maps the `Mark` of an expansion to its containing module or block.
-    expansion_data: FnvHashMap<u32, macros::ExpansionData>,
+    expansion_data: FnvHashMap<u32, macros::ExpansionData<'a>>,
 }
 
 pub struct ResolverArenas<'a> {
@@ -1193,13 +1199,16 @@ impl<'a> Resolver<'a> {
         let mut module_map = NodeMap();
         module_map.insert(CRATE_NODE_ID, graph_root);
 
+        let mut definitions = Definitions::new();
+        DefCollector::new(&mut definitions).collect_root();
+
         let mut expansion_data = FnvHashMap();
-        expansion_data.insert(0, macros::ExpansionData::default()); // Crate root expansion
+        expansion_data.insert(0, macros::ExpansionData::root(graph_root)); // Crate root expansion
 
         Resolver {
             session: session,
 
-            definitions: Definitions::new(),
+            definitions: definitions,
             macros_at_scope: FnvHashMap(),
 
             // The outermost module has def ID 0; this is not reflected in the
@@ -1269,6 +1278,13 @@ impl<'a> Resolver<'a> {
 
     /// Entry point to crate resolution.
     pub fn resolve_crate(&mut self, krate: &Crate) {
+        // Collect `DefId`s for exported macro defs.
+        for def in &krate.exported_macros {
+            DefCollector::new(&mut self.definitions).with_parent(CRATE_DEF_INDEX, |collector| {
+                collector.visit_macro_def(def)
+            })
+        }
+
         self.current_module = self.graph_root;
         visit::walk_crate(self, krate);
 
diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs
index 3a9fb845190..17f2dff28c3 100644
--- a/src/librustc_resolve/macros.rs
+++ b/src/librustc_resolve/macros.rs
@@ -8,36 +8,38 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use Resolver;
-use rustc::middle::cstore::LoadedMacro;
-use rustc::util::nodemap::FnvHashMap;
-use std::cell::RefCell;
-use std::mem;
+use {Module, Resolver};
+use build_reduced_graph::BuildReducedGraphVisitor;
+use rustc::hir::def_id::{CRATE_DEF_INDEX, DefIndex};
+use rustc::hir::map::{self, DefCollector};
 use std::rc::Rc;
-use syntax::ast::{self, Name};
+use syntax::ast;
 use syntax::errors::DiagnosticBuilder;
 use syntax::ext::base::{self, MultiModifier, MultiDecorator, MultiItemModifier};
-use syntax::ext::base::{NormalTT, Resolver as SyntaxResolver, SyntaxExtension};
+use syntax::ext::base::{NormalTT, SyntaxExtension};
 use syntax::ext::expand::{Expansion, Invocation, InvocationKind};
 use syntax::ext::hygiene::Mark;
 use syntax::ext::tt::macro_rules;
-use syntax::feature_gate::{self, emit_feature_err};
-use syntax::parse::token::{self, intern};
+use syntax::parse::token::intern;
 use syntax::util::lev_distance::find_best_match_for_name;
-use syntax::visit::{self, Visitor};
-use syntax_pos::Span;
 
-#[derive(Clone, Default)]
-pub struct ExpansionData {
-    module: Rc<ModuleData>,
+#[derive(Clone)]
+pub struct ExpansionData<'a> {
+    pub module: Module<'a>,
+    def_index: DefIndex,
+    // True if this expansion is in a `const_integer` position, for example `[u32; m!()]`.
+    // c.f. `DefCollector::visit_ast_const_integer`.
+    const_integer: bool,
 }
 
-// FIXME(jseyfried): merge with `::ModuleS`.
-#[derive(Default)]
-struct ModuleData {
-    parent: Option<Rc<ModuleData>>,
-    macros: RefCell<FnvHashMap<Name, Rc<SyntaxExtension>>>,
-    macros_escape: bool,
+impl<'a> ExpansionData<'a> {
+    pub fn root(graph_root: Module<'a>) -> Self {
+        ExpansionData {
+            module: graph_root,
+            def_index: CRATE_DEF_INDEX,
+            const_integer: false,
+        }
+    }
 }
 
 impl<'a> base::Resolver for Resolver<'a> {
@@ -45,11 +47,21 @@ impl<'a> base::Resolver for Resolver<'a> {
         self.session.next_node_id()
     }
 
-    fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion) {
-        expansion.visit_with(&mut ExpansionVisitor {
-            current_module: self.expansion_data[&mark.as_u32()].module.clone(),
-            resolver: self,
+    fn get_module_scope(&mut self, id: ast::NodeId) -> Mark {
+        let mark = Mark::fresh();
+        let module = self.module_map[&id];
+        self.expansion_data.insert(mark.as_u32(), ExpansionData {
+            module: module,
+            def_index: module.def_id().unwrap().index,
+            const_integer: false,
         });
+        mark
+    }
+
+    fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion) {
+        self.collect_def_ids(mark, expansion);
+        self.current_module = self.expansion_data[&mark.as_u32()].module;
+        expansion.visit_with(&mut BuildReducedGraphVisitor { resolver: self });
     }
 
     fn add_macro(&mut self, scope: Mark, mut def: ast::MacroDef) {
@@ -71,9 +83,9 @@ impl<'a> base::Resolver for Resolver<'a> {
             self.macro_names.insert(ident.name);
         }
 
-        let mut module = self.expansion_data[&scope.as_u32()].module.clone();
+        let mut module = self.expansion_data[&scope.as_u32()].module;
         while module.macros_escape {
-            module = module.parent.clone().unwrap();
+            module = module.parent.unwrap();
         }
         module.macros.borrow_mut().insert(ident.name, ext);
     }
@@ -113,12 +125,12 @@ impl<'a> base::Resolver for Resolver<'a> {
             InvocationKind::Attr { ref attr, .. } => (intern(&*attr.name()), attr.span),
         };
 
-        let mut module = self.expansion_data[&scope.as_u32()].module.clone();
+        let mut module = self.expansion_data[&scope.as_u32()].module;
         loop {
             if let Some(ext) = module.macros.borrow().get(&name) {
                 return Some(ext.clone());
             }
-            match module.parent.clone() {
+            match module.parent {
                 Some(parent) => module = parent,
                 None => break,
             }
@@ -147,116 +159,23 @@ impl<'a> Resolver<'a> {
         }
     }
 
-    fn insert_custom_derive(&mut self, name: &str, ext: Rc<MultiItemModifier>, sp: Span) {
-        if !self.session.features.borrow().rustc_macro {
-            let sess = &self.session.parse_sess;
-            let msg = "loading custom derive macro crates is experimentally supported";
-            emit_feature_err(sess, "rustc_macro", sp, feature_gate::GateIssue::Language, msg);
-        }
-        if self.derive_modes.insert(token::intern(name), ext).is_some() {
-            self.session.span_err(sp, &format!("cannot shadow existing derive mode `{}`", name));
-        }
-    }
-}
-
-struct ExpansionVisitor<'b, 'a: 'b> {
-    resolver: &'b mut Resolver<'a>,
-    current_module: Rc<ModuleData>,
-}
+    fn collect_def_ids(&mut self, mark: Mark, expansion: &Expansion) {
+        let expansion_data = &mut self.expansion_data;
+        let ExpansionData { def_index, const_integer, module } = expansion_data[&mark.as_u32()];
+        let visit_macro_invoc = &mut |invoc: map::MacroInvocationData| {
+            expansion_data.entry(invoc.id.as_u32()).or_insert(ExpansionData {
+                def_index: invoc.def_index,
+                const_integer: invoc.const_integer,
+                module: module,
+            });
+        };
 
-impl<'a, 'b> ExpansionVisitor<'a, 'b> {
-    fn visit_invoc(&mut self, id: ast::NodeId) {
-        self.resolver.expansion_data.insert(id.as_u32(), ExpansionData {
-            module: self.current_module.clone(),
+        let mut def_collector = DefCollector::new(&mut self.definitions);
+        def_collector.visit_macro_invoc = Some(visit_macro_invoc);
+        def_collector.with_parent(def_index, |def_collector| if !const_integer {
+            expansion.visit_with(def_collector)
+        } else if let Expansion::Expr(ref expr) = *expansion {
+            def_collector.visit_ast_const_integer(expr);
         });
     }
-
-    // does this attribute list contain "macro_use"?
-    fn contains_macro_use(&mut self, attrs: &[ast::Attribute]) -> bool {
-        for attr in attrs {
-            if attr.check_name("macro_escape") {
-                let msg = "macro_escape is a deprecated synonym for macro_use";
-                let mut err = self.resolver.session.struct_span_warn(attr.span, msg);
-                if let ast::AttrStyle::Inner = attr.node.style {
-                    err.help("consider an outer attribute, #[macro_use] mod ...").emit();
-                } else {
-                    err.emit();
-                }
-            } else if !attr.check_name("macro_use") {
-                continue;
-            }
-
-            if !attr.is_word() {
-                self.resolver.session.span_err(attr.span,
-                                               "arguments to macro_use are not allowed here");
-            }
-            return true;
-        }
-
-        false
-    }
-}
-
-macro_rules! method {
-    ($visit:ident: $ty:ty, $invoc:path, $walk:ident) => {
-        fn $visit(&mut self, node: &$ty) {
-            match node.node {
-                $invoc(..) => self.visit_invoc(node.id),
-                _ => visit::$walk(self, node),
-            }
-        }
-    }
-}
-
-impl<'a, 'b> Visitor for ExpansionVisitor<'a, 'b>  {
-    method!(visit_trait_item: ast::TraitItem, ast::TraitItemKind::Macro, walk_trait_item);
-    method!(visit_impl_item:  ast::ImplItem,  ast::ImplItemKind::Macro,  walk_impl_item);
-    method!(visit_stmt:       ast::Stmt,      ast::StmtKind::Mac,        walk_stmt);
-    method!(visit_expr:       ast::Expr,      ast::ExprKind::Mac,        walk_expr);
-    method!(visit_pat:        ast::Pat,       ast::PatKind::Mac,         walk_pat);
-    method!(visit_ty:         ast::Ty,        ast::TyKind::Mac,          walk_ty);
-
-    fn visit_item(&mut self, item: &ast::Item) {
-        match item.node {
-            ast::ItemKind::Mac(..) if item.id == ast::DUMMY_NODE_ID => {} // Scope placeholder
-            ast::ItemKind::Mac(..) => self.visit_invoc(item.id),
-            ast::ItemKind::Mod(..) => {
-                let module_data = ModuleData {
-                    parent: Some(self.current_module.clone()),
-                    macros: RefCell::new(FnvHashMap()),
-                    macros_escape: self.contains_macro_use(&item.attrs),
-                };
-                let orig_module = mem::replace(&mut self.current_module, Rc::new(module_data));
-                visit::walk_item(self, item);
-                self.current_module = orig_module;
-            }
-            ast::ItemKind::ExternCrate(..) => {
-                // We need to error on `#[macro_use] extern crate` when it isn't at the
-                // crate root, because `$crate` won't work properly.
-                // FIXME(jseyfried): This will be nicer once `ModuleData` is merged with `ModuleS`.
-                let is_crate_root = self.current_module.parent.as_ref().unwrap().parent.is_none();
-                for def in self.resolver.crate_loader.load_macros(item, is_crate_root) {
-                    match def {
-                        LoadedMacro::Def(def) => self.resolver.add_macro(Mark::root(), def),
-                        LoadedMacro::CustomDerive(name, ext) => {
-                            self.resolver.insert_custom_derive(&name, ext, item.span);
-                        }
-                    }
-                }
-                visit::walk_item(self, item);
-            }
-            _ => visit::walk_item(self, item),
-        }
-    }
-
-    fn visit_block(&mut self, block: &ast::Block) {
-        let module_data = ModuleData {
-            parent: Some(self.current_module.clone()),
-            macros: RefCell::new(FnvHashMap()),
-            macros_escape: false,
-        };
-        let orig_module = mem::replace(&mut self.current_module, Rc::new(module_data));
-        visit::walk_block(self, block);
-        self.current_module = orig_module;
-    }
 }