Move special treatment of derive(Copy, PartialEq, Eq) from expansion infrastructure to elsewhere

Author: petrochenkov

Diff for: src/librustc/hir/lowering.rs

@@ -61,6 +61,7 @@ use syntax::ast;
 use syntax::ptr::P as AstP;
 use syntax::ast::*;
 use syntax::errors;
+use syntax::ext::base::SpecialDerives;
 use syntax::ext::hygiene::ExpnId;
 use syntax::print::pprust;
 use syntax::source_map::{respan, ExpnInfo, ExpnKind, DesugaringKind, Spanned};
@@ -176,6 +177,8 @@ pub trait Resolver {
         components: &[Symbol],
         is_value: bool,
     ) -> (ast::Path, Res<NodeId>);
+
+    fn has_derives(&self, node_id: NodeId, derives: SpecialDerives) -> bool;
 }
 
 /// Context of `impl Trait` in code, which determines whether it is allowed in an HIR subtree,
@@ -1329,13 +1332,17 @@ impl<'a> LoweringContext<'a> {
         }
     }
 
-    fn lower_attrs(&mut self, attrs: &[Attribute]) -> hir::HirVec<Attribute> {
+    fn lower_attrs_extendable(&mut self, attrs: &[Attribute]) -> Vec<Attribute> {
         attrs
             .iter()
             .map(|a| self.lower_attr(a))
             .collect()
     }
 
+    fn lower_attrs(&mut self, attrs: &[Attribute]) -> hir::HirVec<Attribute> {
+        self.lower_attrs_extendable(attrs).into()
+    }
+
     fn lower_attr(&mut self, attr: &Attribute) -> Attribute {
         // Note that we explicitly do not walk the path. Since we don't really
         // lower attributes (we use the AST version) there is nowhere to keep
@@ -4028,7 +4035,14 @@ impl<'a> LoweringContext<'a> {
     pub fn lower_item(&mut self, i: &Item) -> Option<hir::Item> {
         let mut ident = i.ident;
         let mut vis = self.lower_visibility(&i.vis, None);
-        let attrs = self.lower_attrs(&i.attrs);
+        let mut attrs = self.lower_attrs_extendable(&i.attrs);
+        if self.resolver.has_derives(i.id, SpecialDerives::PARTIAL_EQ | SpecialDerives::EQ) {
+            // Add `#[structural_match]` if the item derived both `PartialEq` and `Eq`.
+            let ident = Ident::new(sym::structural_match, i.span);
+            attrs.push(attr::mk_attr_outer(attr::mk_word_item(ident)));
+        }
+        let attrs = attrs.into();
+
         if let ItemKind::MacroDef(ref def) = i.node {
             if !def.legacy || attr::contains_name(&i.attrs, sym::macro_export) {
                 let body = self.lower_token_stream(def.stream());

Diff for: src/librustc_resolve/lib.rs

@@ -38,8 +38,7 @@ use rustc_metadata::cstore::CStore;
 use syntax::source_map::SourceMap;
 use syntax::ext::hygiene::{ExpnId, Transparency, SyntaxContext};
 use syntax::ast::{self, Name, NodeId, Ident, FloatTy, IntTy, UintTy};
-use syntax::ext::base::SyntaxExtension;
-use syntax::ext::base::MacroKind;
+use syntax::ext::base::{SyntaxExtension, MacroKind, SpecialDerives};
 use syntax::symbol::{Symbol, kw, sym};
 use syntax::util::lev_distance::find_best_match_for_name;
 
@@ -1685,6 +1684,12 @@ pub struct Resolver<'a> {
     local_macro_def_scopes: FxHashMap<NodeId, Module<'a>>,
     unused_macros: NodeMap<Span>,
     proc_macro_stubs: NodeSet,
+    /// Some built-in derives mark items they are applied to so they are treated specially later.
+    /// Derive macros cannot modify the item themselves and have to store the markers in the global
+    /// context, so they attach the markers to derive container IDs using this resolver table.
+    /// FIXME: Find a way for `PartialEq` and `Eq` to emulate `#[structural_match]`
+    /// by marking the produced impls rather than the original items.
+    special_derives: FxHashMap<ExpnId, SpecialDerives>,
 
     /// Maps the `ExpnId` of an expansion to its containing module or block.
     invocations: FxHashMap<ExpnId, &'a InvocationData<'a>>,
@@ -1813,6 +1818,12 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
     fn definitions(&mut self) -> &mut Definitions {
         &mut self.definitions
     }
+
+    fn has_derives(&self, node_id: NodeId, derives: SpecialDerives) -> bool {
+        let def_id = self.definitions.local_def_id(node_id);
+        let expn_id = self.definitions.expansion_that_defined(def_id.index);
+        self.has_derives(expn_id, derives)
+    }
 }
 
 impl<'a> Resolver<'a> {
@@ -2031,6 +2042,7 @@ impl<'a> Resolver<'a> {
             struct_constructors: Default::default(),
             unused_macros: Default::default(),
             proc_macro_stubs: Default::default(),
+            special_derives: Default::default(),
             current_type_ascription: Vec::new(),
             injected_crate: None,
             active_features:
@@ -2077,6 +2089,10 @@ impl<'a> Resolver<'a> {
         }
     }
 
+    fn has_derives(&self, expn_id: ExpnId, markers: SpecialDerives) -> bool {
+        self.special_derives.get(&expn_id).map_or(false, |m| m.contains(markers))
+    }
+
     /// Entry point to crate resolution.
     pub fn resolve_crate(&mut self, krate: &Crate) {
         ImportResolver { resolver: self }.finalize_imports();

Diff for: src/librustc_resolve/macros.rs

@@ -14,7 +14,7 @@ use rustc::{ty, lint, span_bug};
 use syntax::ast::{self, Ident, ItemKind};
 use syntax::attr::{self, StabilityLevel};
 use syntax::edition::Edition;
-use syntax::ext::base::{self, Indeterminate};
+use syntax::ext::base::{self, Indeterminate, SpecialDerives};
 use syntax::ext::base::{MacroKind, SyntaxExtension};
 use syntax::ext::expand::{AstFragment, Invocation, InvocationKind};
 use syntax::ext::hygiene::{self, ExpnId, ExpnInfo, ExpnKind};
@@ -203,8 +203,28 @@ impl<'a> base::Resolver for Resolver<'a> {
                 (&mac.node.path, MacroKind::Bang, Vec::new(), false),
             InvocationKind::Derive { ref path, .. } =>
                 (path, MacroKind::Derive, Vec::new(), false),
-            InvocationKind::DeriveContainer { .. } =>
-                return Ok(None),
+            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 {
+                    let parent_scope = self.invoc_parent_scope(invoc_id, Vec::new());
+                    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 => {
+                                self.add_derives(invoc.expansion_data.id, SpecialDerives::COPY);
+                                return Ok(None);
+                            }
+                            Err(Determinacy::Undetermined) => result = Err(Indeterminate),
+                            _ => {}
+                        }
+                    }
+                }
+                return result;
+            }
         };
 
         let parent_scope = self.invoc_parent_scope(invoc_id, derives_in_scope);
@@ -234,6 +254,14 @@ impl<'a> base::Resolver for Resolver<'a> {
             );
         }
     }
+
+    fn has_derives(&self, expn_id: ExpnId, derives: SpecialDerives) -> bool {
+        self.has_derives(expn_id, derives)
+    }
+
+    fn add_derives(&mut self, expn_id: ExpnId, derives: SpecialDerives) {
+        *self.special_derives.entry(expn_id).or_default() |= derives;
+    }
 }
 
 impl<'a> Resolver<'a> {

Diff for: src/libsyntax/ext/base.rs

@@ -595,6 +595,8 @@ pub struct SyntaxExtension {
     /// Built-in macros have a couple of special properties (meaning of `$crate`,
     /// availability in `#[no_implicit_prelude]` modules), so we have to keep this flag.
     pub is_builtin: bool,
+    /// We have to identify macros providing a `Copy` impl early for compatibility reasons.
+    pub is_derive_copy: bool,
 }
 
 impl SyntaxExtensionKind {
@@ -640,6 +642,7 @@ impl SyntaxExtension {
             helper_attrs: Vec::new(),
             edition,
             is_builtin: false,
+            is_derive_copy: false,
             kind,
         }
     }
@@ -683,6 +686,16 @@ pub type NamedSyntaxExtension = (Name, SyntaxExtension);
 /// Error type that denotes indeterminacy.
 pub struct Indeterminate;
 
+bitflags::bitflags! {
+    /// Built-in derives that need some extra tracking beyond the usual macro functionality.
+    #[derive(Default)]
+    pub struct SpecialDerives: u8 {
+        const PARTIAL_EQ = 1 << 0;
+        const EQ         = 1 << 1;
+        const COPY       = 1 << 2;
+    }
+}
+
 pub trait Resolver {
     fn next_node_id(&mut self) -> ast::NodeId;
 
@@ -699,6 +712,9 @@ pub trait Resolver {
                                 -> Result<Option<Lrc<SyntaxExtension>>, Indeterminate>;
 
     fn check_unused_macros(&self);
+
+    fn has_derives(&self, expn_id: ExpnId, derives: SpecialDerives) -> bool;
+    fn add_derives(&mut self, expn_id: ExpnId, derives: SpecialDerives);
 }
 
 #[derive(Clone)]
@@ -725,7 +741,6 @@ pub struct ExtCtxt<'a> {
     pub resolver: &'a mut dyn Resolver,
     pub current_expansion: ExpansionData,
     pub expansions: FxHashMap<Span, Vec<String>>,
-    pub allow_derive_markers: Lrc<[Symbol]>,
 }
 
 impl<'a> ExtCtxt<'a> {
@@ -745,7 +760,6 @@ impl<'a> ExtCtxt<'a> {
                 directory_ownership: DirectoryOwnership::Owned { relative: None },
             },
             expansions: FxHashMap::default(),
-            allow_derive_markers: [sym::rustc_attrs, sym::structural_match][..].into(),
         }
     }
 

Diff for: src/libsyntax/ext/expand.rs

@@ -4,7 +4,7 @@ use crate::attr::{self, HasAttrs};
 use crate::source_map::{dummy_spanned, respan};
 use crate::config::StripUnconfigured;
 use crate::ext::base::*;
-use crate::ext::proc_macro::{add_derived_markers, collect_derives};
+use crate::ext::proc_macro::collect_derives;
 use crate::ext::hygiene::{ExpnId, SyntaxContext, ExpnInfo, ExpnKind};
 use crate::ext::placeholders::{placeholder, PlaceholderExpander};
 use crate::feature_gate::{self, Features, GateIssue, is_builtin_attr, emit_feature_err};
@@ -209,7 +209,6 @@ pub enum InvocationKind {
     Derive {
         path: Path,
         item: Annotatable,
-        item_with_markers: Annotatable,
     },
     /// "Invocation" that contains all derives from an item,
     /// broken into multiple `Derive` invocations when expanded.
@@ -360,8 +359,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
 
                 let mut item = self.fully_configure(item);
                 item.visit_attrs(|attrs| attrs.retain(|a| a.path != sym::derive));
-                let mut item_with_markers = item.clone();
-                add_derived_markers(&mut self.cx, item.span(), &traits, &mut item_with_markers);
                 let derives = derives.entry(invoc.expansion_data.id).or_default();
 
                 derives.reserve(traits.len());
@@ -370,11 +367,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                     let expn_id = ExpnId::fresh(self.cx.current_expansion.id, None);
                     derives.push(expn_id);
                     invocations.push(Invocation {
-                        kind: InvocationKind::Derive {
-                            path,
-                            item: item.clone(),
-                            item_with_markers: item_with_markers.clone(),
-                        },
+                        kind: InvocationKind::Derive { path, item: item.clone() },
                         fragment_kind: invoc.fragment_kind,
                         expansion_data: ExpansionData {
                             id: expn_id,
@@ -383,7 +376,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                     });
                 }
                 let fragment = invoc.fragment_kind
-                    .expect_from_annotatables(::std::iter::once(item_with_markers));
+                    .expect_from_annotatables(::std::iter::once(item));
                 self.collect_invocations(fragment, derives)
             } else {
                 unreachable!()
@@ -569,13 +562,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                 }
                 _ => unreachable!()
             }
