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

Also mark derive helpers as known as a part of the derive container's expansion instead of expansion of the derives themselves which may happen too late.

Author: petrochenkov

Diff for: 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);
             }
         };
 

Diff for: 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,

Diff for: 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))

Diff for: 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
 

Diff for: 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)]
    |              ^^^^^

Diff for: 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 {

Diff for: 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
 

Diff for: 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() {}

Diff for: 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
 

Diff for: 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;
         }
 

Diff for: 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`.