Merge branch 'master' into master

2 files changed, 63 insertions, 42 deletions

diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
index fe5078f26bd..d474e990211 100644
--- a/compiler/rustc_resolve/src/lib.rs
+++ b/compiler/rustc_resolve/src/lib.rs
@@ -37,7 +37,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
 use rustc_data_structures::ptr_key::PtrKey;
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
-use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
+use rustc_expand::base::{DeriveResolutions, SyntaxExtension, SyntaxExtensionKind};
 use rustc_hir::def::Namespace::*;
 use rustc_hir::def::{self, CtorOf, DefKind, NonMacroAttrKind, PartialRes};
 use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LocalDefId, CRATE_DEF_INDEX};
@@ -851,6 +851,12 @@ enum BuiltinMacroState {
     AlreadySeen(Span),
 }
 
+struct DeriveData {
+    resolutions: DeriveResolutions,
+    helper_attrs: Vec<(usize, Ident)>,
+    has_derive_copy: bool,
+}
+
 /// The main resolver class.
 ///
 /// This is the visitor that walks the whole crate.
@@ -973,8 +979,9 @@ pub struct Resolver<'a> {
     output_macro_rules_scopes: FxHashMap<ExpnId, MacroRulesScopeRef<'a>>,
     /// Helper attributes that are in scope for the given expansion.
     helper_attrs: FxHashMap<ExpnId, Vec<Ident>>,
-    /// Resolutions for paths inside the `#[derive(...)]` attribute with the given `ExpnId`.
-    derive_resolutions: FxHashMap<ExpnId, Vec<(Lrc<SyntaxExtension>, ast::Path)>>,
+    /// Ready or in-progress results of resolving paths inside the `#[derive(...)]` attribute
+    /// with the given `ExpnId`.
+    derive_data: FxHashMap<ExpnId, DeriveData>,
 
     /// Avoid duplicated errors for "name already defined".
     name_already_seen: FxHashMap<Symbol, Span>,
@@ -1310,7 +1317,7 @@ impl<'a> Resolver<'a> {
             invocation_parent_scopes: Default::default(),
             output_macro_rules_scopes: Default::default(),
             helper_attrs: Default::default(),
-            derive_resolutions: Default::default(),
+            derive_data: Default::default(),
             local_macro_def_scopes: FxHashMap::default(),
             name_already_seen: FxHashMap::default(),
             potentially_unused_imports: Vec::new(),
diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs
index d238f65c941..10e27f33c29 100644
--- a/compiler/rustc_resolve/src/macros.rs
+++ b/compiler/rustc_resolve/src/macros.rs
@@ -4,7 +4,7 @@
 use crate::imports::ImportResolver;
 use crate::Namespace::*;
 use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, BuiltinMacroState, Determinacy};
-use crate::{CrateLint, ParentScope, ResolutionError, Resolver, Scope, ScopeSet, Weak};
+use crate::{CrateLint, DeriveData, ParentScope, ResolutionError, Resolver, Scope, ScopeSet, Weak};
 use crate::{ModuleKind, ModuleOrUniformRoot, NameBinding, PathResult, Segment, ToNameBinding};
 use rustc_ast::{self as ast, Inline, ItemKind, ModKind, NodeId};
 use rustc_ast_lowering::ResolverAstLowering;
@@ -14,8 +14,8 @@ use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::ptr_key::PtrKey;
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::struct_span_err;
-use rustc_expand::base::Annotatable;
-use rustc_expand::base::{Indeterminate, ResolverExpand, SyntaxExtension, SyntaxExtensionKind};
+use rustc_expand::base::{Annotatable, DeriveResolutions, Indeterminate, ResolverExpand};
+use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
 use rustc_expand::compile_declarative_macro;
 use rustc_expand::expand::{AstFragment, Invocation, InvocationKind, SupportsMacroExpansion};
 use rustc_feature::is_builtin_attr_name;
