Auto merge of #34032 - jseyfried:load_macros_in_expansion, r=nrc

Support `#[macro_use]` on macro-expanded crates

This PR loads macros from `#[macro_use]` crates during expansion so that
 - macro-expanded `#[macro_use]` crates work (fixes #33936, fixes #28071), and
 - macros imported from crates have the same scope as macros imported from modules.

This is a [breaking-change]. For example, this will break:
```rust
macro_rules! m {
    () => { #[macro_use(foo)] extern crate core; } //~ ERROR imported macro not found
}
m!();
```
Also, this will break:
```rust
macro_rules! try { () => {} }
// #[macro_use] mod bar { macro_rules! try { ... } } //< ... just like this would ...
fn main() { try!(); } //< ... making this an error
```

r? @nrc

9 files changed, 99 insertions, 97 deletions

diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs
index c63122948ff..386496b071d 100644
--- a/src/librustc_driver/driver.rs
+++ b/src/librustc_driver/driver.rs
@@ -604,10 +604,6 @@ pub fn phase_2_configure_and_expand<'a>(sess: &Session,
         syntax::std_inject::maybe_inject_crates_ref(krate, sess.opts.alt_std_name.clone())
     });
 
-    let macros = time(time_passes,
-                      "macro loading",
-                      || macro_import::read_macro_defs(sess, &cstore, &krate, crate_name));
-
     let mut addl_plugins = Some(addl_plugins);
     let registrars = time(time_passes, "plugin loading", || {
         plugin::load::load_plugins(sess,
@@ -696,13 +692,14 @@ pub fn phase_2_configure_and_expand<'a>(sess: &Session,
             recursion_limit: sess.recursion_limit.get(),
             trace_mac: sess.opts.debugging_opts.trace_macros,
         };
+        let mut loader = macro_import::MacroLoader::new(sess, &cstore, crate_name);
         let mut ecx = syntax::ext::base::ExtCtxt::new(&sess.parse_sess,
                                                       krate.config.clone(),
                                                       cfg,
-                                                      &mut feature_gated_cfgs);
+                                                      &mut feature_gated_cfgs,
+                                                      &mut loader);
         syntax_ext::register_builtins(&mut ecx.syntax_env);
         let (ret, macro_names) = syntax::ext::expand::expand_crate(ecx,
-                                                                   macros,
                                                                    syntax_exts,
                                                                    krate);
         if cfg!(windows) {
diff --git a/src/librustc_metadata/macro_import.rs b/src/librustc_metadata/macro_import.rs
index 911ca7e315c..1c7d37709c2 100644
--- a/src/librustc_metadata/macro_import.rs
+++ b/src/librustc_metadata/macro_import.rs
@@ -20,24 +20,19 @@ use syntax::codemap::Span;
 use syntax::parse::token;
 use syntax::ast;
 use syntax::attr;
-use syntax::visit;
-use syntax::visit::Visitor;
 use syntax::attr::AttrMetaMethods;
+use syntax::ext;
 
-struct MacroLoader<'a> {
+pub struct MacroLoader<'a> {
     sess: &'a Session,
-    span_whitelist: HashSet<Span>,
     reader: CrateReader<'a>,
-    macros: Vec<ast::MacroDef>,
 }
 
 impl<'a> MacroLoader<'a> {
-    fn new(sess: &'a Session, cstore: &'a CStore, crate_name: &str) -> MacroLoader<'a> {
+    pub fn new(sess: &'a Session, cstore: &'a CStore, crate_name: &str) -> MacroLoader<'a> {
         MacroLoader {
             sess: sess,
-            span_whitelist: HashSet::new(),
             reader: CrateReader::new(sess, cstore, crate_name),
-            macros: vec![],
         }
     }
 }
@@ -46,48 +41,15 @@ pub fn call_bad_macro_reexport(a: &Session, b: Span) {
     span_err!(a, b, E0467, "bad macro reexport");
 }
 