-            InvocationKind::Derive { path, item, item_with_markers } => match ext {
+            InvocationKind::Derive { path, item } => match ext {
                 SyntaxExtensionKind::Derive(expander) |
                 SyntaxExtensionKind::LegacyDerive(expander) => {
-                    let (path, item) = match ext {
-                        SyntaxExtensionKind::LegacyDerive(..) => (path, item_with_markers),
-                        _ => (path, item),
-                    };
                     if !item.derive_allowed() {
                         return fragment_kind.dummy(span);
                     }

Diff for: src/libsyntax/ext/proc_macro.rs

@@ -1,17 +1,15 @@
 use crate::ast::{self, ItemKind, Attribute, Mac};
-use crate::attr::{mark_used, mark_known, HasAttrs};
+use crate::attr::{mark_used, mark_known};
 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, Symbol};
+use crate::symbol::sym;
 use crate::tokenstream::{self, TokenStream};
 use crate::visit::Visitor;
 
-use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::sync::Lrc;
-use syntax_pos::hygiene::{ExpnInfo, ExpnKind};
 use syntax_pos::{Span, DUMMY_SP};
 
 const EXEC_STRATEGY: proc_macro::bridge::server::SameThread =
@@ -217,32 +215,3 @@ crate fn collect_derives(cx: &mut ExtCtxt<'_>, attrs: &mut Vec<ast::Attribute>)
     });
     result
 }