@@ -359,8 +359,8 @@ impl<'a> ResolverExpand for Resolver<'a> {
     fn resolve_derives(
         &mut self,
         expn_id: ExpnId,
-        derives: Vec<ast::Path>,
         force: bool,
+        derive_paths: &dyn Fn() -> DeriveResolutions,
     ) -> Result<(), Indeterminate> {
         // Block expansion of the container until we resolve all derives in it.
         // This is required for two reasons:
@@ -368,49 +368,63 @@ impl<'a> ResolverExpand for Resolver<'a> {
         //   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 cache intermediate results to avoid resolving same derives multiple times.
+        // Temporarily take the data to avoid borrow checker conflicts.
+        let mut derive_data = mem::take(&mut self.derive_data);
+        let entry = derive_data.entry(expn_id).or_insert_with(|| DeriveData {
+            resolutions: derive_paths(),
+            helper_attrs: Vec::new(),
+            has_derive_copy: false,
+        });
         let parent_scope = self.invocation_parent_scopes[&expn_id];
-        let mut exts = Vec::new();
-        let mut helper_attrs = Vec::new();
-        let mut has_derive_copy = false;
-        for path in derives {
-            exts.push((
-                match self.resolve_macro_path(
-                    &path,
-                    Some(MacroKind::Derive),
-                    &parent_scope,
-                    true,
-                    force,
-                ) {
-                    Ok((Some(ext), _)) => {
-                        let span =
-                            path.segments.last().unwrap().ident.span.normalize_to_macros_2_0();
-                        helper_attrs
-                            .extend(ext.helper_attrs.iter().map(|name| Ident::new(*name, span)));
-                        has_derive_copy |= ext.builtin_name == Some(sym::Copy);
-                        ext
-                    }
-                    Ok(_) | Err(Determinacy::Determined) => self.dummy_ext(MacroKind::Derive),
-                    Err(Determinacy::Undetermined) => return Err(Indeterminate),
-                },
-                path,
-            ))
+        for (i, (path, opt_ext)) in entry.resolutions.iter_mut().enumerate() {
+            if opt_ext.is_none() {
+                *opt_ext = Some(
+                    match self.resolve_macro_path(
+                        &path,
+                        Some(MacroKind::Derive),
+                        &parent_scope,
+                        true,
+                        force,
+                    ) {
+                        Ok((Some(ext), _)) => {
+                            if !ext.helper_attrs.is_empty() {
+                                let last_seg = path.segments.last().unwrap();
+                                let span = last_seg.ident.span.normalize_to_macros_2_0();
+                                entry.helper_attrs.extend(
+                                    ext.helper_attrs
+                                        .iter()
+                                        .map(|name| (i, Ident::new(*name, span))),
+                                );
+                            }
+                            entry.has_derive_copy |= ext.builtin_name == Some(sym::Copy);
+                            ext
+                        }
+                        Ok(_) | Err(Determinacy::Determined) => self.dummy_ext(MacroKind::Derive),
+                        Err(Determinacy::Undetermined) => {
+                            assert!(self.derive_data.is_empty());
+                            self.derive_data = derive_data;
+                            return Err(Indeterminate);
+                        }
+                    },
+                );
+            }
         }
-        self.derive_resolutions.insert(expn_id, exts);
-        self.helper_attrs.insert(expn_id, helper_attrs);
+        // Sort helpers in a stable way independent from the derive resolution order.
+        entry.helper_attrs.sort_by_key(|(i, _)| *i);
+        self.helper_attrs
+            .insert(expn_id, entry.helper_attrs.iter().map(|(_, ident)| *ident).collect());
         // Mark this derive as having `Copy` either if it has `Copy` itself or if its parent derive
         // has `Copy`, to support cases like `#[derive(Clone, Copy)] #[derive(Debug)]`.
-        if has_derive_copy || self.has_derive_copy(parent_scope.expansion) {
+        if entry.has_derive_copy || self.has_derive_copy(parent_scope.expansion) {
             self.containers_deriving_copy.insert(expn_id);
         }
+        assert!(self.derive_data.is_empty());
+        self.derive_data = derive_data;
         Ok(())
     }
 
-    fn take_derive_resolutions(
-        &mut self,
-        expn_id: ExpnId,
-    ) -> Option<Vec<(Lrc<SyntaxExtension>, ast::Path)>> {
-        self.derive_resolutions.remove(&expn_id)
+    fn take_derive_resolutions(&mut self, expn_id: ExpnId) -> Option<DeriveResolutions> {
+        self.derive_data.remove(&expn_id).map(|data| data.resolutions)
     }
 
     // The function that implements the resolution logic of `#[cfg_accessible(path)]`.