Auto merge of #63867 - petrochenkov:dhelpers, r=matthewjasper

resolve: Block expansion of a derive container until all its derives are resolved

So, it turns out there's one more reason to block expansion of a `#[derive]` container until all the derives inside it are resolved, beside `Copy` (#63248).

The set of derive helper attributes registered by derives in the container also has to be known before the derives themselves are expanded, otherwise it may be too late (see #63468 (comment) and the `#[stable_hasher]`-related test failures in #63468).

So, we stop our attempts to unblock the container earlier, as soon as the `Copy` status is known, and just block until all its derives are resolved.
After all the derives are resolved we immediately go and process their helper attributes in the item, without delaying it until expansion of the individual derives.

Unblocks #63468
r? @matthewjasper (as a reviewer of #63248)
cc @c410-f3r

src/librustc_resolve/macros.rs

@@ -10,8 +10,8 @@ use crate::resolve_imports::ImportResolver;
 use rustc::hir::def::{self, DefKind, NonMacroAttrKind};
 use rustc::middle::stability;
 use rustc::{ty, lint, span_bug};
-use syntax::ast::{self, NodeId, Ident};
-use syntax::attr::StabilityLevel;
+use syntax::ast::{self, NodeId, Ident, Mac, Attribute};
+use syntax::attr::{self, StabilityLevel};
 use syntax::edition::Edition;
 use syntax::ext::base::{self, Indeterminate, SpecialDerives};
 use syntax::ext::base::{MacroKind, SyntaxExtension};
@@ -21,6 +21,7 @@ use syntax::ext::tt::macro_rules;
 use syntax::feature_gate::{emit_feature_err, is_builtin_attr_name};
 use syntax::feature_gate::GateIssue;
 use syntax::symbol::{Symbol, kw, sym};
+use syntax::visit::Visitor;
 use syntax_pos::{Span, DUMMY_SP};
 
 use std::{mem, ptr};
@@ -54,6 +55,21 @@ pub enum LegacyScope<'a> {
     Invocation(ExpnId),
 }
 
+struct MarkDeriveHelpers<'a>(&'a [ast::Name]);
+
+impl<'a> Visitor<'a> for MarkDeriveHelpers<'a> {
+    fn visit_attribute(&mut self, attr: &Attribute) {
+        if let Some(ident) = attr.ident() {
+            if self.0.contains(&ident.name) {
+                attr::mark_used(attr);
+                attr::mark_known(attr);
+            }
+        }
+    }
+
+    fn visit_mac(&mut self, _mac: &Mac) {}
+}
+
 // Macro namespace is separated into two sub-namespaces, one for bang macros and
 // one for attribute-like macros (attributes, derives).
 // We ignore resolutions from one sub-namespace when searching names in scope for another.
