about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2018-09-11 23:28:39 +0000
committerbors <bors@rust-lang.org>2018-09-11 23:28:39 +0000
commit58063894d21c76ab5bc32bda30eee66703e0fdfb (patch)
tree834f7c003db882543320e632c7754ef2016e42eb
parent2f1547c0aa5957b42cc768c00119c6eb7b4262d3 (diff)
parentde153d61f5a4ef8681fc64fc55de2bcfe449c8c4 (diff)
downloadrust-58063894d21c76ab5bc32bda30eee66703e0fdfb.tar.gz
rust-58063894d21c76ab5bc32bda30eee66703e0fdfb.zip
Auto merge of #53913 - petrochenkov:biattr4, r=alexcrichton
resolve: Future proof resolutions for potentially built-in attributes

This is not full "pass all attributes through name resolution", but a more conservative solution.
If built-in attribute is ambiguous with any other macro in scope, then an error is reported.

What complications arise with the full solution - https://github.com/rust-lang/rust/pull/53913#issuecomment-418204136.

cc https://github.com/rust-lang/rust/pull/50911#issuecomment-411605393
cc https://github.com/rust-lang/rust/issues/52269
Closes https://github.com/rust-lang/rust/issues/53531
-rw-r--r--src/librustc_resolve/build_reduced_graph.rs10
-rw-r--r--src/librustc_resolve/lib.rs28
-rw-r--r--src/librustc_resolve/macros.rs128
-rw-r--r--src/librustc_resolve/resolve_imports.rs11
-rw-r--r--src/libsyntax/ext/base.rs9
-rw-r--r--src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs-test.rs20
-rw-r--r--src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs-test.stderr9
-rw-r--r--src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs.rs31
-rw-r--r--src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs.stderr100
-rw-r--r--src/test/ui-fulldeps/proc-macro/auxiliary/builtin-attrs.rs36
-rw-r--r--src/test/ui-fulldeps/proc-macro/reserved-macro-names.rs22
-rw-r--r--src/test/ui-fulldeps/proc-macro/reserved-macro-names.stderr20
12 files changed, 353 insertions, 71 deletions
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 1fa9979edb8..d1a05964c8f 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -39,6 +39,7 @@ use syntax::ext::base::{MacroKind, SyntaxExtension};
 use syntax::ext::base::Determinacy::Undetermined;
 use syntax::ext::hygiene::Mark;
 use syntax::ext::tt::macro_rules;
+use syntax::feature_gate::is_builtin_attr;
 use syntax::parse::token::{self, Token};
 use syntax::std_inject::injected_crate_name;
 use syntax::symbol::keywords;
@@ -1057,4 +1058,13 @@ impl<'a, 'b, 'cl> Visitor<'a> for BuildReducedGraphVisitor<'a, 'b, 'cl> {
             }
         }
     }
+
+    fn visit_attribute(&mut self, attr: &'a ast::Attribute) {
+        if !attr.is_sugared_doc && is_builtin_attr(attr) {
+            self.resolver.current_module.builtin_attrs.borrow_mut().push((
+                attr.path.segments[0].ident, self.expansion, self.current_legacy_scope
+            ));
+        }
+        visit::walk_attribute(self, attr);
+    }
 }
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index bfdf158bbbb..8c769094f52 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -81,7 +81,7 @@ use std::mem::replace;
 use rustc_data_structures::sync::Lrc;
 
 use resolve_imports::{ImportDirective, ImportDirectiveSubclass, NameResolution, ImportResolver};
-use macros::{InvocationData, LegacyBinding};
+use macros::{InvocationData, LegacyBinding, LegacyScope};
 
 // NB: This module needs to be declared first so diagnostics are
 // registered before they are used.
@@ -1010,8 +1010,9 @@ pub struct ModuleData<'a> {
     normal_ancestor_id: DefId,
 
     resolutions: RefCell<FxHashMap<(Ident, Namespace), &'a RefCell<NameResolution<'a>>>>,
