From 51499b6e1fd892b68eeb28eaec9031f01a6a9409 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 2 Jun 2016 01:14:33 +0000 Subject: [PATCH 1/3] Load macros from `extern crate`s during expansion. --- src/librustc_driver/driver.rs | 9 +-- src/librustc_metadata/macro_import.rs | 75 ++++++------------------ src/libsyntax/ext/base.rs | 21 ++++++- src/libsyntax/ext/expand.rs | 41 +++++++------ src/libsyntax/test.rs | 6 +- src/test/compile-fail-fulldeps/qquote.rs | 3 +- src/test/run-fail-fulldeps/qquote.rs | 4 +- src/test/run-pass-fulldeps/qquote.rs | 4 +- 8 files changed, 73 insertions(+), 90 deletions(-) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index c63122948ff3a..386496b071d90 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 911ca7e315c1f..1c7d37709c220 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, reader: CrateReader<'a>, - macros: Vec, } 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 -{ - 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; -// 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 { // 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, - reexport: MacroSelection) { + reexport: MacroSelection) + -> Vec { 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 303187aeba87d..4b7086695eb7d 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; +} + +pub struct DummyMacroLoader; +impl MacroLoader for DummyMacroLoader { + fn load_crate(&mut self, _: &ast::Item, _: bool) -> Vec { + 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, + pub loader: &'a mut MacroLoader, pub mod_path: Vec , pub exported_macros: Vec, @@ -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) -> ExtCtxt<'a> { + feature_gated_cfgs: &'a mut Vec, + 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,8 @@ 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 { + self.chain.len() == 2 + } } diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 7fee27c5dd463..c581a149f43fa 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -750,6 +750,15 @@ fn expand_annotatable(a: Annotatable, } result.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(Annotatable::Item(it)) + }, _ => noop_fold_item(it, fld).into_iter().map(|i| Annotatable::Item(i)).collect(), }, @@ -1137,8 +1146,6 @@ impl<'feat> ExpansionConfig<'feat> { } pub fn expand_crate(mut cx: ExtCtxt, - // these are the macros being imported to this crate: - imported_macros: Vec, user_exts: Vec, c: Crate) -> (Crate, HashSet) { if std_inject::no_core(&c) { @@ -1151,10 +1158,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 +1223,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 +1294,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 +1309,9 @@ mod tests { "".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 +1323,18 @@ mod tests { "".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 6fbbed2ee9842..2ac4aac65debe 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/qquote.rs b/src/test/compile-fail-fulldeps/qquote.rs index 3e153a21e5d38..ca896266411de 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 41a6fd05c3741..fa6ee98317a94 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 0bb3e610020a1..efc1989a4fba2 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 { From 13e3925e8d816b71f3d857217981d2e3d9988b5e Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 2 Jun 2016 02:24:49 +0000 Subject: [PATCH 2/3] Add regression test --- .../expanded-macro-use.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 src/test/compile-fail-fulldeps/expanded-macro-use.rs 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 0000000000000..98ed3e7145bc8 --- /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 or the MIT license +// , 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 +} From dbf0326ddc041e772b5ab07b19e893e8955bf934 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 9 Jun 2016 00:47:52 +0000 Subject: [PATCH 3/3] Add comment and clean up `expand_annotatable` --- src/libsyntax/ext/base.rs | 2 ++ src/libsyntax/ext/expand.rs | 14 ++++++-------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 4b7086695eb7d..95624a433730a 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -942,6 +942,8 @@ impl SyntaxEnv { } 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 c581a149f43fa..15d192b59b81e 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 = match a { Annotatable::Item(it) => match it.node { ast::ItemKind::Mac(..) => { - let new_items: SmallVector> = 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,7 +746,7 @@ fn expand_annotatable(a: Annotatable, if valid_ident { fld.cx.mod_pop(); } - result.into_iter().map(|i| Annotatable::Item(i)).collect() + result }, ast::ItemKind::ExternCrate(_) => { // We need to error on `#[macro_use] extern crate` when it isn't at the @@ -757,10 +755,10 @@ fn expand_annotatable(a: Annotatable, for def in fld.cx.loader.load_crate(&it, allows_macros) { fld.cx.insert_macro(def); } - SmallVector::one(Annotatable::Item(it)) + SmallVector::one(it) }, - _ => noop_fold_item(it, fld).into_iter().map(|i| Annotatable::Item(i)).collect(), - }, + _ => noop_fold_item(it, fld), + }.into_iter().map(|i| Annotatable::Item(i)).collect(), Annotatable::TraitItem(it) => match it.node { ast::TraitItemKind::Method(_, Some(_)) => {