-/// Read exported macros.
-pub fn read_macro_defs(sess: &Session,
-                       cstore: &CStore,
-                       krate: &ast::Crate,
-                       crate_name: &str)
-                       -> Vec<ast::MacroDef>
-{
-    let mut loader = MacroLoader::new(sess, cstore, crate_name);
-
-    // We need to error on `#[macro_use] extern crate` when it isn't at the
-    // crate root, because `$crate` won't work properly. Identify these by
-    // spans, because the crate map isn't set up yet.
-    for item in &krate.module.items {
-        if let ast::ItemKind::ExternCrate(_) = item.node {
-            loader.span_whitelist.insert(item.span);
-        }
-    }
-
-    visit::walk_crate(&mut loader, krate);
-
-    loader.macros
-}
-
 pub type MacroSelection = HashMap<token::InternedString, Span>;
 
-// note that macros aren't expanded yet, and therefore macros can't add macro imports.
-impl<'a, 'v> Visitor<'v> for MacroLoader<'a> {
-    fn visit_item(&mut self, item: &ast::Item) {
-        // We're only interested in `extern crate`.
-        match item.node {
-            ast::ItemKind::ExternCrate(_) => {}
-            _ => {
-                visit::walk_item(self, item);
-                return;
-            }
-        }
-
+impl<'a> ext::base::MacroLoader for MacroLoader<'a> {
+    fn load_crate(&mut self, extern_crate: &ast::Item, allows_macros: bool) -> Vec<ast::MacroDef> {
         // Parse the attributes relating to macros.
         let mut import = Some(HashMap::new());  // None => load all
         let mut reexport = HashMap::new();
 
-        for attr in &item.attrs {
+        for attr in &extern_crate.attrs {
             let mut used = true;
             match &attr.name()[..] {
                 "macro_use" => {
@@ -130,36 +92,33 @@ impl<'a, 'v> Visitor<'v> for MacroLoader<'a> {
             }
         }
 
-        self.load_macros(item, import, reexport)
-    }
-
-    fn visit_mac(&mut self, _: &ast::Mac) {
-        // bummer... can't see macro imports inside macros.
-        // do nothing.
+        self.load_macros(extern_crate, allows_macros, import, reexport)
     }
 }
 
 impl<'a> MacroLoader<'a> {
     fn load_macros<'b>(&mut self,
                        vi: &ast::Item,
+                       allows_macros: bool,
                        import: Option<MacroSelection>,
-                       reexport: MacroSelection) {
+                       reexport: MacroSelection)
+                       -> Vec<ast::MacroDef> {
         if let Some(sel) = import.as_ref() {
             if sel.is_empty() && reexport.is_empty() {
-                return;
+                return Vec::new();
             }
         }
 
-        if !self.span_whitelist.contains(&vi.span) {
+        if !allows_macros {
             span_err!(self.sess, vi.span, E0468,
                       "an `extern crate` loading macros must be at the crate root");
-            return;
+            return Vec::new();
         }
 
-        let macros = self.reader.read_exported_macros(vi);
+        let mut macros = Vec::new();
         let mut seen = HashSet::new();
 
-        for mut def in macros {
+        for mut def in self.reader.read_exported_macros(vi) {
             let name = def.ident.name.as_str();
 
             def.use_locally = match import.as_ref() {
@@ -170,7 +129,7 @@ impl<'a> MacroLoader<'a> {
             def.allow_internal_unstable = attr::contains_name(&def.attrs,
                                                               "allow_internal_unstable");
             debug!("load_macros: loaded: {:?}", def);
-            self.macros.push(def);
+            macros.push(def);
             seen.insert(name);
         }
 
@@ -189,5 +148,7 @@ impl<'a> MacroLoader<'a> {
                           "reexported macro not found");
             }
         }
+
+        macros
     }
 }
diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs
index 303187aeba8..95624a43373 100644
--- a/src/libsyntax/ext/base.rs
+++ b/src/libsyntax/ext/base.rs
@@ -536,6 +536,17 @@ fn initial_syntax_expander_table<'feat>(ecfg: &expand::ExpansionConfig<'feat>)
     syntax_expanders
 }
 
+pub trait MacroLoader {
+    fn load_crate(&mut self, extern_crate: &ast::Item, allows_macros: bool) -> Vec<ast::MacroDef>;
+}
+
+pub struct DummyMacroLoader;
+impl MacroLoader for DummyMacroLoader {
+    fn load_crate(&mut self, _: &ast::Item, _: bool) -> Vec<ast::MacroDef> {
+        Vec::new()
+    }
+}
+
 /// One of these is made during expansion and incrementally updated as we go;
 /// when a macro expansion occurs, the resulting nodes have the backtrace()
 /// -> expn_info of their expansion context stored into their span.
@@ -546,6 +557,7 @@ pub struct ExtCtxt<'a> {
     pub ecfg: expand::ExpansionConfig<'a>,
     pub crate_root: Option<&'static str>,
     pub feature_gated_cfgs: &'a mut Vec<GatedCfgAttr>,
+    pub loader: &'a mut MacroLoader,
 
     pub mod_path: Vec<ast::Ident> ,
     pub exported_macros: Vec<ast::MacroDef>,
@@ -561,7 +573,9 @@ pub struct ExtCtxt<'a> {
 impl<'a> ExtCtxt<'a> {
     pub fn new(parse_sess: &'a parse::ParseSess, cfg: ast::CrateConfig,
                ecfg: expand::ExpansionConfig<'a>,
-               feature_gated_cfgs: &'a mut Vec<GatedCfgAttr>) -> ExtCtxt<'a> {
+               feature_gated_cfgs: &'a mut Vec<GatedCfgAttr>,
+               loader: &'a mut MacroLoader)
+               -> ExtCtxt<'a> {
         let env = initial_syntax_expander_table(&ecfg);
         ExtCtxt {
             parse_sess: parse_sess,
@@ -572,6 +586,7 @@ impl<'a> ExtCtxt<'a> {
             crate_root: None,
             feature_gated_cfgs: feature_gated_cfgs,
             exported_macros: Vec::new(),
+            loader: loader,
             syntax_env: env,
             recursion_count: 0,
 
@@ -925,4 +940,10 @@ impl SyntaxEnv {
         let last_chain_index = self.chain.len() - 1;
         &mut self.chain[last_chain_index].info
     }
+
+    pub fn is_crate_root(&mut self) -> bool {
+        // The first frame is pushed in `SyntaxEnv::new()` and the second frame is
+        // pushed when folding the crate root pseudo-module (c.f. noop_fold_crate).
+        self.chain.len() == 2
+    }
 }
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index 7fee27c5dd4..15d192b59b8 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -726,13 +726,11 @@ fn expand_annotatable(a: Annotatable,
     let new_items: SmallVector<Annotatable> = match a {
         Annotatable::Item(it) => match it.node {
             ast::ItemKind::Mac(..) => {
-                let new_items: SmallVector<P<ast::Item>> = it.and_then(|it| match it.node {
+                it.and_then(|it| match it.node {
                     ItemKind::Mac(mac) =>
                         expand_mac_invoc(mac, Some(it.ident), it.attrs, it.span, fld),
                     _ => unreachable!(),
-                });
-
-                new_items.into_iter().map(|i| Annotatable::Item(i)).collect()
+                })
             }
             ast::ItemKind::Mod(_) | ast::ItemKind::ForeignMod(_) => {
                 let valid_ident =
@@ -748,10 +746,19 @@ fn expand_annotatable(a: Annotatable,
                 if valid_ident {
                     fld.cx.mod_pop();
                 }
-                result.into_iter().map(|i| Annotatable::Item(i)).collect()
+                result
             },
-            _ => noop_fold_item(it, fld).into_iter().map(|i| Annotatable::Item(i)).collect(),
-        },
+            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.
+                let allows_macros = fld.cx.syntax_env.is_crate_root();
+                for def in fld.cx.loader.load_crate(&it, allows_macros) {
+                    fld.cx.insert_macro(def);
+                }
+                SmallVector::one(it)
+            },
+            _ => noop_fold_item(it, fld),
+        }.into_iter().map(|i| Annotatable::Item(i)).collect(),
 
         Annotatable::TraitItem(it) => match it.node {
             ast::TraitItemKind::Method(_, Some(_)) => {
@@ -1137,8 +1144,6 @@ impl<'feat> ExpansionConfig<'feat> {
 }
 
 pub fn expand_crate(mut cx: ExtCtxt,
-                    // these are the macros being imported to this crate:
-                    imported_macros: Vec<ast::MacroDef>,
                     user_exts: Vec<NamedSyntaxExtension>,
                     c: Crate) -> (Crate, HashSet<Name>) {
     if std_inject::no_core(&c) {
@@ -1151,10 +1156,6 @@ pub fn expand_crate(mut cx: ExtCtxt,
     let ret = {
         let mut expander = MacroExpander::new(&mut cx);
 
-        for def in imported_macros {
-            expander.cx.insert_macro(def);
-        }
-
         for (name, extension) in user_exts {
             expander.cx.syntax_env.insert(name, extension);
         }
@@ -1220,7 +1221,7 @@ mod tests {
     use ast;
     use ast::Name;
     use codemap;
-    use ext::base::ExtCtxt;
+    use ext::base::{ExtCtxt, DummyMacroLoader};
     use ext::mtwt;
     use fold::Folder;
     use parse;
@@ -1291,9 +1292,9 @@ mod tests {
             src,
             Vec::new(), &sess).unwrap();
         // should fail:
-        let mut gated_cfgs = vec![];
-        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut gated_cfgs);
-        expand_crate(ecx, vec![], vec![], crate_ast);
+        let (mut gated_cfgs, mut loader) = (vec![], DummyMacroLoader);
+        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut gated_cfgs, &mut loader);
+        expand_crate(ecx, vec![], crate_ast);
     }
 
     // make sure that macros can't escape modules
@@ -1306,9 +1307,9 @@ mod tests {
             "<test>".to_string(),
             src,
             Vec::new(), &sess).unwrap();
-        let mut gated_cfgs = vec![];
-        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut gated_cfgs);
-        expand_crate(ecx, vec![], vec![], crate_ast);
+        let (mut gated_cfgs, mut loader) = (vec![], DummyMacroLoader);
+        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut gated_cfgs, &mut loader);
+        expand_crate(ecx, vec![], crate_ast);
     }
 
     // macro_use modules should allow macros to escape
@@ -1320,18 +1321,18 @@ mod tests {
             "<test>".to_string(),
             src,
             Vec::new(), &sess).unwrap();
-        let mut gated_cfgs = vec![];
-        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut gated_cfgs);
-        expand_crate(ecx, vec![], vec![], crate_ast);
+        let (mut gated_cfgs, mut loader) = (vec![], DummyMacroLoader);
+        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut gated_cfgs, &mut loader);
+        expand_crate(ecx, vec![], crate_ast);
     }
 
     fn expand_crate_str(crate_str: String) -> ast::Crate {
         let ps = parse::ParseSess::new();
         let crate_ast = panictry!(string_to_parser(&ps, crate_str).parse_crate_mod());
         // the cfg argument actually does matter, here...
-        let mut gated_cfgs = vec![];
-        let ecx = ExtCtxt::new(&ps, vec![], test_ecfg(), &mut gated_cfgs);
-        expand_crate(ecx, vec![], vec![], crate_ast).0
+        let (mut gated_cfgs, mut loader) = (vec![], DummyMacroLoader);
+        let ecx = ExtCtxt::new(&ps, vec![], test_ecfg(), &mut gated_cfgs, &mut loader);
+        expand_crate(ecx, vec![], crate_ast).0
     }
 
     // find the pat_ident paths in a crate
diff --git a/src/libsyntax/test.rs b/src/libsyntax/test.rs
index 6fbbed2ee98..2ac4aac65de 100644
--- a/src/libsyntax/test.rs
+++ b/src/libsyntax/test.rs
@@ -25,7 +25,7 @@ use codemap;
 use errors;
 use config;
 use entry::{self, EntryPointType};
-use ext::base::ExtCtxt;
+use ext::base::{ExtCtxt, DummyMacroLoader};
 use ext::build::AstBuilder;
 use ext::expand::ExpansionConfig;
 use fold::Folder;
@@ -271,12 +271,14 @@ fn generate_test_harness(sess: &ParseSess,
     let krate = cleaner.fold_crate(krate);
 
     let mut feature_gated_cfgs = vec![];
+    let mut loader = DummyMacroLoader;
     let mut cx: TestCtxt = TestCtxt {
         sess: sess,
         span_diagnostic: sd,
         ext_cx: ExtCtxt::new(sess, vec![],
                              ExpansionConfig::default("test".to_string()),
-                             &mut feature_gated_cfgs),
+                             &mut feature_gated_cfgs,
+                             &mut loader),
         path: Vec::new(),
         testfns: Vec::new(),
         reexport_test_harness_main: reexport_test_harness_main,
diff --git a/src/test/compile-fail-fulldeps/expanded-macro-use.rs b/src/test/compile-fail-fulldeps/expanded-macro-use.rs
new file mode 100644
index 00000000000..98ed3e7145b
--- /dev/null
+++ b/src/test/compile-fail-fulldeps/expanded-macro-use.rs
@@ -0,0 +1,19 @@
+// 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.
+
+#![feature(rustc_private)]
+macro_rules! m {
+    () => { #[macro_use] extern crate syntax; }
+}
+m!();
+
+fn main() {
+    help!(); //~ ERROR unexpected end of macro invocation
+}
diff --git a/src/test/compile-fail-fulldeps/qquote.rs b/src/test/compile-fail-fulldeps/qquote.rs
index 3e153a21e5d..ca896266411 100644
--- a/src/test/compile-fail-fulldeps/qquote.rs
+++ b/src/test/compile-fail-fulldeps/qquote.rs
@@ -21,10 +21,11 @@ use syntax::print::pprust;
 
 fn main() {
     let ps = syntax::parse::ParseSess::new();
+    let mut loader = syntax::ext::base::DummyMacroLoader;
     let mut cx = syntax::ext::base::ExtCtxt::new(
         &ps, vec![],
         syntax::ext::expand::ExpansionConfig::default("qquote".to_string()),
-        &mut Vec::new());
+        &mut Vec::new(), &mut loader);
     cx.bt_push(syntax::codemap::ExpnInfo {
         call_site: DUMMY_SP,
         callee: syntax::codemap::NameAndSpan {
diff --git a/src/test/run-fail-fulldeps/qquote.rs b/src/test/run-fail-fulldeps/qquote.rs
index 41a6fd05c37..fa6ee98317a 100644
--- a/src/test/run-fail-fulldeps/qquote.rs
+++ b/src/test/run-fail-fulldeps/qquote.rs
@@ -23,11 +23,11 @@ use syntax::print::pprust;
 
 fn main() {
     let ps = syntax::parse::ParseSess::new();
-    let mut feature_gated_cfgs = vec![];
+    let (mut feature_gated_cfgs, mut loader) = (vec![], syntax::ext::base::DummyMacroLoader);
     let mut cx = syntax::ext::base::ExtCtxt::new(
         &ps, vec![],
         syntax::ext::expand::ExpansionConfig::default("qquote".to_string()),
-        &mut feature_gated_cfgs);
+        &mut feature_gated_cfgs, &mut loader);
     cx.bt_push(syntax::codemap::ExpnInfo {
         call_site: DUMMY_SP,
         callee: syntax::codemap::NameAndSpan {
diff --git a/src/test/run-pass-fulldeps/qquote.rs b/src/test/run-pass-fulldeps/qquote.rs
index 0bb3e610020..efc1989a4fb 100644
--- a/src/test/run-pass-fulldeps/qquote.rs
+++ b/src/test/run-pass-fulldeps/qquote.rs
@@ -20,11 +20,11 @@ use syntax::parse::token::intern;
 
 fn main() {
     let ps = syntax::parse::ParseSess::new();
-    let mut feature_gated_cfgs = vec![];
+    let (mut feature_gated_cfgs, mut loader) = (vec![], syntax::ext::base::DummyMacroLoader);
     let mut cx = syntax::ext::base::ExtCtxt::new(
         &ps, vec![],
         syntax::ext::expand::ExpansionConfig::default("qquote".to_string()),
-        &mut feature_gated_cfgs);
+        &mut feature_gated_cfgs, &mut loader);
     cx.bt_push(syntax::codemap::ExpnInfo {
         call_site: DUMMY_SP,
         callee: syntax::codemap::NameAndSpan {