-    legacy_macro_resolutions: RefCell<Vec<(Mark, Ident, MacroKind, Option<Def>)>>,
+    legacy_macro_resolutions: RefCell<Vec<(Ident, MacroKind, Mark, LegacyScope<'a>, Option<Def>)>>,
     macro_resolutions: RefCell<Vec<(Box<[Ident]>, Span)>>,
+    builtin_attrs: RefCell<Vec<(Ident, Mark, LegacyScope<'a>)>>,
 
     // Macro invocations that can expand into items in this module.
     unresolved_invocations: RefCell<FxHashSet<Mark>>,
@@ -1050,6 +1051,7 @@ impl<'a> ModuleData<'a> {
             resolutions: RefCell::new(FxHashMap()),
             legacy_macro_resolutions: RefCell::new(Vec::new()),
             macro_resolutions: RefCell::new(Vec::new()),
+            builtin_attrs: RefCell::new(Vec::new()),
             unresolved_invocations: RefCell::new(FxHashSet()),
             no_implicit_prelude: false,
             glob_importers: RefCell::new(Vec::new()),
@@ -1268,6 +1270,7 @@ impl<'a> NameBinding<'a> {
     fn macro_kind(&self) -> Option<MacroKind> {
         match self.def_ignoring_ambiguity() {
             Def::Macro(_, kind) => Some(kind),
+            Def::NonMacroAttr(..) => Some(MacroKind::Attr),
             _ => None,
         }
     }
@@ -1276,19 +1279,18 @@ impl<'a> NameBinding<'a> {
         if self.is_extern_crate() { "extern crate" } else { self.def().kind_name() }
     }
 
-    // Suppose that we resolved macro invocation with `invoc_id` to binding `binding` at some
-    // expansion round `max(invoc_id, binding)` when they both emerged from macros.
+    // Suppose that we resolved macro invocation with `invoc_parent_expansion` to binding `binding`
+    // at some expansion round `max(invoc, binding)` when they both emerged from macros.
     // Then this function returns `true` if `self` may emerge from a macro *after* that
     // in some later round and screw up our previously found resolution.
     // See more detailed explanation in
     // https://github.com/rust-lang/rust/pull/53778#issuecomment-419224049
-    fn may_appear_after(&self, invoc_id: Mark, binding: &NameBinding) -> bool {
-        // self > max(invoc_id, binding) => !(self <= invoc_id || self <= binding)
+    fn may_appear_after(&self, invoc_parent_expansion: Mark, binding: &NameBinding) -> bool {
+        // self > max(invoc, binding) => !(self <= invoc || self <= binding)
         // Expansions are partially ordered, so "may appear after" is an inversion of
         // "certainly appears before or simultaneously" and includes unordered cases.
         let self_parent_expansion = self.expansion;
         let other_parent_expansion = binding.expansion;
-        let invoc_parent_expansion = invoc_id.parent();
         let certainly_before_other_or_simultaneously =
             other_parent_expansion.is_descendant_of(self_parent_expansion);
         let certainly_before_invoc_or_simultaneously =
@@ -3493,16 +3495,16 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         path_span: Span,
         crate_lint: CrateLint,
     ) -> PathResult<'a> {
-        self.resolve_path_with_invoc_id(base_module, path, opt_ns, Mark::root(),
-                                        record_used, path_span, crate_lint)
+        self.resolve_path_with_parent_expansion(base_module, path, opt_ns, Mark::root(),
+                                                record_used, path_span, crate_lint)
     }
 
-    fn resolve_path_with_invoc_id(
+    fn resolve_path_with_parent_expansion(
         &mut self,
         base_module: Option<ModuleOrUniformRoot<'a>>,
         path: &[Ident],
         opt_ns: Option<Namespace>, // `None` indicates a module path
-        invoc_id: Mark,
+        parent_expansion: Mark,
         record_used: bool,
         path_span: Span,
         crate_lint: CrateLint,
@@ -3595,8 +3597,8 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                 self.resolve_ident_in_module(module, ident, ns, record_used, path_span)
             } else if opt_ns == Some(MacroNS) {
                 assert!(ns == TypeNS);
-                self.resolve_lexical_macro_path_segment(ident, ns, invoc_id, record_used,
-                                                        record_used, false, path_span)
+                self.resolve_lexical_macro_path_segment(ident, ns, None, parent_expansion,
+                                                        record_used, record_used, path_span)
                                                         .map(|(binding, _)| binding)
             } else {
                 let record_used_id =
diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs
index fe0cb523a15..7733ce475e3 100644
--- a/src/librustc_resolve/macros.rs
+++ b/src/librustc_resolve/macros.rs
@@ -109,6 +109,13 @@ pub struct ProcMacError {
     warn_msg: &'static str,
 }
 
+// For compatibility bang macros are skipped when resolving potentially built-in attributes.
+fn macro_kind_mismatch(name: Name, requirement: Option<MacroKind>, candidate: Option<MacroKind>)
+                       -> bool {
+    requirement == Some(MacroKind::Attr) && candidate == Some(MacroKind::Bang) &&
+    (name == "test" || name == "bench" || is_builtin_attr_name(name))
+}
+
 impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
     fn next_node_id(&mut self) -> ast::NodeId {
         self.session.next_node_id()
@@ -313,7 +320,7 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
         None
     }
 
-    fn resolve_macro_invocation(&mut self, invoc: &Invocation, scope: Mark, force: bool)
+    fn resolve_macro_invocation(&mut self, invoc: &Invocation, invoc_id: Mark, force: bool)
                                 -> Result<Option<Lrc<SyntaxExtension>>, Determinacy> {
         let (path, kind, derives_in_scope) = match invoc.kind {
             InvocationKind::Attr { attr: None, .. } =>
@@ -326,7 +333,7 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
                 (path, MacroKind::Derive, &[][..]),
         };
 
-        let (def, ext) = self.resolve_macro_to_def(path, kind, scope, derives_in_scope, force)?;
+        let (def, ext) = self.resolve_macro_to_def(path, kind, invoc_id, derives_in_scope, force)?;
 
         if let Def::Macro(def_id, _) = def {
             self.macro_defs.insert(invoc.expansion_data.mark, def_id);
@@ -341,10 +348,10 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
         Ok(Some(ext))
     }
 
-    fn resolve_macro_path(&mut self, path: &ast::Path, kind: MacroKind, scope: Mark,
+    fn resolve_macro_path(&mut self, path: &ast::Path, kind: MacroKind, invoc_id: Mark,
                           derives_in_scope: &[ast::Path], force: bool)
                           -> Result<Lrc<SyntaxExtension>, Determinacy> {
-        Ok(self.resolve_macro_to_def(path, kind, scope, derives_in_scope, force)?.1)
+        Ok(self.resolve_macro_to_def(path, kind, invoc_id, derives_in_scope, force)?.1)
     }
 
     fn check_unused_macros(&self) {
@@ -366,10 +373,10 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
 }
 
 impl<'a, 'cl> Resolver<'a, 'cl> {
-    fn resolve_macro_to_def(&mut self, path: &ast::Path, kind: MacroKind, scope: Mark,
+    fn resolve_macro_to_def(&mut self, path: &ast::Path, kind: MacroKind, invoc_id: Mark,
                             derives_in_scope: &[ast::Path], force: bool)
                             -> Result<(Def, Lrc<SyntaxExtension>), Determinacy> {
-        let def = self.resolve_macro_to_def_inner(path, kind, scope, derives_in_scope, force);
+        let def = self.resolve_macro_to_def_inner(path, kind, invoc_id, derives_in_scope, force);
 
         // Report errors and enforce feature gates for the resolved macro.
         if def != Err(Determinacy::Undetermined) {
@@ -439,8 +446,9 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         let ast::Path { ref segments, span } = *path;
         let mut path: Vec<_> = segments.iter().map(|seg| seg.ident).collect();
         let invocation = self.invocations[&invoc_id];
-        let module = invocation.module.get();
-        self.current_module = if module.is_trait() { module.parent.unwrap() } else { module };
+        let parent_expansion = invoc_id.parent();
+        let parent_legacy_scope = invocation.parent_legacy_scope.get();
+        self.current_module = invocation.module.get().nearest_item_scope();
 
         // Possibly apply the macro helper hack
         if kind == MacroKind::Bang && path.len() == 1 &&
@@ -450,8 +458,9 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         }
 
         if path.len() > 1 {
-            let def = match self.resolve_path_with_invoc_id(None, &path, Some(MacroNS), invoc_id,
-                                                            false, span, CrateLint::No) {
+            let def = match self.resolve_path_with_parent_expansion(None, &path, Some(MacroNS),
+                                                                    parent_expansion, false, span,
+                                                                    CrateLint::No) {
                 PathResult::NonModule(path_res) => match path_res.base_def() {
                     Def::Err => Err(Determinacy::Determined),
                     def @ _ => {
@@ -471,19 +480,19 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     Err(Determinacy::Determined)
                 },
             };
-            self.current_module.nearest_item_scope().macro_resolutions.borrow_mut()
+            self.current_module.macro_resolutions.borrow_mut()
                 .push((path.into_boxed_slice(), span));
             return def;
         }
 
         let legacy_resolution = self.resolve_legacy_scope(
-            path[0], invoc_id, invocation.parent_legacy_scope.get(), false, kind == MacroKind::Attr
+            path[0], Some(kind), parent_expansion, parent_legacy_scope, false
         );
         let result = if let Some(legacy_binding) = legacy_resolution {
             Ok(legacy_binding.def())
         } else {
-            match self.resolve_lexical_macro_path_segment(path[0], MacroNS, invoc_id, false, force,
-                                                          kind == MacroKind::Attr, span) {
+            match self.resolve_lexical_macro_path_segment(path[0], MacroNS, Some(kind),
+                                                          parent_expansion, false, force, span) {
                 Ok((binding, _)) => Ok(binding.def_ignoring_ambiguity()),
                 Err(Determinacy::Undetermined) => return Err(Determinacy::Undetermined),
                 Err(Determinacy::Determined) => {
@@ -493,8 +502,8 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             }
         };
 
-        self.current_module.nearest_item_scope().legacy_macro_resolutions.borrow_mut()
-            .push((invoc_id, path[0], kind, result.ok()));
+        self.current_module.legacy_macro_resolutions.borrow_mut()
+            .push((path[0], kind, parent_expansion, parent_legacy_scope, result.ok()));
 
         if let Ok(Def::NonMacroAttr(NonMacroAttrKind::Custom)) = result {} else {
             return result;
@@ -541,10 +550,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         &mut self,
         mut ident: Ident,
         ns: Namespace,
-        invoc_id: Mark,
+        kind: Option<MacroKind>,
+        parent_expansion: Mark,
         record_used: bool,
         force: bool,
-        is_attr: bool,
         path_span: Span,
     ) -> Result<(&'a NameBinding<'a>, FromPrelude), Determinacy> {
         // General principles:
@@ -620,19 +629,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                 }
                 WhereToResolve::MacroUsePrelude => {
                     match self.macro_use_prelude.get(&ident.name).cloned() {
-                        Some(binding) => {
-                            let mut result = Ok((binding, FromPrelude(true)));
-                            // FIXME: Keep some built-in macros working even if they are
-                            // shadowed by non-attribute macros imported with `macro_use`.
-                            // We need to come up with some more principled approach instead.
-                            if is_attr && (ident.name == "test" || ident.name == "bench") {
-                                if let Def::Macro(_, MacroKind::Bang) =
-                                        binding.def_ignoring_ambiguity() {
-                                    result = Err(Determinacy::Determined);
-                                }
-                            }
-                            result
-                        }
+                        Some(binding) => Ok((binding, FromPrelude(true))),
                         None => Err(Determinacy::Determined),
                     }
                 }
@@ -646,7 +643,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     // FIXME: Only built-in attributes are not considered as candidates for
                     // non-attributes to fight off regressions on stable channel (#53205).
                     // We need to come up with some more principled approach instead.
-                    if is_attr && is_builtin_attr_name(ident.name) {
+                    if kind == Some(MacroKind::Attr) && is_builtin_attr_name(ident.name) {
                         let binding = (Def::NonMacroAttr(NonMacroAttrKind::Builtin),
                                        ty::Visibility::Public, ident.span, Mark::root())
                                        .to_name_binding(self.arenas);
@@ -746,6 +743,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
 
             match result {
                 Ok(result) => {
+                    if macro_kind_mismatch(ident.name, kind, result.0.macro_kind()) {
+                        continue_search!();
+                    }
+
                     if !record_used {
                         return Ok(result);
                     }
@@ -754,7 +755,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                         // Found another solution, if the first one was "weak", report an error.
                         if result.0.def() != innermost_result.0.def() &&
                            (innermost_result.0.is_glob_import() ||
-                            innermost_result.0.may_appear_after(invoc_id, result.0)) {
+                            innermost_result.0.may_appear_after(parent_expansion, result.0)) {
                             self.ambiguity_errors.push(AmbiguityError {
                                 ident,
                                 b1: innermost_result.0,
@@ -782,9 +783,9 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         }
 
         let determinacy = Determinacy::determined(force);
-        if determinacy == Determinacy::Determined && is_attr {
+        if determinacy == Determinacy::Determined && kind == Some(MacroKind::Attr) {
             // For single-segment attributes interpret determinate "no resolution" as a custom
-            // attribute. (Lexical resolution implies the first segment and is_attr should imply
+            // attribute. (Lexical resolution implies the first segment and attr kind should imply
             // the last segment, so we are certainly working with a single-segment attribute here.)
             assert!(ns == MacroNS);
             let binding = (Def::NonMacroAttr(NonMacroAttrKind::Custom),
@@ -798,15 +799,12 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
 
     fn resolve_legacy_scope(&mut self,
                             ident: Ident,
-                            invoc_id: Mark,
-                            invoc_parent_legacy_scope: LegacyScope<'a>,
-                            record_used: bool,
-                            is_attr: bool)
+                            kind: Option<MacroKind>,
+                            parent_expansion: Mark,
+                            parent_legacy_scope: LegacyScope<'a>,
+                            record_used: bool)
                             -> Option<&'a NameBinding<'a>> {
-        if is_attr && (ident.name == "test" || ident.name == "bench") {
-            // FIXME: Keep some built-in macros working even if they are
-            // shadowed by user-defined `macro_rules`.
-            // We need to come up with some more principled approach instead.
+        if macro_kind_mismatch(ident.name, kind, Some(MacroKind::Bang)) {
             return None;
         }
 
@@ -826,7 +824,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         let mut innermost_result: Option<&NameBinding> = None;
 
         // Go through all the scopes and try to resolve the name.
-        let mut where_to_resolve = invoc_parent_legacy_scope;
+        let mut where_to_resolve = parent_legacy_scope;
         loop {
             let result = match where_to_resolve {
                 LegacyScope::Binding(legacy_binding) if ident == legacy_binding.ident =>
@@ -854,7 +852,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     if let Some(innermost_result) = innermost_result {
                         // Found another solution, if the first one was "weak", report an error.
                         if result.def() != innermost_result.def() &&
-                           innermost_result.may_appear_after(invoc_id, result) {
+                           innermost_result.may_appear_after(parent_expansion, result) {
                             self.ambiguity_errors.push(AmbiguityError {
                                 ident,
                                 b1: innermost_result,
@@ -891,14 +889,14 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             }
         }
 
-        for &(invoc_id, ident, kind, def) in module.legacy_macro_resolutions.borrow().iter() {
+        for &(ident, kind, parent_expansion, parent_legacy_scope, def)
+                in module.legacy_macro_resolutions.borrow().iter() {
             let span = ident.span;
-            let invocation = self.invocations[&invoc_id];
             let legacy_resolution = self.resolve_legacy_scope(
-                ident, invoc_id, invocation.parent_legacy_scope.get(), true, kind == MacroKind::Attr
+                ident, Some(kind), parent_expansion, parent_legacy_scope, true
             );
             let resolution = self.resolve_lexical_macro_path_segment(
-                ident, MacroNS, invoc_id, true, true, kind == MacroKind::Attr, span
+                ident, MacroNS, Some(kind), parent_expansion, true, true, span
             );
 
             let check_consistency = |this: &Self, new_def: Def| {
@@ -932,12 +930,13 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     err.emit();
                 },
                 (Some(legacy_binding), Ok((binding, FromPrelude(from_prelude))))
-                        if !from_prelude || legacy_binding.may_appear_after(invoc_id, binding) => {
-                    if legacy_binding.def_ignoring_ambiguity() != binding.def_ignoring_ambiguity() {
-                        self.report_ambiguity_error(ident, legacy_binding, binding);
-                    }
+                        if legacy_binding.def() != binding.def_ignoring_ambiguity() &&
+                           (!from_prelude ||
+                            legacy_binding.may_appear_after(parent_expansion, binding)) => {
+                    self.report_ambiguity_error(ident, legacy_binding, binding);
                 },
                 // OK, non-macro-expanded legacy wins over prelude even if defs are different
+                // Also, legacy and modern can co-exist if their defs are same
                 (Some(legacy_binding), Ok(_)) |
                 // OK, unambiguous resolution
                 (Some(legacy_binding), Err(_)) => {
@@ -953,6 +952,26 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                 }
             };
         }
+
+        for &(ident, parent_expansion, parent_legacy_scope)
+                in module.builtin_attrs.borrow().iter() {
+            let resolve_legacy = |this: &mut Self| this.resolve_legacy_scope(
+                ident, Some(MacroKind::Attr), parent_expansion, parent_legacy_scope, true
+            );
+            let resolve_modern = |this: &mut Self| this.resolve_lexical_macro_path_segment(
+                ident, MacroNS, Some(MacroKind::Attr), parent_expansion, true, true, ident.span
+            ).map(|(binding, _)| binding).ok();
+
+            if let Some(binding) = resolve_legacy(self).or_else(|| resolve_modern(self)) {
+                if binding.def_ignoring_ambiguity() !=
+                        Def::NonMacroAttr(NonMacroAttrKind::Builtin) {
+                    let builtin_binding = (Def::NonMacroAttr(NonMacroAttrKind::Builtin),
+                                           ty::Visibility::Public, ident.span, Mark::root())
+                                           .to_name_binding(self.arenas);
+                    self.report_ambiguity_error(ident, binding, builtin_binding);
+                }
+            }
+        }
     }
 
     fn suggest_macro_name(&mut self, name: &str, kind: MacroKind,
@@ -1064,6 +1083,9 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                 self.define(module, ident, MacroNS,
                             (def, vis, item.span, expansion, IsMacroExport));
             } else {
+                if !attr::contains_name(&item.attrs, "rustc_doc_only_macro") {
+                    self.check_reserved_macro_name(ident, MacroNS);
+                }
                 self.unused_macros.insert(def_id);
             }
         } else {
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index 68709f6891a..dfbea0ffe22 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -452,6 +452,16 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
         })
     }
 
+    crate fn check_reserved_macro_name(&self, ident: Ident, ns: Namespace) {
+        // Reserve some names that are not quite covered by the general check
+        // performed on `Resolver::builtin_attrs`.
+        if ns == MacroNS &&
+           (ident.name == "cfg" || ident.name == "cfg_attr" || ident.name == "derive") {
+            self.session.span_err(ident.span,
+                                  &format!("name `{}` is reserved in macro namespace", ident));
+        }
+    }
+
     // Define the name or return the existing binding if there is a collision.
     pub fn try_define(&mut self,
                       module: Module<'a>,
@@ -459,6 +469,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
                       ns: Namespace,
                       binding: &'a NameBinding<'a>)
                       -> Result<(), &'a NameBinding<'a>> {
+        self.check_reserved_macro_name(ident, ns);
         self.update_resolution(module, ident, ns, |this, resolution| {
             if let Some(old_binding) = resolution.binding {
                 if binding.is_glob_import() {
diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs
index 1ea71009766..07c3e578e5b 100644
--- a/src/libsyntax/ext/base.rs
+++ b/src/libsyntax/ext/base.rs
@@ -727,10 +727,9 @@ pub trait Resolver {
     fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<Attribute>, allow_derive: bool)
                               -> Option<Attribute>;
 
-    fn resolve_macro_invocation(&mut self, invoc: &Invocation, scope: Mark, force: bool)
+    fn resolve_macro_invocation(&mut self, invoc: &Invocation, invoc_id: Mark, force: bool)
                                 -> Result<Option<Lrc<SyntaxExtension>>, Determinacy>;
-
-    fn resolve_macro_path(&mut self, path: &ast::Path, kind: MacroKind, scope: Mark,
+    fn resolve_macro_path(&mut self, path: &ast::Path, kind: MacroKind, invoc_id: Mark,
                           derives_in_scope: &[ast::Path], force: bool)
                           -> Result<Lrc<SyntaxExtension>, Determinacy>;
 
@@ -764,11 +763,11 @@ impl Resolver for DummyResolver {
     fn resolve_imports(&mut self) {}
     fn find_legacy_attr_invoc(&mut self, _attrs: &mut Vec<Attribute>, _allow_derive: bool)
                               -> Option<Attribute> { None }
-    fn resolve_macro_invocation(&mut self, _invoc: &Invocation, _scope: Mark, _force: bool)
+    fn resolve_macro_invocation(&mut self, _invoc: &Invocation, _invoc_id: Mark, _force: bool)
                                 -> Result<Option<Lrc<SyntaxExtension>>, Determinacy> {
         Err(Determinacy::Determined)
     }
-    fn resolve_macro_path(&mut self, _path: &ast::Path, _kind: MacroKind, _scope: Mark,
+    fn resolve_macro_path(&mut self, _path: &ast::Path, _kind: MacroKind, _invoc_id: Mark,
                           _derives_in_scope: &[ast::Path], _force: bool)
                           -> Result<Lrc<SyntaxExtension>, Determinacy> {
         Err(Determinacy::Determined)
diff --git a/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs-test.rs b/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs-test.rs
new file mode 100644
index 00000000000..6a47e50f62d
--- /dev/null
+++ b/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs-test.rs
@@ -0,0 +1,20 @@
+// aux-build:builtin-attrs.rs
+// compile-flags:--test
+
+#![feature(decl_macro, test)]
+
+extern crate test;
+extern crate builtin_attrs;
+use builtin_attrs::{test, bench};
+
+#[test] // OK, shadowed
+fn test() {}
+
+#[bench] // OK, shadowed
+fn bench(b: &mut test::Bencher) {}
+
+fn not_main() {
+    Test;
+    Bench;
+    NonExistent; //~ ERROR cannot find value `NonExistent` in this scope
+}
diff --git a/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs-test.stderr b/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs-test.stderr
new file mode 100644
index 00000000000..db07055b6a1
--- /dev/null
+++ b/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs-test.stderr
@@ -0,0 +1,9 @@
+error[E0425]: cannot find value `NonExistent` in this scope
+  --> $DIR/ambiguous-builtin-attrs-test.rs:19:5
+   |
+LL |     NonExistent; //~ ERROR cannot find value `NonExistent` in this scope
+   |     ^^^^^^^^^^^ not found in this scope
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0425`.
diff --git a/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs.rs b/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs.rs
new file mode 100644
index 00000000000..9f4f0abf324
--- /dev/null
+++ b/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs.rs
@@ -0,0 +1,31 @@
+// aux-build:builtin-attrs.rs
+
+#![feature(decl_macro)] //~ ERROR `feature` is ambiguous
+
+extern crate builtin_attrs;
+use builtin_attrs::{test, bench};
+use builtin_attrs::*;
+
+#[repr(C)] //~ ERROR `repr` is ambiguous
+struct S;
+#[cfg_attr(all(), repr(C))] //~ ERROR `repr` is ambiguous
+struct SCond;
+
+#[test] // OK, shadowed
+fn test() {}
+
+#[bench] // OK, shadowed
+fn bench() {}
+
+fn non_macro_expanded_location<#[repr(C)] T>() { //~ ERROR `repr` is ambiguous
+    match 0u8 {
+        #[repr(C)] //~ ERROR `repr` is ambiguous
+        _ => {}
+    }
+}
+
+fn main() {
+    Test;
+    Bench;
+    NonExistent; //~ ERROR cannot find value `NonExistent` in this scope
+}
diff --git a/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs.stderr b/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs.stderr
new file mode 100644
index 00000000000..ea867faf47b
--- /dev/null
+++ b/src/test/ui-fulldeps/proc-macro/ambiguous-builtin-attrs.stderr
@@ -0,0 +1,100 @@
+error[E0659]: `repr` is ambiguous
+  --> $DIR/ambiguous-builtin-attrs.rs:9:3
+   |
+LL | #[repr(C)] //~ ERROR `repr` is ambiguous
+   |   ^^^^ ambiguous name
+   |
+note: `repr` could refer to the name imported here
+  --> $DIR/ambiguous-builtin-attrs.rs:7:5
+   |
+LL | use builtin_attrs::*;
+   |     ^^^^^^^^^^^^^^^^
+note: `repr` could also refer to the name defined here
+  --> $DIR/ambiguous-builtin-attrs.rs:9:3
+   |
+LL | #[repr(C)] //~ ERROR `repr` is ambiguous
+   |   ^^^^
+   = note: consider adding an explicit import of `repr` to disambiguate
+
+error[E0659]: `repr` is ambiguous
+  --> $DIR/ambiguous-builtin-attrs.rs:11:19
+   |
+LL | #[cfg_attr(all(), repr(C))] //~ ERROR `repr` is ambiguous
+   |                   ^^^^ ambiguous name
+   |
+note: `repr` could refer to the name imported here
+  --> $DIR/ambiguous-builtin-attrs.rs:7:5
+   |
+LL | use builtin_attrs::*;
+   |     ^^^^^^^^^^^^^^^^
+note: `repr` could also refer to the name defined here
+  --> $DIR/ambiguous-builtin-attrs.rs:11:19
+   |
+LL | #[cfg_attr(all(), repr(C))] //~ ERROR `repr` is ambiguous
+   |                   ^^^^
+   = note: consider adding an explicit import of `repr` to disambiguate
+
+error[E0659]: `repr` is ambiguous
+  --> $DIR/ambiguous-builtin-attrs.rs:20:34
+   |
+LL | fn non_macro_expanded_location<#[repr(C)] T>() { //~ ERROR `repr` is ambiguous
+   |                                  ^^^^ ambiguous name
+   |
+note: `repr` could refer to the name imported here
+  --> $DIR/ambiguous-builtin-attrs.rs:7:5
+   |
+LL | use builtin_attrs::*;
+   |     ^^^^^^^^^^^^^^^^
+note: `repr` could also refer to the name defined here
+  --> $DIR/ambiguous-builtin-attrs.rs:20:34
+   |
+LL | fn non_macro_expanded_location<#[repr(C)] T>() { //~ ERROR `repr` is ambiguous
+   |                                  ^^^^
+   = note: consider adding an explicit import of `repr` to disambiguate
+
+error[E0659]: `repr` is ambiguous
+  --> $DIR/ambiguous-builtin-attrs.rs:22:11
+   |
+LL |         #[repr(C)] //~ ERROR `repr` is ambiguous
+   |           ^^^^ ambiguous name
+   |
+note: `repr` could refer to the name imported here
+  --> $DIR/ambiguous-builtin-attrs.rs:7:5
+   |
+LL | use builtin_attrs::*;
+   |     ^^^^^^^^^^^^^^^^
+note: `repr` could also refer to the name defined here
+  --> $DIR/ambiguous-builtin-attrs.rs:22:11
+   |
+LL |         #[repr(C)] //~ ERROR `repr` is ambiguous
+   |           ^^^^
+   = note: consider adding an explicit import of `repr` to disambiguate
+
+error[E0659]: `feature` is ambiguous
+  --> $DIR/ambiguous-builtin-attrs.rs:3:4
+   |
+LL | #![feature(decl_macro)] //~ ERROR `feature` is ambiguous
+   |    ^^^^^^^ ambiguous name
+   |
+note: `feature` could refer to the name imported here
+  --> $DIR/ambiguous-builtin-attrs.rs:7:5
+   |
+LL | use builtin_attrs::*;
+   |     ^^^^^^^^^^^^^^^^
+note: `feature` could also refer to the name defined here
+  --> $DIR/ambiguous-builtin-attrs.rs:3:4
+   |
+LL | #![feature(decl_macro)] //~ ERROR `feature` is ambiguous
+   |    ^^^^^^^
+   = note: consider adding an explicit import of `feature` to disambiguate
+
+error[E0425]: cannot find value `NonExistent` in this scope
+  --> $DIR/ambiguous-builtin-attrs.rs:30:5
+   |
+LL |     NonExistent; //~ ERROR cannot find value `NonExistent` in this scope
+   |     ^^^^^^^^^^^ not found in this scope
+
+error: aborting due to 6 previous errors
+
+Some errors occurred: E0425, E0659.
+For more information about an error, try `rustc --explain E0425`.
diff --git a/src/test/ui-fulldeps/proc-macro/auxiliary/builtin-attrs.rs b/src/test/ui-fulldeps/proc-macro/auxiliary/builtin-attrs.rs
new file mode 100644
index 00000000000..e18ca57aab1
--- /dev/null
+++ b/src/test/ui-fulldeps/proc-macro/auxiliary/builtin-attrs.rs
@@ -0,0 +1,36 @@
+// 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.
+
+// no-prefer-dynamic
+
+#![crate_type = "proc-macro"]
+
+extern crate proc_macro;
+use proc_macro::*;
+
+#[proc_macro_attribute]
+pub fn feature(_: TokenStream, input: TokenStream) -> TokenStream {
+    input
+}
+
+#[proc_macro_attribute]
+pub fn repr(_: TokenStream, input: TokenStream) -> TokenStream {
+    input
+}
+
+#[proc_macro_attribute]
+pub fn test(_: TokenStream, input: TokenStream) -> TokenStream {
+    "struct Test;".parse().unwrap()
+}
+
+#[proc_macro_attribute]
+pub fn bench(_: TokenStream, input: TokenStream) -> TokenStream {
+    "struct Bench;".parse().unwrap()
+}
diff --git a/src/test/ui-fulldeps/proc-macro/reserved-macro-names.rs b/src/test/ui-fulldeps/proc-macro/reserved-macro-names.rs
new file mode 100644
index 00000000000..ff5984aa67c
--- /dev/null
+++ b/src/test/ui-fulldeps/proc-macro/reserved-macro-names.rs
@@ -0,0 +1,22 @@
+#![crate_type = "proc-macro"]
+
+extern crate proc_macro;
+use proc_macro::*;
+
+#[proc_macro_attribute]
+pub fn cfg(_: TokenStream, input: TokenStream) -> TokenStream {
+    //~^ ERROR name `cfg` is reserved in macro namespace
+    input
+}
+
+#[proc_macro_attribute]
+pub fn cfg_attr(_: TokenStream, input: TokenStream) -> TokenStream {
+    //~^ ERROR name `cfg_attr` is reserved in macro namespace
+    input
+}
+
+#[proc_macro_attribute]
+pub fn derive(_: TokenStream, input: TokenStream) -> TokenStream {
+    //~^ ERROR name `derive` is reserved in macro namespace
+    input
+}
diff --git a/src/test/ui-fulldeps/proc-macro/reserved-macro-names.stderr b/src/test/ui-fulldeps/proc-macro/reserved-macro-names.stderr
new file mode 100644
index 00000000000..be6e80c3878
--- /dev/null
+++ b/src/test/ui-fulldeps/proc-macro/reserved-macro-names.stderr
@@ -0,0 +1,20 @@
+error: name `cfg` is reserved in macro namespace
+  --> $DIR/reserved-macro-names.rs:7:8
+   |
+LL | pub fn cfg(_: TokenStream, input: TokenStream) -> TokenStream {
+   |        ^^^
+
+error: name `cfg_attr` is reserved in macro namespace
+  --> $DIR/reserved-macro-names.rs:13:8
+   |
+LL | pub fn cfg_attr(_: TokenStream, input: TokenStream) -> TokenStream {
+   |        ^^^^^^^^
+
+error: name `derive` is reserved in macro namespace
+  --> $DIR/reserved-macro-names.rs:19:8
+   |
+LL | pub fn derive(_: TokenStream, input: TokenStream) -> TokenStream {
+   |        ^^^^^^
+
+error: aborting due to 3 previous errors
+