resolve: Introduce a new scope for derive helpers

Author: petrochenkov

src/librustc_resolve/diagnostics.rs

@@ -368,6 +368,15 @@ impl<'a> Resolver<'a> {
         let mut suggestions = Vec::new();
         self.visit_scopes(scope_set, parent_scope, ident, |this, scope, use_prelude, _| {
             match scope {
+                Scope::DeriveHelpers(expn_id) => {
+                    let res = Res::NonMacroAttr(NonMacroAttrKind::DeriveHelper);
+                    if filter_fn(res) {
+                        suggestions.extend(this.helper_attrs.get(&expn_id)
+                                               .into_iter().flatten().map(|ident| {
+                            TypoSuggestion::from_res(ident.name, res)
+                        }));
+                    }
+                }
                 Scope::DeriveHelpersCompat => {
                     let res = Res::NonMacroAttr(NonMacroAttrKind::DeriveHelper);
                     if filter_fn(res) {

src/librustc_resolve/lib.rs

@@ -45,7 +45,7 @@ use syntax::symbol::{kw, sym};
 use syntax::source_map::Spanned;
 use syntax::visit::{self, Visitor};
 use syntax_expand::base::SyntaxExtension;
-use syntax_pos::hygiene::{MacroKind, ExpnId, Transparency, SyntaxContext};
+use syntax_pos::hygiene::{MacroKind, ExpnId, ExpnKind, Transparency, SyntaxContext};
 use syntax_pos::{Span, DUMMY_SP};
 use errors::{Applicability, DiagnosticBuilder};
 
@@ -97,6 +97,7 @@ impl Determinacy {
 /// but not for late resolution yet.
 #[derive(Clone, Copy)]
 enum Scope<'a> {
+    DeriveHelpers(ExpnId),
     DeriveHelpersCompat,
     MacroRules(LegacyScope<'a>),
     CrateRoot,
@@ -942,6 +943,8 @@ pub struct Resolver<'a> {
     /// Legacy scopes *produced* by expanding the macro invocations,
     /// include all the `macro_rules` items and other invocations generated by them.
     output_legacy_scopes: FxHashMap<ExpnId, LegacyScope<'a>>,
+    /// Helper attributes that are in scope for the given expansion.
+    helper_attrs: FxHashMap<ExpnId, Vec<Ident>>,
 
     /// Avoid duplicated errors for "name already defined".
     name_already_seen: FxHashMap<Name, Span>,
@@ -1219,6 +1222,7 @@ impl<'a> Resolver<'a> {
             non_macro_attrs: [non_macro_attr(false), non_macro_attr(true)],
             invocation_parent_scopes,
             output_legacy_scopes: Default::default(),
+            helper_attrs: Default::default(),
             macro_defs,
             local_macro_def_scopes: FxHashMap::default(),
             name_already_seen: FxHashMap::default(),
@@ -1467,23 +1471,26 @@ impl<'a> Resolver<'a> {
         //    in prelude, not sure where exactly (creates ambiguities with any other prelude names).
 
         let rust_2015 = ident.span.rust_2015();
-        let (ns, is_absolute_path) = match scope_set {
-            ScopeSet::All(ns, _) => (ns, false),
-            ScopeSet::AbsolutePath(ns) => (ns, true),
-            ScopeSet::Macro(_) => (MacroNS, false),
+        let (ns, macro_kind, is_absolute_path) = match scope_set {
+            ScopeSet::All(ns, _) => (ns, None, false),
+            ScopeSet::AbsolutePath(ns) => (ns, None, true),
+            ScopeSet::Macro(macro_kind) => (MacroNS, Some(macro_kind), false),
         };
         // Jump out of trait or enum modules, they do not act as scopes.
         let module = parent_scope.module.nearest_item_scope();
         let mut scope = match ns {
             _ if is_absolute_path => Scope::CrateRoot,
             TypeNS | ValueNS => Scope::Module(module),
-            MacroNS => Scope::DeriveHelpersCompat,
+            MacroNS => Scope::DeriveHelpers(parent_scope.expansion),
         };
         let mut ident = ident.modern();
         let mut use_prelude = !module.no_implicit_prelude;
 
         loop {
             let visit = match scope {
+                // Derive helpers are not in scope when resolving derives in the same container.
+                Scope::DeriveHelpers(expn_id) =>
+                    !(expn_id == parent_scope.expansion && macro_kind == Some(MacroKind::Derive)),
                 Scope::DeriveHelpersCompat => true,
                 Scope::MacroRules(..) => true,
                 Scope::CrateRoot => true,
@@ -1505,6 +1512,17 @@ impl<'a> Resolver<'a> {
             }
 
             scope = match scope {
+                Scope::DeriveHelpers(expn_id) if expn_id != ExpnId::root() => {
+                    // Derive helpers are not visible to code generated by bang or derive macros.
+                    let expn_data = expn_id.expn_data();
+                    match expn_data.kind {
+                        ExpnKind::Root |
+                        ExpnKind::Macro(MacroKind::Bang, _) |
+                        ExpnKind::Macro(MacroKind::Derive, _) => Scope::DeriveHelpersCompat,
+                        _ => Scope::DeriveHelpers(expn_data.parent),
+                    }
+                }
+                Scope::DeriveHelpers(..) => Scope::DeriveHelpersCompat,
                 Scope::DeriveHelpersCompat =>
                     Scope::MacroRules(parent_scope.legacy),
                 Scope::MacroRules(legacy_scope) => match legacy_scope {

src/librustc_resolve/macros.rs

@@ -237,15 +237,23 @@ impl<'a> base::Resolver for Resolver<'a> {
                 // - Derives in the container need to know whether one of them is a built-in `Copy`.
                 // FIXME: Try to avoid repeated resolutions for derives here and in expansion.
                 let mut exts = Vec::new();
+                let mut helper_attrs = Vec::new();
                 for path in derives {
                     exts.push(match self.resolve_macro_path(
                         path, Some(MacroKind::Derive), &parent_scope, true, force
                     ) {
-                        Ok((Some(ext), _)) => ext,
+                        Ok((Some(ext), _)) => {
+                            let span = path.segments.last().unwrap().ident.span.modern();
+                            helper_attrs.extend(
+                                ext.helper_attrs.iter().map(|name| Ident::new(*name, span))
+                            );
+                            ext
+                        }
                         Ok(_) | Err(Determinacy::Determined) => self.dummy_ext(MacroKind::Derive),
                         Err(Determinacy::Undetermined) => return Err(Indeterminate),
                     })
                 }
+                self.helper_attrs.insert(invoc_id, helper_attrs);
                 return Ok(InvocationRes::DeriveContainer(exts));
             }
         };
@@ -498,6 +506,18 @@ impl<'a> Resolver<'a> {
                 Flags::empty(),
             ));
             let result = match scope {
+                Scope::DeriveHelpers(expn_id) => {
+                    if let Some(attr) = this.helper_attrs.get(&expn_id).and_then(|attrs| {
+                        attrs.iter().rfind(|i| ident == **i)
+                    }) {
+                        let binding = (Res::NonMacroAttr(NonMacroAttrKind::DeriveHelper),
+                                       ty::Visibility::Public, attr.span, expn_id)
+                                       .to_name_binding(this.arenas);
+                        Ok((binding, Flags::empty()))
+                    } else {
+                        Err(Determinacy::Determined)
+                    }
+                }
                 Scope::DeriveHelpersCompat => {
                     let mut result = Err(Determinacy::Determined);
                     for derive in parent_scope.derives {

src/test/ui/proc-macro/derive-helper-shadowing.rs

@@ -1,3 +1,4 @@
+// edition:2018
 // aux-build:test-macros.rs
 
 #[macro_use]
@@ -11,8 +12,7 @@ struct S {
     // FIXME No ambiguity, attributes in non-macro positions are not resolved properly
     #[empty_helper]
     field: [u8; {
-        // FIXME No ambiguity, derive helpers are not put into scope for non-attributes
-        use empty_helper;
+        use empty_helper; //~ ERROR `empty_helper` is ambiguous
 
         // FIXME No ambiguity, derive helpers are not put into scope for inner items
         #[empty_helper]

src/test/ui/proc-macro/derive-helper-shadowing.stderr

@@ -1,21 +1,39 @@
+error[E0659]: `empty_helper` is ambiguous (name vs any other name during import resolution)
+  --> $DIR/derive-helper-shadowing.rs:15:13
+   |
+LL |         use empty_helper;
+   |             ^^^^^^^^^^^^ ambiguous name
+   |
+note: `empty_helper` could refer to the derive helper attribute defined here
+  --> $DIR/derive-helper-shadowing.rs:10:10
+   |
+LL | #[derive(Empty)]
+   |          ^^^^^
+note: `empty_helper` could also refer to the attribute macro imported here
+  --> $DIR/derive-helper-shadowing.rs:7:5
+   |
+LL | use test_macros::empty_attr as empty_helper;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: use `crate::empty_helper` to refer to this attribute macro unambiguously
+
 error[E0659]: `empty_helper` is ambiguous (derive helper attribute vs any other name)
-  --> $DIR/derive-helper-shadowing.rs:8:3
+  --> $DIR/derive-helper-shadowing.rs:9:3
    |
 LL | #[empty_helper]
    |   ^^^^^^^^^^^^ ambiguous name
    |
 note: `empty_helper` could refer to the derive helper attribute defined here
-  --> $DIR/derive-helper-shadowing.rs:9:10
+  --> $DIR/derive-helper-shadowing.rs:10:10
    |
 LL | #[derive(Empty)]
    |          ^^^^^
 note: `empty_helper` could also refer to the attribute macro imported here
-  --> $DIR/derive-helper-shadowing.rs:6:5
+  --> $DIR/derive-helper-shadowing.rs:7:5
    |
 LL | use test_macros::empty_attr as empty_helper;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = help: use `crate::empty_helper` to refer to this attribute macro unambiguously
 
-error: aborting due to previous error
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0659`.