-
-crate fn add_derived_markers<T: HasAttrs>(
-    cx: &mut ExtCtxt<'_>, span: Span, traits: &[ast::Path], item: &mut T
-) {
-    let (mut names, mut pretty_name) = (FxHashSet::default(), String::new());
-    for (i, path) in traits.iter().enumerate() {
-        if i > 0 {
-            pretty_name.push_str(", ");
-        }
-        pretty_name.push_str(&path.to_string());
-        names.insert(unwrap_or!(path.segments.get(0), continue).ident.name);
-    }
-
-    let span = span.fresh_expansion(cx.current_expansion.id, ExpnInfo::allow_unstable(
-        ExpnKind::Macro(MacroKind::Derive, Symbol::intern(&pretty_name)), span,
-        cx.parse_sess.edition, cx.allow_derive_markers.clone(),
-    ));
-
-    item.visit_attrs(|attrs| {
-        if names.contains(&sym::Eq) && names.contains(&sym::PartialEq) {
-            let meta = cx.meta_word(span, sym::structural_match);
-            attrs.push(cx.attribute(meta));
-        }
-        if names.contains(&sym::Copy) {
-            let meta = cx.meta_word(span, sym::rustc_copy_clone_marker);
-            attrs.push(cx.attribute(meta));
-        }
-    });
-}

