Auto merge of #36767 - jseyfried:enforce_rfc_1560_shadowing, r=nrc

Enforce the shadowing restrictions from RFC 1560 for today's macros

This PR enforces a weakened version of the shadowing restrictions from RFC 1560. More specifically,
 - If a macro expansion contains a `macro_rules!` macro definition that is used outside of the expansion, the defined macro may not shadow an existing macro.
 - If a macro expansion contains a `#[macro_use] extern crate` macro import that is used outside of the expansion, the imported macro may not shadow an existing macro.

This is a [breaking-change]. For example,
```rust
macro_rules! m { () => {} }
macro_rules! n { () => {
    macro_rules! m { () => {} } //< This shadows an existing macro.
    m!(); //< This is inside the expansion that generated `m`'s definition, so it is OK.
} }
n!();
m!(); //< This use of `m` is outside the expansion, so it causes the shadowing to be an error.
```

r? @nrc

12 files changed, 210 insertions, 79 deletions

diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs
index 49d889ff08d..421843a7f11 100644
--- a/src/librustc/hir/map/def_collector.rs
+++ b/src/librustc/hir/map/def_collector.rs
@@ -17,6 +17,7 @@ use hir::def_id::{CRATE_DEF_INDEX, DefId, DefIndex};
 use middle::cstore::InlinedItem;
 
 use syntax::ast::*;
+use syntax::ext::hygiene::Mark;
 use syntax::visit;
 use syntax::parse::token::{self, keywords};
 
@@ -31,7 +32,7 @@ pub struct DefCollector<'a> {
 }
 
 pub struct MacroInvocationData {
-    pub id: NodeId,
+    pub mark: Mark,
     pub def_index: DefIndex,
     pub const_integer: bool,
 }
