expand: Stop marking derive helper attributes as known

Pass them through name resolution instead

Author: petrochenkov

src/librustc_resolve/macros.rs

@@ -247,6 +247,9 @@ impl<'a> base::Resolver for Resolver<'a> {
                             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),

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/derive-helper-shadowing.rs

@@ -9,17 +9,15 @@ use test_macros::empty_attr as empty_helper;
 #[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; {
         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;
         }

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

@@ -1,5 +1,5 @@
 error[E0659]: `empty_helper` is ambiguous (name vs any other name during import resolution)
-  --> $DIR/derive-helper-shadowing.rs:15:13
+  --> $DIR/derive-helper-shadowing.rs:14:13
    |
 LL |         use empty_helper;
    |             ^^^^^^^^^^^^ ambiguous name
@@ -34,6 +34,42 @@ 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 2 previous errors
+error[E0659]: `empty_helper` is ambiguous (derive helper attribute vs any other name)
+  --> $DIR/derive-helper-shadowing.rs:12:7
+   |
+LL |     #[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:16:11
+   |
+LL |         #[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: aborting due to 4 previous errors
 
 For more information about this error, try `rustc --explain E0659`.