Only derive all of PartialOrd's methods when not also deriving PartialEq

Author: rylev

compiler/rustc_builtin_macros/src/deriving/clone.rs

@@ -4,7 +4,7 @@ use crate::deriving::path_std;
 
 use rustc_ast::ptr::P;
 use rustc_ast::{self as ast, Expr, GenericArg, Generics, ItemKind, MetaItem, VariantData};
-use rustc_expand::base::{Annotatable, ExtCtxt};
+use rustc_expand::base::{Annotatable, BuiltinDerive, ExtCtxt};
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::Span;
 
@@ -37,7 +37,7 @@ pub fn expand_deriving_clone(
             ItemKind::Struct(_, Generics { ref params, .. })
             | ItemKind::Enum(_, Generics { ref params, .. }) => {
                 let container_id = cx.current_expansion.id.expn_data().parent;
-                if cx.resolver.has_derive_copy(container_id)
+                if cx.resolver.has_derive(container_id, BuiltinDerive::Copy)
                     && !params.iter().any(|param| match param.kind {
                         ast::GenericParamKind::Type { .. } => true,
                         _ => false,

compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs

@@ -6,7 +6,7 @@ use crate::deriving::{path_local, path_std, pathvec_std};
 
 use rustc_ast::ptr::P;
 use rustc_ast::{self as ast, BinOpKind, Expr, MetaItem};
-use rustc_expand::base::{Annotatable, ExtCtxt};
+use rustc_expand::base::{Annotatable, BuiltinDerive, ExtCtxt};
 use rustc_span::symbol::{sym, Ident, Symbol};
 use rustc_span::Span;
 
@@ -63,9 +63,12 @@ pub fn expand_deriving_partial_ord(
     };
 
     // avoid defining extra methods if we can
-    // c-like enums, enums without any fields and structs without fields
-    // can safely define only `partial_cmp`.
-    let methods = if is_type_without_fields(item) {
+    // c-like enums, enums without any fields, structs without fields,
+    // as well as types that derive `PartialEq` can safely define only `partial_cmp`.
+    let container_id = cx.current_expansion.id.expn_data().parent;
+    let methods = if is_type_without_fields(item)
+        || cx.resolver.has_derive(container_id, BuiltinDerive::PartialEq)
+    {
         vec![partial_cmp_def]
     } else {
         vec![

compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

@@ -186,7 +186,7 @@ use rustc_ast::{self as ast, BinOpKind, EnumDef, Expr, Generics, PatKind};
 use rustc_ast::{GenericArg, GenericParamKind, VariantData};
 use rustc_attr as attr;
 use rustc_data_structures::map_in_place::MapInPlace;
-use rustc_expand::base::{Annotatable, ExtCtxt};
+use rustc_expand::base::{Annotatable, BuiltinDerive, ExtCtxt};
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::Span;
 
@@ -410,7 +410,8 @@ impl<'a> TraitDef<'a> {
                     _ => unreachable!(),
                 };
                 let container_id = cx.current_expansion.id.expn_data().parent;
-                let always_copy = has_no_type_params && cx.resolver.has_derive_copy(container_id);
+                let always_copy =
+                    has_no_type_params && cx.resolver.has_derive(container_id, BuiltinDerive::Copy);
                 let use_temporaries = is_packed && always_copy;
 
                 let newitem = match item.kind {

compiler/rustc_expand/src/base.rs

@@ -726,8 +726,8 @@ pub struct SyntaxExtension {
     /// Built-in macros have a couple of special properties like 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,
+    /// Keeps track of which builtin derives (if any) this is
+    pub derive: Option<BuiltinDerive>,
 }
 
 impl SyntaxExtension {
@@ -756,7 +756,7 @@ impl SyntaxExtension {
             helper_attrs: Vec::new(),
             edition,
             is_builtin: false,
-            is_derive_copy: false,
+            derive: None,
             kind,
         }
     }
@@ -789,6 +789,7 @@ impl SyntaxExtension {
                 .span_diagnostic
                 .span_err(span, "macros cannot have const stability attributes");
         }
+        let derive = if is_builtin { BuiltinDerive::from_name(name) } else { None };
 
         SyntaxExtension {
             kind,
@@ -801,7 +802,7 @@ impl SyntaxExtension {
             helper_attrs,
             edition,
             is_builtin,
-            is_derive_copy: is_builtin && name == sym::Copy,
+            derive,
         }
     }
 
@@ -855,6 +856,25 @@ impl SyntaxExtension {
     }
 }
 
+/// Built-in derives
+/// Only derives that are being used for special casing expansion are represented here
+#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)]
+pub enum BuiltinDerive {
+    Copy,
+    PartialEq,
+}
+
+impl BuiltinDerive {
+    /// Possibly turn a symbol into a built-in derive
+    fn from_name(name: Symbol) -> Option<Self> {
+        match name {
+            sym::Copy => Some(Self::Copy),
+            sym::PartialEq => Some(Self::PartialEq),
+            _ => None,
+        }
+    }
+}
+
 /// Result of resolving a macro invocation.
 pub enum InvocationRes {
     Single(Lrc<SyntaxExtension>),
@@ -894,8 +914,8 @@ pub trait ResolverExpand {
     fn lint_node_id(&mut self, expn_id: ExpnId) -> NodeId;
 
     // Resolver interfaces for specific built-in macros.
-    /// Does `#[derive(...)]` attribute with the given `ExpnId` have built-in `Copy` inside it?
-    fn has_derive_copy(&self, expn_id: ExpnId) -> bool;
+    /// Does `#[derive(...)]` attribute with the given `ExpnId` have the given built-in derive inside it?
+    fn has_derive(&self, expn_id: ExpnId, derive: BuiltinDerive) -> bool;
     /// Path resolution logic for `#[cfg_accessible(path)]`.
     fn cfg_accessible(&mut self, expn_id: ExpnId, path: &ast::Path) -> Result<bool, Indeterminate>;
 }

compiler/rustc_resolve/src/lib.rs

@@ -33,7 +33,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;
+use rustc_expand::base::{BuiltinDerive, SyntaxExtension};
 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};
@@ -967,10 +967,10 @@ pub struct Resolver<'a> {
     multi_segment_macro_resolutions:
         Vec<(Vec<Segment>, Span, MacroKind, ParentScope<'a>, Option<Res>)>,
     builtin_attrs: Vec<(Ident, ParentScope<'a>)>,
-    /// `derive(Copy)` marks items they are applied to so they are treated specially later.
+    /// 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.
-    containers_deriving_copy: FxHashSet<ExpnId>,
+    containers_derives: FxHashMap<ExpnId, FxHashSet<BuiltinDerive>>,
     /// Parent scopes in which the macros were invoked.
     /// FIXME: `derives` are missing in these parent scopes and need to be taken from elsewhere.
     invocation_parent_scopes: FxHashMap<ExpnId, ParentScope<'a>>,
@@ -1318,7 +1318,7 @@ impl<'a> Resolver<'a> {
             single_segment_macro_resolutions: Default::default(),
             multi_segment_macro_resolutions: Default::default(),
             builtin_attrs: Default::default(),
-            containers_deriving_copy: Default::default(),
+            containers_derives: Default::default(),
             active_features: features
                 .declared_lib_features
                 .iter()

compiler/rustc_resolve/src/macros.rs

@@ -14,7 +14,9 @@ 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::{Indeterminate, InvocationRes, ResolverExpand, SyntaxExtension};
+use rustc_expand::base::{
+    BuiltinDerive, Indeterminate, InvocationRes, ResolverExpand, SyntaxExtension,
+};
 use rustc_expand::compile_declarative_macro;
 use rustc_expand::expand::{AstFragment, Invocation, InvocationKind};
 use rustc_feature::is_builtin_attr_name;
@@ -285,8 +287,11 @@ impl<'a> ResolverExpand for Resolver<'a> {
                                 helper_attrs.extend(
                                     ext.helper_attrs.iter().map(|name| Ident::new(*name, span)),
                                 );
-                                if ext.is_derive_copy {
-                                    self.containers_deriving_copy.insert(invoc_id);
+                                if let Some(derive) = ext.derive {
+                                    self.containers_derives
+                                        .entry(invoc_id)
+                                        .or_default()
+                                        .insert(derive);
                                 }
                                 ext
                             }
@@ -347,8 +352,8 @@ impl<'a> ResolverExpand for Resolver<'a> {
             .map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[*id])
     }
 
-    fn has_derive_copy(&self, expn_id: ExpnId) -> bool {
-        self.containers_deriving_copy.contains(&expn_id)
+    fn has_derive(&self, expn_id: ExpnId, derive: BuiltinDerive) -> bool {
+        self.containers_derives.get(&expn_id).map(|d| d.contains(&derive)).unwrap_or(false)
     }
 
     // The function that implements the resolution logic of `#[cfg_accessible(path)]`.