@@ -164,26 +180,37 @@ impl<'a> base::Resolver for Resolver<'a> {
                 (&mac.path, MacroKind::Bang, &[][..], false),
             InvocationKind::Derive { ref path, .. } =>
                 (path, MacroKind::Derive, &[][..], false),
-            InvocationKind::DeriveContainer { ref derives, .. } => {
-                // Block expansion of derives in the container until we know whether one of them
-                // is a built-in `Copy`. Skip the resolution if there's only one derive - either
-                // it's not a `Copy` and we don't need to do anything, or it's a `Copy` and it
-                // will automatically knows about itself.
-                let mut result = Ok(None);
-                if derives.len() > 1 {
-                    for path in derives {
-                        match self.resolve_macro_path(path, Some(MacroKind::Derive),
-                                                      &parent_scope, true, force) {
-                            Ok((Some(ref ext), _)) if ext.is_derive_copy => {
+            InvocationKind::DeriveContainer { ref derives, ref item } => {
+                // Block expansion of the container until we resolve all derives in it.
+                // This is required for two reasons:
+                // - Derive helper attributes are in scope for the item to which the `#[derive]`
+                //   is applied, so they have to be produced by the container's expansion rather
+                //   than by individual derives.
+                // - 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 derive_helpers = Vec::new();
+                for path in derives {
+                    match self.resolve_macro_path(
+                        path, Some(MacroKind::Derive), &parent_scope, true, force
+                    ) {
+                        Ok((Some(ref ext), _)) => {
+                            derive_helpers.extend(&ext.helper_attrs);
+                            if ext.is_derive_copy {
                                 self.add_derives(invoc_id, SpecialDerives::COPY);
-                                return Ok(None);
                             }
-                            Err(Determinacy::Undetermined) => result = Err(Indeterminate),
-                            _ => {}
                         }
+                        Ok(_) | Err(Determinacy::Determined) => {}
+                        Err(Determinacy::Undetermined) => return Err(Indeterminate),
                     }
                 }
-                return result;
+
+                // 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 MarkDeriveHelpers(&derive_helpers));
+
+                return Ok(None);
             }
         };
 

src/libsyntax/ext/base.rs

@@ -11,6 +11,7 @@ use crate::ptr::P;
 use crate::symbol::{kw, sym, Ident, Symbol};
 use crate::{ThinVec, MACRO_ARGUMENTS};
 use crate::tokenstream::{self, TokenStream, TokenTree};
+use crate::visit::Visitor;
 
 use errors::{DiagnosticBuilder, DiagnosticId};
 use smallvec::{smallvec, SmallVec};
@@ -72,6 +73,17 @@ impl Annotatable {
         }
     }
 
+    pub fn visit_with<'a, V: Visitor<'a>>(&'a self, visitor: &mut V) {
+        match self {
+            Annotatable::Item(item) => visitor.visit_item(item),
+            Annotatable::TraitItem(trait_item) => visitor.visit_trait_item(trait_item),
+            Annotatable::ImplItem(impl_item) => visitor.visit_impl_item(impl_item),
+            Annotatable::ForeignItem(foreign_item) => visitor.visit_foreign_item(foreign_item),
+            Annotatable::Stmt(stmt) => visitor.visit_stmt(stmt),
+            Annotatable::Expr(expr) => visitor.visit_expr(expr),
+        }
+    }
+
     pub fn expect_item(self) -> P<ast::Item> {
         match self {
             Annotatable::Item(i) => i,

src/libsyntax/ext/proc_macro.rs

@@ -1,13 +1,11 @@
-use crate::ast::{self, ItemKind, Attribute, Mac};
-use crate::attr::{mark_used, mark_known};
+use crate::ast::{self, ItemKind, Attribute};
 use crate::errors::{Applicability, FatalError};
 use crate::ext::base::{self, *};
 use crate::ext::proc_macro_server;
 use crate::parse::{self, token};
 use crate::parse::parser::PathStyle;
 use crate::symbol::sym;
 use crate::tokenstream::{self, TokenStream};
-use crate::visit::Visitor;
 
 use rustc_data_structures::sync::Lrc;
 use syntax_pos::{Span, DUMMY_SP};
@@ -111,9 +109,6 @@ impl MultiItemModifier for ProcMacroDerive {
             }
         }
 
-        // Mark attributes as known, and used.
-        MarkAttrs(&self.attrs).visit_item(&item);
-
         let token = token::Interpolated(Lrc::new(token::NtItem(item)));
         let input = tokenstream::TokenTree::token(token, DUMMY_SP).into();
 
@@ -164,21 +159,6 @@ impl MultiItemModifier for ProcMacroDerive {
     }
 }
 
-struct MarkAttrs<'a>(&'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) {}
-}
-
 pub fn is_proc_macro_attr(attr: &Attribute) -> bool {
     [sym::proc_macro, sym::proc_macro_attribute, sym::proc_macro_derive]
         .iter().any(|kind| attr.check_name(*kind))

src/test/ui/derives/deriving-bounds.stderr

@@ -1,15 +1,3 @@
-error: cannot find derive macro `Send` in this scope
-  --> $DIR/deriving-bounds.rs:1:10
-   |
-LL | #[derive(Send)]
-   |          ^^^^
-   |
-note: unsafe traits like `Send` should be implemented explicitly
-  --> $DIR/deriving-bounds.rs:1:10
-   |
-LL | #[derive(Send)]
-   |          ^^^^
-
 error: cannot find derive macro `Sync` in this scope
   --> $DIR/deriving-bounds.rs:5:10
    |
@@ -22,5 +10,17 @@ note: unsafe traits like `Sync` should be implemented explicitly
 LL | #[derive(Sync)]
    |          ^^^^
 
+error: cannot find derive macro `Send` in this scope
+  --> $DIR/deriving-bounds.rs:1:10
+   |
+LL | #[derive(Send)]
+   |          ^^^^
+   |
+note: unsafe traits like `Send` should be implemented explicitly
+  --> $DIR/deriving-bounds.rs:1:10
+   |
+LL | #[derive(Send)]
+   |          ^^^^
+
 error: aborting due to 2 previous errors
 

src/test/ui/feature-gate/issue-43106-gating-of-derive-2.stderr

@@ -1,5 +1,5 @@
 error: cannot find derive macro `x3300` in this scope
-  --> $DIR/issue-43106-gating-of-derive-2.rs:4:14
+  --> $DIR/issue-43106-gating-of-derive-2.rs:12:14
    |
 LL |     #[derive(x3300)]
    |              ^^^^^
@@ -11,7 +11,7 @@ LL |     #[derive(x3300)]
    |              ^^^^^
 
 error: cannot find derive macro `x3300` in this scope
-  --> $DIR/issue-43106-gating-of-derive-2.rs:12:14
+  --> $DIR/issue-43106-gating-of-derive-2.rs:4:14
    |
 LL |     #[derive(x3300)]
    |              ^^^^^

src/test/ui/feature-gate/issue-43106-gating-of-derive.rs

@@ -1,9 +1,6 @@
 // `#![derive]` raises errors when it occurs at contexts other than ADT
 // definitions.
 
-#![derive(Debug)]
-//~^ ERROR `derive` may only be applied to structs, enums and unions
-
 #[derive(Debug)]
 //~^ ERROR `derive` may only be applied to structs, enums and unions
 mod derive {

src/test/ui/feature-gate/issue-43106-gating-of-derive.stderr

@@ -1,38 +1,32 @@
 error: `derive` may only be applied to structs, enums and unions
   --> $DIR/issue-43106-gating-of-derive.rs:4:1
    |
-LL | #![derive(Debug)]
-   | ^^^^^^^^^^^^^^^^^ help: try an outer attribute: `#[derive(Debug)]`
-
-error: `derive` may only be applied to structs, enums and unions
-  --> $DIR/issue-43106-gating-of-derive.rs:7:1
-   |
 LL | #[derive(Debug)]
    | ^^^^^^^^^^^^^^^^
 
 error: `derive` may only be applied to structs, enums and unions
-  --> $DIR/issue-43106-gating-of-derive.rs:10:17
+  --> $DIR/issue-43106-gating-of-derive.rs:7:17
    |
 LL |     mod inner { #![derive(Debug)] }
    |                 ^^^^^^^^^^^^^^^^^ help: try an outer attribute: `#[derive(Debug)]`
 
 error: `derive` may only be applied to structs, enums and unions
-  --> $DIR/issue-43106-gating-of-derive.rs:13:5
+  --> $DIR/issue-43106-gating-of-derive.rs:10:5
    |
 LL |     #[derive(Debug)]
    |     ^^^^^^^^^^^^^^^^
 
 error: `derive` may only be applied to structs, enums and unions
-  --> $DIR/issue-43106-gating-of-derive.rs:26:5
+  --> $DIR/issue-43106-gating-of-derive.rs:23:5
    |
 LL |     #[derive(Debug)]
    |     ^^^^^^^^^^^^^^^^
 
 error: `derive` may only be applied to structs, enums and unions
-  --> $DIR/issue-43106-gating-of-derive.rs:30:5
+  --> $DIR/issue-43106-gating-of-derive.rs:27:5
    |
 LL |     #[derive(Debug)]
    |     ^^^^^^^^^^^^^^^^
 
-error: aborting due to 6 previous errors
+error: aborting due to 5 previous errors
 

src/test/ui/issues/issue-36617.rs

@@ -1,3 +1,4 @@
 #![derive(Copy)] //~ ERROR `derive` may only be applied to structs, enums and unions
+                 //~| ERROR cannot determine resolution for the derive macro `Copy`
 
 fn main() {}

src/test/ui/issues/issue-36617.stderr

@@ -4,5 +4,13 @@ error: `derive` may only be applied to structs, enums and unions
 LL | #![derive(Copy)]
    | ^^^^^^^^^^^^^^^^ help: try an outer attribute: `#[derive(Copy)]`
 
-error: aborting due to previous error
+error: cannot determine resolution for the derive macro `Copy`
+  --> $DIR/issue-36617.rs:1:11
+   |
+LL | #![derive(Copy)]
+   |           ^^^^
+   |
+   = note: import resolution is stuck, try simplifying macro imports
+
+error: aborting due to 2 previous errors
 

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

@@ -19,7 +19,8 @@ struct S {
         struct U;
 
         mod inner {
-            #[empty_helper] //~ ERROR cannot find attribute macro `empty_helper` in this scope
+            // FIXME No ambiguity, attributes in non-macro positions are not resolved properly
+            #[empty_helper]
             struct V;
         }
 

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

@@ -1,9 +1,3 @@
-error: cannot find attribute macro `empty_helper` in this scope
-  --> $DIR/derive-helper-shadowing.rs:22:15
-   |
-LL |             #[empty_helper]
-   |               ^^^^^^^^^^^^
-
 error[E0659]: `empty_helper` is ambiguous (derive helper attribute vs any other name)
   --> $DIR/derive-helper-shadowing.rs:8:3
    |
@@ -22,6 +16,6 @@ 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: aborting due to previous error
 
 For more information about this error, try `rustc --explain E0659`.