@@ -126,7 +127,7 @@ impl<'a> DefCollector<'a> {
     fn visit_macro_invoc(&mut self, id: NodeId, const_integer: bool) {
         if let Some(ref mut visit) = self.visit_macro_invoc {
             visit(MacroInvocationData {
-                id: id,
+                mark: Mark::from_placeholder_id(id),
                 const_integer: const_integer,
                 def_index: self.parent_def.unwrap(),
             })
diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs
index 4e83cc328f8..be91b86dbcc 100644
--- a/src/librustc/middle/cstore.rs
+++ b/src/librustc/middle/cstore.rs
@@ -423,7 +423,12 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
     fn metadata_encoding_version(&self) -> &[u8] { bug!("metadata_encoding_version") }
 }
 
-pub enum LoadedMacro {
+pub struct LoadedMacro {
+    pub import_site: Span,
+    pub kind: LoadedMacroKind,
+}
+
+pub enum LoadedMacroKind {
     Def(ast::MacroDef),
     CustomDerive(String, Rc<MultiItemModifier>),
 }
diff --git a/src/librustc_metadata/macro_import.rs b/src/librustc_metadata/macro_import.rs
index 2ff7a6c41b5..385f582e2d3 100644
--- a/src/librustc_metadata/macro_import.rs
+++ b/src/librustc_metadata/macro_import.rs
@@ -18,7 +18,7 @@ use std::mem;
 use creader::{CrateLoader, Macros};
 
 use rustc::hir::def_id::DefIndex;
-use rustc::middle::cstore::LoadedMacro;
+use rustc::middle::cstore::{LoadedMacro, LoadedMacroKind};
 use rustc::session::Session;
 use rustc::util::nodemap::FnvHashMap;
 use rustc_back::dynamic_lib::DynamicLibrary;
@@ -28,7 +28,7 @@ use syntax::ast;
 use syntax::attr;
 use syntax::parse::token;
 use syntax_ext::deriving::custom::CustomDerive;
-use syntax_pos::Span;
+use syntax_pos::{Span, DUMMY_SP};
 
 pub fn call_bad_macro_reexport(a: &Session, b: Span) {
     span_err!(a, b, E0467, "bad macro reexport");
@@ -36,6 +36,11 @@ pub fn call_bad_macro_reexport(a: &Session, b: Span) {
 
 pub type MacroSelection = FnvHashMap<token::InternedString, Span>;
 
+enum ImportSelection {
+    All(Span),
+    Some(MacroSelection),
+}
+
 pub fn load_macros(loader: &mut CrateLoader, extern_crate: &ast::Item, allows_macros: bool)
                    -> Vec<LoadedMacro> {
     loader.load_crate(extern_crate, allows_macros)
@@ -46,7 +51,7 @@ impl<'a> CrateLoader<'a> {
                   extern_crate: &ast::Item,
                   allows_macros: bool) -> Vec<LoadedMacro> {
         // Parse the attributes relating to macros.
-        let mut import = Some(FnvHashMap());  // None => load all
+        let mut import = ImportSelection::Some(FnvHashMap());
         let mut reexport = FnvHashMap();
 
         for attr in &extern_crate.attrs {
@@ -55,11 +60,9 @@ impl<'a> CrateLoader<'a> {
                 "macro_use" => {
                     let names = attr.meta_item_list();
                     if names.is_none() {
-                        // no names => load all
-                        import = None;
-                    }
-                    if let (Some(sel), Some(names)) = (import.as_mut(), names) {
-                        for attr in names {
+                        import = ImportSelection::All(attr.span);
+                    } else if let ImportSelection::Some(ref mut sel) = import {
+                        for attr in names.unwrap() {
                             if let Some(word) = attr.word() {
                                 sel.insert(word.name().clone(), attr.span());
                             } else {
@@ -98,10 +101,10 @@ impl<'a> CrateLoader<'a> {
     fn load_macros<'b>(&mut self,
                        vi: &ast::Item,
                        allows_macros: bool,
-                       import: Option<MacroSelection>,
+                       import: ImportSelection,
                        reexport: MacroSelection)
                        -> Vec<LoadedMacro> {
-        if let Some(sel) = import.as_ref() {
+        if let ImportSelection::Some(ref sel) = import {
             if sel.is_empty() && reexport.is_empty() {
                 return Vec::new();
             }
@@ -120,15 +123,19 @@ impl<'a> CrateLoader<'a> {
         for mut def in macros.macro_rules.drain(..) {
             let name = def.ident.name.as_str();
 
-            def.use_locally = match import.as_ref() {
-                None => true,
-                Some(sel) => sel.contains_key(&name),
+            let import_site = match import {
+                ImportSelection::All(span) => Some(span),
+                ImportSelection::Some(ref sel) => sel.get(&name).cloned()
             };
+            def.use_locally = import_site.is_some();
             def.export = reexport.contains_key(&name);
             def.allow_internal_unstable = attr::contains_name(&def.attrs,
                                                               "allow_internal_unstable");
             debug!("load_macros: loaded: {:?}", def);
-            ret.push(LoadedMacro::Def(def));
+            ret.push(LoadedMacro {
+                kind: LoadedMacroKind::Def(def),
+                import_site: import_site.unwrap_or(DUMMY_SP),
+            });
             seen.insert(name);
         }
 
@@ -137,7 +144,7 @@ impl<'a> CrateLoader<'a> {
             // exported macros, enforced elsewhere
             assert_eq!(ret.len(), 0);
 
-            if import.is_some() {
+            if let ImportSelection::Some(..) = import {
                 self.sess.span_err(vi.span, "`rustc-macro` crates cannot be \
                                              selectively imported from, must \
                                              use `#[macro_use]`");
@@ -151,10 +158,10 @@ impl<'a> CrateLoader<'a> {
             self.load_derive_macros(vi.span, &macros, index, &mut ret);
         }
 
-        if let Some(sel) = import.as_ref() {
+        if let ImportSelection::Some(sel) = import {
             for (name, span) in sel {
                 if !seen.contains(&name) {
-                    span_err!(self.sess, *span, E0469,
+                    span_err!(self.sess, span, E0469,
                               "imported macro not found");
                 }
             }
@@ -199,18 +206,21 @@ impl<'a> CrateLoader<'a> {
             mem::transmute::<*mut u8, fn(&mut Registry)>(sym)
         };
 
-        struct MyRegistrar<'a>(&'a mut Vec<LoadedMacro>);
+        struct MyRegistrar<'a>(&'a mut Vec<LoadedMacro>, Span);
 
         impl<'a> Registry for MyRegistrar<'a> {
             fn register_custom_derive(&mut self,
                                       trait_name: &str,
                                       expand: fn(TokenStream) -> TokenStream) {
                 let derive = Rc::new(CustomDerive::new(expand));
-                self.0.push(LoadedMacro::CustomDerive(trait_name.to_string(), derive));
+                self.0.push(LoadedMacro {
+                    kind: LoadedMacroKind::CustomDerive(trait_name.to_string(), derive),
+                    import_site: self.1,
+                });
             }
         }
 
-        registrar(&mut MyRegistrar(ret));
+        registrar(&mut MyRegistrar(ret, span));
 
         // Intentionally leak the dynamic library. We can't ever unload it
         // since the library can make things that will live arbitrarily long.
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 55bf5dcf1cf..2bf517600b7 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -13,6 +13,7 @@
 //! Here we build the "reduced graph": the graph of the module tree without
 //! any imports resolved.
 
+use macros;
 use resolve_imports::ImportDirectiveSubclass::{self, GlobImport};
 use {Module, ModuleS, ModuleKind};
 use Namespace::{self, TypeNS, ValueNS};
@@ -20,7 +21,7 @@ use {NameBinding, NameBindingKind, ToNameBinding};
 use Resolver;
 use {resolve_error, resolve_struct_error, ResolutionError};
 
-use rustc::middle::cstore::LoadedMacro;
+use rustc::middle::cstore::LoadedMacroKind;
 use rustc::hir::def::*;
 use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
 use rustc::hir::map::DefPathData;
@@ -39,6 +40,7 @@ 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::ext::tt::macro_rules;
 use syntax::parse::token::keywords;
 use syntax::visit::{self, Visitor};
 
@@ -77,7 +79,7 @@ impl<'b> Resolver<'b> {
     }
 
     /// Constructs the reduced graph for one item.
-    fn build_reduced_graph_for_item(&mut self, item: &Item) {
+    fn build_reduced_graph_for_item(&mut self, item: &Item, expansion: Mark) {
         let parent = self.current_module;
         let name = item.ident.name;
         let sp = item.span;
@@ -188,10 +190,29 @@ 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();
-                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) => {
+                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 = macro_rules::compile(&self.session.parse_sess, &def);
+                                let shadowing =
+                                    self.resolve_macro_name(Mark::root(), name, false).is_some();
+                                self.expansion_data[&Mark::root()].module.macros.borrow_mut()
+                                    .insert(name, macros::NameBinding {
+                                        ext: Rc::new(ext),
+                                        expansion: expansion,
+                                        shadowing: shadowing,
+                                        span: loaded_macro.import_site,
+                                    });
+                                self.macro_names.insert(name);
+                            }
+                            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);
                         }
                     }
@@ -527,11 +548,12 @@ impl<'b> Resolver<'b> {
 
 pub struct BuildReducedGraphVisitor<'a, 'b: 'a> {
     pub resolver: &'a mut Resolver<'b>,
+    pub expansion: Mark,
 }
 
 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.expansion_data.get_mut(&Mark::from_placeholder_id(id)).unwrap().module =
             self.resolver.current_module;
     }
 }
@@ -562,7 +584,7 @@ impl<'a, 'b> Visitor for BuildReducedGraphVisitor<'a, 'b> {
         }
 
         let parent = self.resolver.current_module;
-        self.resolver.build_reduced_graph_for_item(item);
+        self.resolver.build_reduced_graph_for_item(item, self.expansion);
         visit::walk_item(self, item);
         self.resolver.current_module = parent;
     }
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index d1bfb7d786e..90a9e596061 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -57,7 +57,6 @@ 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;
 
@@ -793,7 +792,7 @@ pub struct ModuleS<'a> {
     // `populate_module_if_necessary` call.
     populated: Cell<bool>,
 
-    macros: RefCell<FnvHashMap<Name, Rc<SyntaxExtension>>>,
+    macros: RefCell<FnvHashMap<Name, macros::NameBinding>>,
     macros_escape: bool,
 }
 
@@ -1074,6 +1073,7 @@ pub struct Resolver<'a> {
 
     privacy_errors: Vec<PrivacyError<'a>>,
     ambiguity_errors: Vec<AmbiguityError<'a>>,
+    macro_shadowing_errors: FnvHashSet<Span>,
 
     arenas: &'a ResolverArenas<'a>,
     dummy_binding: &'a NameBinding<'a>,
@@ -1085,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<'a>>,
+    expansion_data: FnvHashMap<Mark, macros::ExpansionData<'a>>,
 }
 
 pub struct ResolverArenas<'a> {
@@ -1203,7 +1203,7 @@ impl<'a> Resolver<'a> {
         DefCollector::new(&mut definitions).collect_root();
 
         let mut expansion_data = FnvHashMap();
-        expansion_data.insert(0, macros::ExpansionData::root(graph_root)); // Crate root expansion
+        expansion_data.insert(Mark::root(), macros::ExpansionData::root(graph_root));
 
         Resolver {
             session: session,
@@ -1249,6 +1249,7 @@ impl<'a> Resolver<'a> {
 
             privacy_errors: Vec::new(),
             ambiguity_errors: Vec::new(),
+            macro_shadowing_errors: FnvHashSet(),
 
             arenas: arenas,
             dummy_binding: arenas.alloc_name_binding(NameBinding {
diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs
index 947aeb4439e..3f6c69278be 100644
--- a/src/librustc_resolve/macros.rs
+++ b/src/librustc_resolve/macros.rs
@@ -18,13 +18,23 @@ use syntax::errors::DiagnosticBuilder;
 use syntax::ext::base::{self, MultiModifier, MultiDecorator, MultiItemModifier};
 use syntax::ext::base::{NormalTT, SyntaxExtension};
 use syntax::ext::expand::{Expansion, Invocation, InvocationKind};
-use syntax::ext::hygiene::Mark;
+use syntax::ext::hygiene::{Mark, SyntaxContext};
 use syntax::ext::tt::macro_rules;
 use syntax::parse::token::intern;
 use syntax::util::lev_distance::find_best_match_for_name;
+use syntax_pos::{Span, DUMMY_SP};
+
+// FIXME(jseyfried) Merge with `::NameBinding`.
+pub struct NameBinding {
+    pub ext: Rc<SyntaxExtension>,
+    pub expansion: Mark,
+    pub shadowing: bool,
+    pub span: Span,
+}
 
 #[derive(Clone)]
 pub struct ExpansionData<'a> {
+    backtrace: SyntaxContext,
     pub module: Module<'a>,
     def_index: DefIndex,
     // True if this expansion is in a `const_integer` position, for example `[u32; m!()]`.
@@ -35,6 +45,7 @@ pub struct ExpansionData<'a> {
 impl<'a> ExpansionData<'a> {
     pub fn root(graph_root: Module<'a>) -> Self {
         ExpansionData {
+            backtrace: SyntaxContext::empty(),
             module: graph_root,
             def_index: CRATE_DEF_INDEX,
             const_integer: false,
@@ -50,7 +61,8 @@ impl<'a> base::Resolver for Resolver<'a> {
     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 {
+        self.expansion_data.insert(mark, ExpansionData {
+            backtrace: SyntaxContext::empty(),
             module: module,
             def_index: module.def_id().unwrap().index,
             const_integer: false,
@@ -60,8 +72,8 @@ impl<'a> base::Resolver for Resolver<'a> {
 
     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 });
+        self.current_module = self.expansion_data[&mark].module;
+        expansion.visit_with(&mut BuildReducedGraphVisitor { resolver: self, expansion: mark });
     }
 
     fn add_macro(&mut self, scope: Mark, mut def: ast::MacroDef) {
@@ -69,8 +81,18 @@ impl<'a> base::Resolver for Resolver<'a> {
             self.session.span_err(def.span, "user-defined macros may not be named `macro_rules`");
         }
         if def.use_locally {
-            let ext = macro_rules::compile(&self.session.parse_sess, &def);
-            self.add_ext(scope, def.ident, Rc::new(ext));
+            let ExpansionData { mut module, backtrace, .. } = self.expansion_data[&scope];
+            while module.macros_escape {
+                module = module.parent.unwrap();
+            }
+            let binding = NameBinding {
+                ext: Rc::new(macro_rules::compile(&self.session.parse_sess, &def)),
+                expansion: backtrace.data().prev_ctxt.data().outer_mark,
+                shadowing: self.resolve_macro_name(scope, def.ident.name, false).is_some(),
+                span: def.span,
+            };
+            module.macros.borrow_mut().insert(def.ident.name, binding);
+            self.macro_names.insert(def.ident.name);
         }
         if def.export {
             def.id = self.next_node_id();
@@ -78,16 +100,16 @@ impl<'a> base::Resolver for Resolver<'a> {
         }
     }
 
-    fn add_ext(&mut self, scope: Mark, ident: ast::Ident, ext: Rc<SyntaxExtension>) {
+    fn add_ext(&mut self, ident: ast::Ident, ext: Rc<SyntaxExtension>) {
         if let NormalTT(..) = *ext {
             self.macro_names.insert(ident.name);
         }
-
-        let mut module = self.expansion_data[&scope.as_u32()].module;
-        while module.macros_escape {
-            module = module.parent.unwrap();
-        }
-        module.macros.borrow_mut().insert(ident.name, ext);
+        self.graph_root.macros.borrow_mut().insert(ident.name, NameBinding {
+            ext: ext,
+            expansion: Mark::root(),
+            shadowing: false,
+            span: DUMMY_SP,
+        });
     }
 
     fn add_expansions_at_stmt(&mut self, id: ast::NodeId, macros: Vec<Mark>) {
@@ -97,8 +119,8 @@ impl<'a> base::Resolver for Resolver<'a> {
     fn find_attr_invoc(&mut self, attrs: &mut Vec<ast::Attribute>) -> Option<ast::Attribute> {
         for i in 0..attrs.len() {
             let name = intern(&attrs[i].name());
-            match self.expansion_data[&0].module.macros.borrow().get(&name) {
-                Some(ext) => match **ext {
+            match self.expansion_data[&Mark::root()].module.macros.borrow().get(&name) {
+                Some(binding) => match *binding.ext {
                     MultiModifier(..) | MultiDecorator(..) | SyntaxExtension::AttrProcMacro(..) => {
                         return Some(attrs.remove(i))
                     }
@@ -125,30 +147,53 @@ 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;
+        self.resolve_macro_name(scope, name, true).or_else(|| {
+            let mut err =
+                self.session.struct_span_err(span, &format!("macro undefined: '{}!'", name));
+            self.suggest_macro_name(&name.as_str(), &mut err);
+            err.emit();
+            None
+        })
+    }
+
+    fn resolve_derive_mode(&mut self, ident: ast::Ident) -> Option<Rc<MultiItemModifier>> {
+        self.derive_modes.get(&ident.name).cloned()
+    }
+}
+
+impl<'a> Resolver<'a> {
+    pub fn resolve_macro_name(&mut self, scope: Mark, name: ast::Name, record_used: bool)
+                              -> Option<Rc<SyntaxExtension>> {
+        let ExpansionData { mut module, backtrace, .. } = self.expansion_data[&scope];
         loop {
-            if let Some(ext) = module.macros.borrow().get(&name) {
-                return Some(ext.clone());
+            if let Some(binding) = module.macros.borrow().get(&name) {
+                let mut backtrace = backtrace.data();
+                while binding.expansion != backtrace.outer_mark {
+                    if backtrace.outer_mark != Mark::root() {
+                        backtrace = backtrace.prev_ctxt.data();
+                        continue
+                    }
+
+                    if record_used && binding.shadowing &&
+                       self.macro_shadowing_errors.insert(binding.span) {
+                        let msg = format!("`{}` is already in scope", name);
+                        self.session.struct_span_err(binding.span, &msg)
+                            .note("macro-expanded `macro_rules!`s and `#[macro_use]`s \
+                                   may not shadow existing macros (see RFC 1560)")
+                            .emit();
+                    }
+                    break
+                }
+                return Some(binding.ext.clone());
             }
             match module.parent {
                 Some(parent) => module = parent,
                 None => break,
             }
         }
-
-        let mut err =
-            self.session.struct_span_err(span, &format!("macro undefined: '{}!'", name));
-        self.suggest_macro_name(&name.as_str(), &mut err);
-        err.emit();
         None
     }
 
-    fn resolve_derive_mode(&mut self, ident: ast::Ident) -> Option<Rc<MultiItemModifier>> {
-        self.derive_modes.get(&ident.name).cloned()
-    }
-}
-
-impl<'a> Resolver<'a> {
     fn suggest_macro_name(&mut self, name: &str, err: &mut DiagnosticBuilder<'a>) {
         if let Some(suggestion) = find_best_match_for_name(self.macro_names.iter(), name, None) {
             if suggestion != name {
@@ -161,9 +206,10 @@ impl<'a> Resolver<'a> {
 
     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 ExpansionData { backtrace, def_index, const_integer, module } = expansion_data[&mark];
         let visit_macro_invoc = &mut |invoc: map::MacroInvocationData| {
-            expansion_data.entry(invoc.id.as_u32()).or_insert(ExpansionData {
+            expansion_data.entry(invoc.mark).or_insert(ExpansionData {
+                backtrace: backtrace.apply_mark(invoc.mark),
                 def_index: invoc.def_index,
                 const_integer: invoc.const_integer,
                 module: module,
diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs
index 4f64b3f2e1d..b56018e1e9d 100644
--- a/src/libsyntax/ext/base.rs
+++ b/src/libsyntax/ext/base.rs
@@ -519,7 +519,7 @@ pub trait Resolver {
 
     fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion);
     fn add_macro(&mut self, scope: Mark, def: ast::MacroDef);
-    fn add_ext(&mut self, scope: Mark, ident: ast::Ident, ext: Rc<SyntaxExtension>);
+    fn add_ext(&mut self, ident: ast::Ident, ext: Rc<SyntaxExtension>);
     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>;
@@ -535,7 +535,7 @@ impl Resolver for DummyResolver {
 
     fn visit_expansion(&mut self, _invoc: Mark, _expansion: &Expansion) {}
     fn add_macro(&mut self, _scope: Mark, _def: ast::MacroDef) {}
-    fn add_ext(&mut self, _scope: Mark, _ident: ast::Ident, _ext: Rc<SyntaxExtension>) {}
+    fn add_ext(&mut self, _ident: ast::Ident, _ext: Rc<SyntaxExtension>) {}
     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 }
@@ -749,7 +749,7 @@ impl<'a> ExtCtxt<'a> {
 
         for (name, extension) in user_exts {
             let ident = ast::Ident::with_empty_ctxt(name);
-            self.resolver.add_ext(Mark::root(), ident, Rc::new(extension));
+            self.resolver.add_ext(ident, Rc::new(extension));
         }
 
         let mut module = ModuleData {
diff --git a/src/libsyntax/ext/hygiene.rs b/src/libsyntax/ext/hygiene.rs
index 34126fac4ac..0fd72277cca 100644
--- a/src/libsyntax/ext/hygiene.rs
+++ b/src/libsyntax/ext/hygiene.rs
@@ -15,6 +15,7 @@
 //! and definition contexts*. J. Funct. Program. 22, 2 (March 2012), 181-216.
 //! DOI=10.1017/S0956796812000093 http://dx.doi.org/10.1017/S0956796812000093
 
+use ast::NodeId;
 use std::cell::RefCell;
 use std::collections::HashMap;
 use std::fmt;
@@ -46,6 +47,10 @@ impl Mark {
         Mark(0)
     }
 
+    pub fn from_placeholder_id(id: NodeId) -> Self {
+        Mark(id.as_u32())
+    }
+
     pub fn as_u32(&self) -> u32 {
         self.0
     }
diff --git a/src/libsyntax_ext/lib.rs b/src/libsyntax_ext/lib.rs
index bd9f1cf0d77..6e4f3dde4bd 100644
--- a/src/libsyntax_ext/lib.rs
+++ b/src/libsyntax_ext/lib.rs
@@ -51,13 +51,12 @@ pub mod deriving;
 use std::rc::Rc;
 use syntax::ast;
 use syntax::ext::base::{MacroExpanderFn, NormalTT, IdentTT, MultiModifier};
-use syntax::ext::hygiene::Mark;
 use syntax::ext::tt::macro_rules::MacroRulesExpander;
 use syntax::parse::token::intern;
 
 pub fn register_builtins(resolver: &mut syntax::ext::base::Resolver, enable_quotes: bool) {
     let mut register = |name, ext| {
-        resolver.add_ext(Mark::root(), ast::Ident::with_empty_ctxt(intern(name)), Rc::new(ext));
+        resolver.add_ext(ast::Ident::with_empty_ctxt(intern(name)), Rc::new(ext));
     };
 
     register("macro_rules", IdentTT(Box::new(MacroRulesExpander), None, false));
diff --git a/src/test/compile-fail/issue-32922.rs b/src/test/compile-fail/issue-32922.rs
index 491c087c101..317a47156c1 100644
--- a/src/test/compile-fail/issue-32922.rs
+++ b/src/test/compile-fail/issue-32922.rs
@@ -17,7 +17,7 @@ macro_rules! foo { () => {
     let _ = bar!();
 }}
 
-macro_rules! bar { // test issue #31856
+macro_rules! m { // test issue #31856
     ($n:ident) => (
         let a = 1;
         let $n = a;
diff --git a/src/test/compile-fail/macro-shadowing.rs b/src/test/compile-fail/macro-shadowing.rs
new file mode 100644
index 00000000000..22463825ef9
--- /dev/null
+++ b/src/test/compile-fail/macro-shadowing.rs
@@ -0,0 +1,42 @@
+// 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.
+
+// aux-build:two_macros.rs
+
+macro_rules! foo { () => {} }
+macro_rules! macro_one { () => {} }
+
+macro_rules! m1 { () => {
+    macro_rules! foo { () => {} } //~ ERROR `foo` is already in scope
+    //~^ NOTE macro-expanded `macro_rules!`s and `#[macro_use]`s may not shadow existing macros
+
+    #[macro_use] //~ ERROR `macro_one` is already in scope
+    //~^ NOTE macro-expanded `macro_rules!`s and `#[macro_use]`s may not shadow existing macros
+    extern crate two_macros;
+}}
+m1!(); //~ NOTE in this expansion
+       //~| NOTE in this expansion
+       //~| NOTE in this expansion
+       //~| NOTE in this expansion
+
+fn f() { macro_one!(); }
+foo!();
+
+macro_rules! m2 { () => {
+    macro_rules! foo { () => {} }
+    #[macro_use] extern crate two_macros as __;
+
+    fn g() { macro_one!(); }
+    foo!();
+}}
+m2!();
+//^ Since `foo` and `macro_one` are not used outside this expansion, they are not shadowing errors.
+
+fn main() {}
diff --git a/src/test/run-pass/hygiene.rs b/src/test/run-pass/hygiene.rs
index d72386190ec..91648ee5798 100644
--- a/src/test/run-pass/hygiene.rs
+++ b/src/test/run-pass/hygiene.rs
@@ -22,23 +22,23 @@ fn f() {
 
 fn g() {
     let x = 0;
-    macro_rules! m { ($x:ident) => {
-        macro_rules! m2 { () => { ($x, x) } }
+    macro_rules! m { ($m1:ident, $m2:ident, $x:ident) => {
+        macro_rules! $m1 { () => { ($x, x) } }
         let x = 1;
-        macro_rules! m3 { () => { ($x, x) } }
+        macro_rules! $m2 { () => { ($x, x) } }
     } }
 
     let x = 2;
-    m!(x);
+    m!(m2, m3, x);
 
     let x = 3;
     assert_eq!(m2!(), (2, 0));
     assert_eq!(m3!(), (2, 1));
 
     let x = 4;
-    m!(x);
-    assert_eq!(m2!(), (4, 0));
-    assert_eq!(m3!(), (4, 1));
+    m!(m4, m5, x);
+    assert_eq!(m4!(), (4, 0));
+    assert_eq!(m5!(), (4, 1));
 }
 
 mod foo {