Skip to content

Only derive PartialOrd::partial_cmp when also deriving PartialEq #80050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_builtin_macros/src/deriving/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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![
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down
32 changes: 26 additions & 6 deletions compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -756,7 +756,7 @@ impl SyntaxExtension {
helper_attrs: Vec::new(),
edition,
is_builtin: false,
is_derive_copy: false,
derive: None,
kind,
}
}
Expand Down Expand Up @@ -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,
Expand All @@ -801,7 +802,7 @@ impl SyntaxExtension {
helper_attrs,
edition,
is_builtin,
is_derive_copy: is_builtin && name == sym::Copy,
derive,
}
}

Expand Down Expand Up @@ -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,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually had this enum in the past, but it was represented as bitflags.
See #65892 where it was removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow, I didn't realize! I was considering using bitflags, but I didn't want to add a dependency to rustc_expand. I can make this change if we don't close the PR.


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>),
Expand Down Expand Up @@ -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>;
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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>>,
Expand Down Expand Up @@ -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()
Expand Down
15 changes: 10 additions & 5 deletions compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)]`.
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/derives/derives-span-Hash-enum.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// This file was auto-generated using 'src/etc/generate-deriving-span-tests.py'


struct Error;

#[derive(Hash)]
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/derives/derives-span-Hash-enum.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0277]: the trait bound `Error: Hash` is not satisfied
--> $DIR/derives-span-Hash-enum.rs:8:6
--> $DIR/derives-span-Hash-enum.rs:9:6
|
LL | Error
| ^^^^^ the trait `Hash` is not implemented for `Error`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@ struct Error;
#[derive(PartialOrd,PartialEq)]
enum Enum {
A {
x: Error //~ ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
x: Error //~ ERROR
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,46 +8,6 @@ LL | x: Error
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-enum-struct-variant.rs:9:6
|
LL | x: Error
| ^^^^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-enum-struct-variant.rs:9:6
|
LL | x: Error
| ^^^^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-enum-struct-variant.rs:9:6
|
LL | x: Error
| ^^^^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-enum-struct-variant.rs:9:6
|
LL | x: Error
| ^^^^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 5 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
6 changes: 1 addition & 5 deletions src/test/ui/derives/derives-span-PartialOrd-enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@ struct Error;
#[derive(PartialOrd,PartialEq)]
enum Enum {
A(
Error //~ ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
Error //~ ERROR
)
}

Expand Down
42 changes: 1 addition & 41 deletions src/test/ui/derives/derives-span-PartialOrd-enum.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,46 +8,6 @@ LL | Error
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-enum.rs:9:6
|
LL | Error
| ^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-enum.rs:9:6
|
LL | Error
| ^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-enum.rs:9:6
|
LL | Error
| ^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-enum.rs:9:6
|
LL | Error
| ^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 5 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
6 changes: 1 addition & 5 deletions src/test/ui/derives/derives-span-PartialOrd-struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@ struct Error;

#[derive(PartialOrd,PartialEq)]
struct Struct {
x: Error //~ ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
//~| ERROR can't compare `Error` with `Error`
x: Error //~ ERROR
}

fn main() {}
42 changes: 1 addition & 41 deletions src/test/ui/derives/derives-span-PartialOrd-struct.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,46 +8,6 @@ LL | x: Error
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-struct.rs:8:5
|
LL | x: Error
| ^^^^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-struct.rs:8:5
|
LL | x: Error
| ^^^^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-struct.rs:8:5
|
LL | x: Error
| ^^^^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Error` with `Error`
--> $DIR/derives-span-PartialOrd-struct.rs:8:5
|
LL | x: Error
| ^^^^^^^^ no implementation for `Error < Error` and `Error > Error`
|
= help: the trait `PartialOrd` is not implemented for `Error`
= note: required by `std::cmp::PartialOrd::partial_cmp`
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 5 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
Loading