Skip to content

Commit

Permalink
Fix PartialOrd implementations when using bounds
Browse files Browse the repository at this point in the history
  • Loading branch information
daxpedda committed Aug 23, 2023
1 parent 0930cb0 commit c8205c3
Show file tree
Hide file tree
Showing 15 changed files with 101 additions and 15 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Fixed
- Don't use `Ord` in `PartialOrd` implementations if using any bounds.

## [1.2.2] - 2023-08-22

### Fixed
Expand Down
19 changes: 13 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ fn generate_impl(
let mut where_clause = where_clause.map(Cow::Borrowed);
derive_where.where_clause(&mut where_clause, trait_, item);

let body = generate_body(&derive_where.traits, trait_, item);
let body = generate_body(derive_where, &derive_where.traits, trait_, item);

let ident = item.ident();
let path = trait_.impl_path(trait_);
Expand All @@ -654,19 +654,26 @@ fn generate_impl(
}

/// Generate implementation method body for a [`Trait`].
fn generate_body(traits: &[DeriveTrait], trait_: &DeriveTrait, item: &Item) -> TokenStream {
fn generate_body(
derive_where: &DeriveWhere,
traits: &[DeriveTrait],
trait_: &DeriveTrait,
item: &Item,
) -> TokenStream {
let any_bound = !derive_where.generics.is_empty();

match &item {
Item::Item(data) => {
let body = trait_.build_body(traits, trait_, data);
trait_.build_signature(item, traits, trait_, &body)
let body = trait_.build_body(any_bound, traits, trait_, data);
trait_.build_signature(any_bound, item, traits, trait_, &body)
}
Item::Enum { variants, .. } => {
let body: TokenStream = variants
.iter()
.map(|data| trait_.build_body(traits, trait_, data))
.map(|data| trait_.build_body(any_bound, traits, trait_, data))
.collect();

trait_.build_signature(item, traits, trait_, &body)
trait_.build_signature(any_bound, item, traits, trait_, &body)
}
}
}
Expand Down
22 changes: 20 additions & 2 deletions src/test/bound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,16 @@ fn check_trait_bounds() -> Result<()> {
{
#[inline]
fn partial_cmp(&self, __other: &Self) -> ::core::option::Option<::core::cmp::Ordering> {
::core::option::Option::Some(::core::cmp::Ord::cmp(self, __other))
match (self, __other) {
(Test(ref __field_0, ref __field_1), Test(ref __other_field_0, ref __other_field_1)) =>
match ::core::cmp::PartialOrd::partial_cmp(__field_0, __other_field_0) {
::core::option::Option::Some(::core::cmp::Ordering::Equal) => match ::core::cmp::PartialOrd::partial_cmp(__field_1, __other_field_1) {
::core::option::Option::Some(::core::cmp::Ordering::Equal) => ::core::option::Option::Some(::core::cmp::Ordering::Equal),
__cmp => __cmp,
},
__cmp => __cmp,
},
}
}
}
},
Expand Down Expand Up @@ -378,7 +387,16 @@ fn check_multiple_trait_bounds() -> Result<()> {
{
#[inline]
fn partial_cmp(&self, __other: &Self) -> ::core::option::Option<::core::cmp::Ordering> {
::core::option::Option::Some(::core::cmp::Ord::cmp(self, __other))
match (self, __other) {
(Test(ref __field_0, ref __field_1), Test(ref __other_field_0, ref __other_field_1)) =>
match ::core::cmp::PartialOrd::partial_cmp(__field_0, __other_field_0) {
::core::option::Option::Some(::core::cmp::Ordering::Equal) => match ::core::cmp::PartialOrd::partial_cmp(__field_1, __other_field_1) {
::core::option::Option::Some(::core::cmp::Ordering::Equal) => ::core::option::Option::Some(::core::cmp::Ordering::Equal),
__cmp => __cmp,
},
__cmp => __cmp,
},
}
}
}
},
Expand Down
16 changes: 13 additions & 3 deletions src/trait_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,17 +133,25 @@ impl TraitImpl for Trait {

fn build_signature(
&self,
any_bound: bool,
item: &Item,
traits: &[DeriveTrait],
trait_: &DeriveTrait,
body: &TokenStream,
) -> TokenStream {
self.implementation()
.build_signature(item, traits, trait_, body)
.build_signature(any_bound, item, traits, trait_, body)
}

fn build_body(&self, traits: &[DeriveTrait], trait_: &DeriveTrait, data: &Data) -> TokenStream {
self.implementation().build_body(traits, trait_, data)
fn build_body(
&self,
any_bound: bool,
traits: &[DeriveTrait],
trait_: &DeriveTrait,
data: &Data,
) -> TokenStream {
self.implementation()
.build_body(any_bound, traits, trait_, data)
}
}

Expand Down Expand Up @@ -190,6 +198,7 @@ pub trait TraitImpl {
/// Build method signature for this [`Trait`].
fn build_signature(
&self,
_any_bound: bool,
_item: &Item,
_traits: &[DeriveTrait],
_trait_: &DeriveTrait,
Expand All @@ -201,6 +210,7 @@ pub trait TraitImpl {
/// Build method body for this [`Trait`].
fn build_body(
&self,
_any_bound: bool,
_traits: &[DeriveTrait],
_trait_: &DeriveTrait,
_data: &Data,
Expand Down
9 changes: 8 additions & 1 deletion src/trait_/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ impl TraitImpl for Clone {

fn build_signature(
&self,
_any_bound: bool,
item: &Item,
traits: &[DeriveTrait],
_trait_: &DeriveTrait,
Expand Down Expand Up @@ -81,7 +82,13 @@ impl TraitImpl for Clone {
}
}

fn build_body(&self, traits: &[DeriveTrait], trait_: &DeriveTrait, data: &Data) -> TokenStream {
fn build_body(
&self,
_any_bound: bool,
traits: &[DeriveTrait],
trait_: &DeriveTrait,
data: &Data,
) -> TokenStream {
if traits.iter().any(|trait_| trait_ == Trait::Copy) {
return TokenStream::new();
}
Expand Down
2 changes: 2 additions & 0 deletions src/trait_/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ impl TraitImpl for Debug {

fn build_signature(
&self,
_any_bound: bool,
_item: &Item,
_traits: &[DeriveTrait],
_trait_: &DeriveTrait,
Expand All @@ -36,6 +37,7 @@ impl TraitImpl for Debug {

fn build_body(
&self,
_any_bound: bool,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
data: &Data,
Expand Down
2 changes: 2 additions & 0 deletions src/trait_/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ impl TraitImpl for Default {

fn build_signature(
&self,
_any_bound: bool,
_item: &Item,
_traits: &[DeriveTrait],
_trait_: &DeriveTrait,
Expand All @@ -34,6 +35,7 @@ impl TraitImpl for Default {

fn build_body(
&self,
_any_bound: bool,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
data: &Data,
Expand Down
2 changes: 2 additions & 0 deletions src/trait_/eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ impl TraitImpl for Eq {

fn build_signature(
&self,
_any_bound: bool,
_item: &Item,
_traits: &[DeriveTrait],
_trait_: &DeriveTrait,
Expand All @@ -37,6 +38,7 @@ impl TraitImpl for Eq {

fn build_body(
&self,
_any_bound: bool,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
data: &Data,
Expand Down
2 changes: 2 additions & 0 deletions src/trait_/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ impl TraitImpl for Hash {

fn build_signature(
&self,
_any_bound: bool,
_item: &Item,
_traits: &[DeriveTrait],
_trait_: &DeriveTrait,
Expand All @@ -36,6 +37,7 @@ impl TraitImpl for Hash {

fn build_body(
&self,
_any_bound: bool,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
data: &Data,
Expand Down
2 changes: 2 additions & 0 deletions src/trait_/ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ impl TraitImpl for Ord {

fn build_signature(
&self,
_any_bound: bool,
item: &Item,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
Expand All @@ -38,6 +39,7 @@ impl TraitImpl for Ord {

fn build_body(
&self,
_any_bound: bool,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
data: &Data,
Expand Down
2 changes: 2 additions & 0 deletions src/trait_/partial_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ impl TraitImpl for PartialEq {

fn build_signature(
&self,
_any_bound: bool,
item: &Item,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
Expand Down Expand Up @@ -105,6 +106,7 @@ impl TraitImpl for PartialEq {

fn build_body(
&self,
_any_bound: bool,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
data: &Data,
Expand Down
13 changes: 10 additions & 3 deletions src/trait_/partial_ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ impl TraitImpl for PartialOrd {

fn build_signature(
&self,
any_bound: bool,
item: &Item,
traits: &[DeriveTrait],
trait_: &DeriveTrait,
body: &TokenStream,
) -> TokenStream {
let body = if traits.iter().any(|trait_| trait_ == Trait::Ord) {
let body = if !any_bound && traits.iter().any(|trait_| trait_ == Trait::Ord) {
quote! {
::core::option::Option::Some(::core::cmp::Ord::cmp(self, __other))
}
Expand All @@ -42,10 +43,16 @@ impl TraitImpl for PartialOrd {
}
}

fn build_body(&self, traits: &[DeriveTrait], trait_: &DeriveTrait, data: &Data) -> TokenStream {
fn build_body(
&self,
any_bound: bool,
traits: &[DeriveTrait],
trait_: &DeriveTrait,
data: &Data,
) -> TokenStream {
if data.is_empty(**trait_)
|| data.is_incomparable()
|| traits.iter().any(|trait_| trait_ == Trait::Ord)
|| (!any_bound && traits.iter().any(|trait_| trait_ == Trait::Ord))
{
TokenStream::new()
} else {
Expand Down
2 changes: 2 additions & 0 deletions src/trait_/zeroize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ impl TraitImpl for Zeroize {

fn build_signature(
&self,
_any_bound: bool,
item: &Item,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
Expand All @@ -105,6 +106,7 @@ impl TraitImpl for Zeroize {

fn build_body(
&self,
_any_bound: bool,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
data: &Data,
Expand Down
2 changes: 2 additions & 0 deletions src/trait_/zeroize_on_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ impl TraitImpl for ZeroizeOnDrop {

fn build_signature(
&self,
_any_bound: bool,
item: &Item,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
Expand Down Expand Up @@ -133,6 +134,7 @@ impl TraitImpl for ZeroizeOnDrop {

fn build_body(
&self,
_any_bound: bool,
_traits: &[DeriveTrait],
trait_: &DeriveTrait,
data: &Data,
Expand Down
16 changes: 16 additions & 0 deletions tests/bound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,19 @@ fn custom_bound() {
let test_clone = test_1.clone();
assert_eq!(test_clone.0, 42);
}

#[test]
fn ord_requirement() {
trait Trait {
type Type;
}

struct Impl;

impl Trait for Impl {
type Type = i32;
}

#[derive_where(Eq, Ord, PartialEq, PartialOrd; T::Type)]
struct Test<T: Trait>(T::Type);
}

0 comments on commit c8205c3

Please sign in to comment.