Diff for: src/libsyntax/ext/tt/macro_rules.rs

@@ -429,6 +429,8 @@ pub fn compile(
         }
     }
 
+    let is_builtin = attr::contains_name(&def.attrs, sym::rustc_builtin_macro);
+
     SyntaxExtension {
         kind: SyntaxExtensionKind::LegacyBang(expander),
         span: def.span,
@@ -440,7 +442,8 @@ pub fn compile(
         deprecation: attr::find_deprecation(&sess, &def.attrs, def.span),
         helper_attrs: Vec::new(),
         edition,
-        is_builtin: attr::contains_name(&def.attrs, sym::rustc_builtin_macro),
+        is_builtin,
+        is_derive_copy: is_builtin && def.ident.name == sym::Copy,
     }
 }
 

Diff for: src/libsyntax/feature_gate.rs

@@ -1329,11 +1329,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
                                                 /*opt*/ attributes(name1, name2, ...)"),
                                     Ungated),
 
-    (sym::rustc_copy_clone_marker, Whitelisted, template!(Word), Gated(Stability::Unstable,
-                                                sym::rustc_attrs,
-                                                "internal implementation detail",
-                                                cfg_fn!(rustc_attrs))),
-
     (sym::rustc_allocator, Whitelisted, template!(Word), Gated(Stability::Unstable,
                                                 sym::rustc_attrs,
                                                 "internal implementation detail",

Diff for: src/libsyntax_ext/deriving/clone.rs

@@ -3,8 +3,7 @@ use crate::deriving::generic::*;
 use crate::deriving::generic::ty::*;
 
 use syntax::ast::{self, Expr, GenericArg, Generics, ItemKind, MetaItem, VariantData};
-use syntax::attr;
-use syntax::ext::base::{Annotatable, ExtCtxt};
+use syntax::ext::base::{Annotatable, ExtCtxt, SpecialDerives};
 use syntax::ptr::P;
 use syntax::symbol::{kw, sym, Symbol};
 use syntax_pos::Span;
@@ -36,7 +35,8 @@ pub fn expand_deriving_clone(cx: &mut ExtCtxt<'_>,
             match annitem.node {
                 ItemKind::Struct(_, Generics { ref params, .. }) |
                 ItemKind::Enum(_, Generics { ref params, .. }) => {
-                    if attr::contains_name(&annitem.attrs, sym::rustc_copy_clone_marker) &&
+                    let container_id = cx.current_expansion.id.parent();
+                    if cx.resolver.has_derives(container_id, SpecialDerives::COPY) &&
                         !params.iter().any(|param| match param.kind {
                             ast::GenericParamKind::Type { .. } => true,
                             _ => false,