Fully integrate derive helpers into name resolution

src/librustc_resolve/diagnostics.rs

@@ -368,7 +368,16 @@ 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 => {
+                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) {
                         for derive in parent_scope.derives {

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,7 +97,8 @@ impl Determinacy {
 /// but not for late resolution yet.
 #[derive(Clone, Copy)]
 enum Scope<'a> {
-    DeriveHelpers,
+    DeriveHelpers(ExpnId),
+    DeriveHelpersCompat,
     MacroRules(LegacyScope<'a>),
     CrateRoot,
     Module(Module<'a>),
@@ -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,24 +1471,27 @@ 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::DeriveHelpers,
+            MacroNS => Scope::DeriveHelpers(parent_scope.expansion),
         };
         let mut ident = ident.modern();
         let mut use_prelude = !module.no_implicit_prelude;
 
         loop {
             let visit = match scope {
-                Scope::DeriveHelpers => true,
+                // 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,
                 Scope::Module(..) => true,
@@ -1505,7 +1512,18 @@ impl<'a> Resolver<'a> {
             }
 
             scope = match scope {
-                Scope::DeriveHelpers =>
+                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 {
                     LegacyScope::Binding(binding) => Scope::MacroRules(

src/librustc_resolve/macros.rs

@@ -237,15 +237,26 @@ 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))
+                            );
+                            if ext.is_derive_copy {
+                                self.add_derive_copy(invoc_id);
+                            }
+                            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,7 +509,19 @@ impl<'a> Resolver<'a> {
                 Flags::empty(),
             ));
             let result = match scope {
-                Scope::DeriveHelpers => {
+                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 {
                         let parent_scope = &ParentScope { derives: &[], ..*parent_scope };

src/libsyntax_expand/expand.rs

@@ -1,5 +1,5 @@
 use crate::base::*;
-use crate::proc_macro::{collect_derives, MarkAttrs};
+use crate::proc_macro::collect_derives;
 use crate::hygiene::{ExpnId, SyntaxContext, ExpnData, ExpnKind};
 use crate::mbe::macro_rules::annotate_err_with_kind;
 use crate::placeholders::{placeholder, PlaceholderExpander};
@@ -394,7 +394,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                     let fragment = self.expand_invoc(invoc, &ext.kind);
                     self.collect_invocations(fragment, &[])
                 }
-                InvocationRes::DeriveContainer(exts) => {
+                InvocationRes::DeriveContainer(_exts) => {
+                    // FIXME: Consider using the derive resolutions (`_exts`) immediately,
+                    // instead of enqueuing the derives to be resolved again later.
                     let (derives, item) = match invoc.kind {
                         InvocationKind::DeriveContainer { derives, item } => (derives, item),
                         _ => unreachable!(),
@@ -421,20 +423,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
 
                     let mut item = self.fully_configure(item);
                     item.visit_attrs(|attrs| attrs.retain(|a| !a.has_name(sym::derive)));
-                    let mut helper_attrs = Vec::new();
-                    let mut has_copy = false;
-                    for ext in exts {
-                        helper_attrs.extend(&ext.helper_attrs);
-                        has_copy |= ext.is_derive_copy;
-                    }
-                    // Mark derive helpers inside this item as known and used.
-                    // FIXME: This is a hack, derive helpers should be integrated with regular name
-                    // resolution instead. For example, helpers introduced by a derive container
-                    // can be in scope for all code produced by that container's expansion.
-                    item.visit_with(&mut MarkAttrs(&helper_attrs));
-                    if has_copy {
-                        self.cx.resolver.add_derive_copy(invoc.expansion_data.id);
-                    }
 
                     let mut derive_placeholders = Vec::with_capacity(derives.len());
                     invocations.reserve(derives.len());

src/libsyntax_expand/proc_macro.rs

@@ -1,13 +1,11 @@
 use crate::base::{self, *};
 use crate::proc_macro_server;
 
-use syntax::ast::{self, ItemKind, Attribute, Mac};
-use syntax::attr::{mark_used, mark_known};
+use syntax::ast::{self, ItemKind};
 use syntax::errors::{Applicability, FatalError};
 use syntax::symbol::sym;
 use syntax::token;
 use syntax::tokenstream::{self, TokenStream};
-use syntax::visit::Visitor;
 
 use rustc_data_structures::sync::Lrc;
 use syntax_pos::{Span, DUMMY_SP};
@@ -167,21 +165,6 @@ impl MultiItemModifier for ProcMacroDerive {
     }
 }
 
-crate struct MarkAttrs<'a>(crate &'a [ast::Name]);
-
-impl<'a> Visitor<'a> for MarkAttrs<'a> {
-    fn visit_attribute(&mut self, attr: &Attribute) {
-        if let Some(ident) = attr.ident() {
-            if self.0.contains(&ident.name) {
-                mark_used(attr);
-                mark_known(attr);
-            }
-        }
-    }
-
-    fn visit_mac(&mut self, _mac: &Mac) {}
-}
-
 crate fn collect_derives(cx: &mut ExtCtxt<'_>, attrs: &mut Vec<ast::Attribute>) -> Vec<ast::Path> {
     let mut result = Vec::new();
     attrs.retain(|attr| {

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

@@ -0,0 +1,15 @@
+// force-host
+// no-prefer-dynamic
+
+#![crate_type = "proc-macro"]
+
+extern crate proc_macro;
+use proc_macro::*;
+
+#[proc_macro_derive(GenHelperUse)]
+pub fn derive_a(_: TokenStream) -> TokenStream {
+    "
+    #[empty_helper]
+    struct Uwu;
+    ".parse().unwrap()
+}

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

@@ -1,33 +1,54 @@
+// edition:2018
 // aux-build:test-macros.rs
+// aux-build:derive-helper-shadowing.rs
 
 #[macro_use]
 extern crate test_macros;
+#[macro_use]
+extern crate derive_helper_shadowing;
 
 use test_macros::empty_attr as empty_helper;
 
+macro_rules! gen_helper_use {
+    () => {
+        #[empty_helper] //~ ERROR cannot find attribute `empty_helper` in this scope
+        struct W;
+    }
+}
+
 #[empty_helper] //~ ERROR `empty_helper` is ambiguous
 #[derive(Empty)]
 struct S {
-    // FIXME No ambiguity, attributes in non-macro positions are not resolved properly
-    #[empty_helper]
+    #[empty_helper] //~ ERROR `empty_helper` is ambiguous
     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]
+        #[empty_helper] //~ ERROR `empty_helper` is ambiguous
         struct U;
 
         mod inner {
-            // FIXME No ambiguity, attributes in non-macro positions are not resolved properly
+            // OK, no ambiguity, the non-helper attribute is not in scope here, only the helper.
             #[empty_helper]
             struct V;
+
+            gen_helper_use!();
+
+            #[derive(GenHelperUse)] //~ ERROR cannot find attribute `empty_helper` in this scope
+            struct Owo;
+
+            use empty_helper as renamed;
+            #[renamed] //~ ERROR cannot use a derive helper attribute through an import
+            struct Wow;
         }
 
         0
     }]
 }
 
+// OK, no ambiguity, only the non-helper attribute is in scope.
+#[empty_helper]
+struct Z;
+
 fn main() {
     let s = S { field: [] };
 }