macros 1.1: future proofing and cleanup

src/librustc/middle/cstore.rs

@@ -35,7 +35,6 @@ use session::Session;
 use session::search_paths::PathKind;
 use util::nodemap::{NodeSet, DefIdMap};
 use std::path::PathBuf;
-use std::rc::Rc;
 use syntax::ast;
 use syntax::attr;
 use syntax::ext::base::MultiItemModifier;
@@ -425,7 +424,7 @@ pub struct LoadedMacro {
 
 pub enum LoadedMacroKind {
     Def(ast::MacroDef),
-    CustomDerive(String, Rc<MultiItemModifier>),
+    CustomDerive(String, Box<MultiItemModifier>),
 }
 
 pub trait CrateLoader {

src/librustc_metadata/macro_import.rs

@@ -11,7 +11,6 @@
 //! Used by `rustc` when loading a crate with exported macros.
 
 use std::collections::HashSet;
-use std::rc::Rc;
 use std::env;
 use std::mem;
 
@@ -212,7 +211,7 @@ impl<'a> CrateLoader<'a> {
             fn register_custom_derive(&mut self,
                                       trait_name: &str,
                                       expand: fn(TokenStream) -> TokenStream) {
-                let derive = Rc::new(CustomDerive::new(expand));
+                let derive = Box::new(CustomDerive::new(expand));
                 self.0.push(LoadedMacro {
                     kind: LoadedMacroKind::CustomDerive(trait_name.to_string(), derive),
                     import_site: self.1,

src/librustc_resolve/build_reduced_graph.rs

@@ -36,7 +36,7 @@ use syntax::parse::token;
 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::base::{SyntaxExtension, Resolver as SyntaxResolver};
 use syntax::ext::hygiene::Mark;
 use syntax::feature_gate::{self, emit_feature_err};
 use syntax::ext::tt::macro_rules;
@@ -195,33 +195,45 @@ impl<'b> Resolver<'b> {
                 // 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();
+                let import_macro = |this: &mut Self, name, ext, span| {
+                    let shadowing = this.builtin_macros.insert(name, Rc::new(ext)).is_some();
+                    if shadowing && expansion != Mark::root() {
+                        let msg = format!("`{}` is already in scope", name);
+                        this.session.struct_span_err(span, &msg)
+                            .note("macro-expanded `#[macro_use]`s may not shadow \
+                                   existing macros (see RFC 1560)")
+                            .emit();
+                    }
+                };
+
+                let mut custom_derive_crate = false;
                 for loaded_macro in self.crate_loader.load_macros(item, is_crate_root) {
                     match loaded_macro.kind {
                         LoadedMacroKind::Def(mut def) => {
-                            let name = def.ident.name;
                             if def.use_locally {
-                                let ext =
-                                    Rc::new(macro_rules::compile(&self.session.parse_sess, &def));
-                                if self.builtin_macros.insert(name, ext).is_some() &&
-                                   expansion != Mark::root() {
-                                    let msg = format!("`{}` is already in scope", name);
-                                    self.session.struct_span_err(loaded_macro.import_site, &msg)
-                                        .note("macro-expanded `#[macro_use]`s may not shadow \
-                                               existing macros (see RFC 1560)")
-                                        .emit();
-                                }
-                                self.macro_names.insert(name);
+                                self.macro_names.insert(def.ident.name);
+                                let ext = macro_rules::compile(&self.session.parse_sess, &def);
+                                import_macro(self, def.ident.name, ext, loaded_macro.import_site);
                             }
                             if def.export {
                                 def.id = self.next_node_id();
                                 self.exported_macros.push(def);
                             }
                         }
                         LoadedMacroKind::CustomDerive(name, ext) => {
-                            self.insert_custom_derive(&name, ext, item.span);
+                            custom_derive_crate = true;
+                            let ext = SyntaxExtension::CustomDerive(ext);
+                            import_macro(self, token::intern(&name), ext, loaded_macro.import_site);
                         }
                     }
                 }
+
+                if custom_derive_crate && !self.session.features.borrow().proc_macro {
+                    let issue = feature_gate::GateIssue::Language;
+                    let msg = "loading custom derive macro crates is experimentally supported";
+                    emit_feature_err(&self.session.parse_sess, "proc_macro", item.span, issue, msg);
+                }
+
                 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
@@ -238,6 +250,12 @@ impl<'b> Resolver<'b> {
                     self.define(parent, name, TypeNS, (module, sp, vis));
 
                     self.populate_module_if_necessary(module);
+                } else if custom_derive_crate {
+                    // Define an empty module
+                    let def = Def::Mod(self.definitions.local_def_id(item.id));
+                    let module = ModuleS::new(Some(parent), ModuleKind::Def(def, name));
+                    let module = self.arenas.alloc_module(module);
+                    self.define(parent, name, TypeNS, (module, sp, vis));
                 }
             }
 
@@ -504,17 +522,6 @@ impl<'b> Resolver<'b> {
 
         false
     }
-
-    fn insert_custom_derive(&mut self, name: &str, ext: Rc<MultiItemModifier>, sp: Span) {
-        if !self.session.features.borrow().proc_macro {
-            let sess = &self.session.parse_sess;
-            let msg = "loading custom derive macro crates is experimentally supported";
-            emit_feature_err(sess, "proc_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> {

src/librustc_resolve/lib.rs

@@ -53,7 +53,6 @@ use rustc::ty;
 use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap};
 use rustc::util::nodemap::{NodeMap, NodeSet, FnvHashMap, FnvHashSet};
 
-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};
@@ -1080,7 +1079,6 @@ pub struct Resolver<'a> {
     new_import_semantics: bool, // true if `#![feature(item_like_imports)]`
 
     pub exported_macros: Vec<ast::MacroDef>,
-    pub derive_modes: FnvHashMap<Name, Rc<MultiItemModifier>>,
     crate_loader: &'a mut CrateLoader,
     macro_names: FnvHashSet<Name>,
     builtin_macros: FnvHashMap<Name, Rc<SyntaxExtension>>,
@@ -1271,7 +1269,6 @@ impl<'a> Resolver<'a> {
             new_import_semantics: session.features.borrow().item_like_imports,
 
             exported_macros: Vec::new(),
-            derive_modes: FnvHashMap(),
             crate_loader: crate_loader,
             macro_names: FnvHashSet(),
             builtin_macros: FnvHashMap(),

src/librustc_resolve/macros.rs

@@ -17,9 +17,9 @@ use std::cell::Cell;
 use std::rc::Rc;
 use syntax::ast;
 use syntax::errors::DiagnosticBuilder;
-use syntax::ext::base::{self, Determinacy, MultiModifier, MultiDecorator, MultiItemModifier};
+use syntax::ext::base::{self, Determinacy, MultiModifier, MultiDecorator};
 use syntax::ext::base::{NormalTT, SyntaxExtension};
-use syntax::ext::expand::{Expansion, Invocation, InvocationKind};
+use syntax::ext::expand::Expansion;
 use syntax::ext::hygiene::Mark;
 use syntax::ext::tt::macro_rules;
 use syntax::parse::token::intern;
@@ -162,30 +162,22 @@ impl<'a> base::Resolver for Resolver<'a> {
         None
     }
 
-    fn resolve_invoc(&mut self, scope: Mark, invoc: &Invocation, force: bool)
+    fn resolve_macro(&mut self, scope: Mark, path: &ast::Path, force: bool)
                      -> Result<Rc<SyntaxExtension>, Determinacy> {
-        let (name, span) = match invoc.kind {
-            InvocationKind::Bang { ref mac, .. } => {
-                let path = &mac.node.path;
-                if path.segments.len() > 1 || path.global ||
-                   !path.segments[0].parameters.is_empty() {
-                    self.session.span_err(path.span,
-                                          "expected macro name without module separators");
-                    return Err(Determinacy::Determined);
-                }
-                (path.segments[0].identifier.name, path.span)
-            }
-            InvocationKind::Attr { ref attr, .. } => (intern(&*attr.name()), attr.span),
-        };
+        if path.segments.len() > 1 || path.global || !path.segments[0].parameters.is_empty() {
+            self.session.span_err(path.span, "expected macro name without module separators");
+            return Err(Determinacy::Determined);
+        }
+        let name = path.segments[0].identifier.name;
 
         let invocation = self.invocations[&scope];
         if let LegacyScope::Expansion(parent) = invocation.legacy_scope.get() {
             invocation.legacy_scope.set(LegacyScope::simplify_expansion(parent));
         }
         self.resolve_macro_name(invocation.legacy_scope.get(), name, true).ok_or_else(|| {
             if force {
-                let mut err =
-                    self.session.struct_span_err(span, &format!("macro undefined: '{}!'", name));
+                let msg = format!("macro undefined: '{}!'", name);
+                let mut err = self.session.struct_span_err(path.span, &msg);
                 self.suggest_macro_name(&name.as_str(), &mut err);
                 err.emit();
                 Determinacy::Determined
@@ -194,10 +186,6 @@ impl<'a> base::Resolver for Resolver<'a> {
             }
         })
     }
-
-    fn resolve_derive_mode(&mut self, ident: ast::Ident) -> Option<Rc<MultiItemModifier>> {
-        self.derive_modes.get(&ident.name).cloned()
-    }
 }
 
 impl<'a> Resolver<'a> {

src/libsyntax/ext/base.rs

@@ -15,7 +15,7 @@ use attr::HasAttrs;
 use codemap::{self, CodeMap, ExpnInfo, Spanned, respan};
 use syntax_pos::{Span, ExpnId, NO_EXPANSION};
 use errors::DiagnosticBuilder;
-use ext::expand::{self, Invocation, Expansion};
+use ext::expand::{self, Expansion};
 use ext::hygiene::Mark;
 use fold::{self, Folder};
 use parse::{self, parser};
@@ -508,6 +508,8 @@ pub enum SyntaxExtension {
     /// the block.
     ///
     IdentTT(Box<IdentMacroExpander>, Option<Span>, bool),
+
+    CustomDerive(Box<MultiItemModifier>),
 }
 
 pub type NamedSyntaxExtension = (Name, SyntaxExtension);
@@ -522,9 +524,8 @@ pub trait Resolver {
     fn add_expansions_at_stmt(&mut self, id: ast::NodeId, macros: Vec<Mark>);
 
     fn find_attr_invoc(&mut self, attrs: &mut Vec<Attribute>) -> Option<Attribute>;
-    fn resolve_invoc(&mut self, scope: Mark, invoc: &Invocation, force: bool)
+    fn resolve_macro(&mut self, scope: Mark, path: &ast::Path, force: bool)
                      -> Result<Rc<SyntaxExtension>, Determinacy>;
-    fn resolve_derive_mode(&mut self, ident: ast::Ident) -> Option<Rc<MultiItemModifier>>;
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -545,8 +546,7 @@ impl Resolver for DummyResolver {
     fn add_expansions_at_stmt(&mut self, _id: ast::NodeId, _macros: Vec<Mark>) {}
 
     fn find_attr_invoc(&mut self, _attrs: &mut Vec<Attribute>) -> Option<Attribute> { None }
-    fn resolve_derive_mode(&mut self, _ident: ast::Ident) -> Option<Rc<MultiItemModifier>> { None }
-    fn resolve_invoc(&mut self, _scope: Mark, _invoc: &Invocation, _force: bool)
+    fn resolve_macro(&mut self, _scope: Mark, _path: &ast::Path, _force: bool)
                      -> Result<Rc<SyntaxExtension>, Determinacy> {
         Err(Determinacy::Determined)
     }

src/libsyntax/ext/expand.rs

@@ -240,7 +240,17 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
 
             let scope =
                 if self.monotonic { invoc.expansion_data.mark } else { orig_expansion_data.mark };
-            let ext = match self.cx.resolver.resolve_invoc(scope, &invoc, force) {
+            let resolution = match invoc.kind {
+                InvocationKind::Bang { ref mac, .. } => {
+                    self.cx.resolver.resolve_macro(scope, &mac.node.path, force)
+                }
+                InvocationKind::Attr { ref attr, .. } => {
+                    let ident = ast::Ident::with_empty_ctxt(intern(&*attr.name()));
+                    let path = ast::Path::from_ident(attr.span, ident);
+                    self.cx.resolver.resolve_macro(scope, &path, force)
+                }
+            };
+            let ext = match resolution {
                 Ok(ext) => Some(ext),
                 Err(Determinacy::Determined) => None,
                 Err(Determinacy::Undetermined) => {
@@ -354,7 +364,15 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                 let tok_result = mac.expand(self.cx, attr.span, attr_toks, item_toks);
                 self.parse_expansion(tok_result, kind, name, attr.span)
             }
-            _ => unreachable!(),
+            SyntaxExtension::CustomDerive(_) => {
+                self.cx.span_err(attr.span, &format!("`{}` is a derive mode", name));
+                kind.dummy(attr.span)
+            }
+            _ => {
+                let msg = &format!("macro `{}` may not be used in attributes", name);
+                self.cx.span_err(attr.span, &msg);
+                kind.dummy(attr.span)
+            }
         }
     }
 
@@ -429,6 +447,11 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                 return kind.dummy(span);
             }
 
+            SyntaxExtension::CustomDerive(..) => {
+                self.cx.span_err(path.span, &format!("`{}` is a derive mode", extname));
+                return kind.dummy(span);
+            }
+
             SyntaxExtension::ProcMacro(ref expandfun) => {
                 if ident.name != keywords::Invalid.name() {
                     let msg =

src/libsyntax_ext/deriving/mod.rs

@@ -12,7 +12,7 @@
 
 use syntax::ast::{self, MetaItem};
 use syntax::attr::HasAttrs;
-use syntax::ext::base::{Annotatable, ExtCtxt};
+use syntax::ext::base::{Annotatable, ExtCtxt, SyntaxExtension};
 use syntax::ext::build::AstBuilder;
 use syntax::feature_gate;
 use syntax::codemap;
@@ -158,10 +158,14 @@ pub fn expand_derive(cx: &mut ExtCtxt,
         let tword = titem.word().unwrap();
         let tname = tword.name();
 
-        let derive_mode = ast::Ident::with_empty_ctxt(intern(&tname));
-        let derive_mode = cx.resolver.resolve_derive_mode(derive_mode);
-        if is_builtin_trait(&tname) || derive_mode.is_some() {
-            return true
+        if is_builtin_trait(&tname) || {
+            let derive_mode =
+                ast::Path::from_ident(titem.span, ast::Ident::with_empty_ctxt(intern(&tname)));
+            cx.resolver.resolve_macro(cx.current_expansion.mark, &derive_mode, false).map(|ext| {
+                if let SyntaxExtension::CustomDerive(_) = *ext { true } else { false }
+            }).unwrap_or(false)
+        } {
+            return true;
         }
 
         if !cx.ecfg.enable_custom_derive() {
@@ -216,7 +220,9 @@ pub fn expand_derive(cx: &mut ExtCtxt,
                                  .next();
     if let Some((i, titem)) = macros_11_derive {
         let tname = ast::Ident::with_empty_ctxt(intern(&titem.name().unwrap()));
-        let ext = cx.resolver.resolve_derive_mode(tname).unwrap();
+        let path = ast::Path::from_ident(titem.span, tname);
+        let ext = cx.resolver.resolve_macro(cx.current_expansion.mark, &path, false).unwrap();
+
         traits.remove(i);
         if traits.len() > 0 {
             item = item.map(|mut i| {
@@ -232,7 +238,11 @@ pub fn expand_derive(cx: &mut ExtCtxt,
                                  intern_and_get_ident("derive"),
                                  vec![titem]);
         let item = Annotatable::Item(item);
-        return ext.expand(cx, mitem.span, &mitem, item)
+        if let SyntaxExtension::CustomDerive(ref ext) = *ext {
+            return ext.expand(cx, mitem.span, &mitem, item);
+        } else {
+            unreachable!()
+        }
     }
 
     // Ok, at this point we know that there are no old-style `#[derive_Foo]` nor

src/test/compile-fail-fulldeps/proc-macro/shadow.rs

@@ -9,13 +9,12 @@
 // except according to those terms.
 
 // aux-build:derive-a.rs
-// aux-build:derive-a-2.rs
 
 #![feature(proc_macro)]
 
 #[macro_use]
 extern crate derive_a;
 #[macro_use]
-extern crate derive_a_2; //~ ERROR: cannot shadow existing derive mode `A`
+extern crate derive_a; //~ ERROR `derive_a` has already been defined
 
 fn main() {}