Skip to content

Improve code generation of Ord and PartialOrd #38

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 22, 2022
Merged

Improve code generation of Ord and PartialOrd #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2e54f7
Improve code generation of `Ord` and `PartialOrd` for the first and l…
daxpedda Jan 21, 2022
94d0008
Improve code generation of `Ord` and `PartialOrd` for variants betwee…
daxpedda Jan 21, 2022
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 src/data.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,9 @@ impl<'a> Data<'a> {

/// Returns the destructuring `other` pattern of this [`Data`].
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
pub fn other_pattern(&self) -> &Pat {
pub fn other_pattern_skip(&self) -> &Pat {
match self.fields() {
Either::Left(fields) => &fields.other_pattern,
Either::Left(fields) => &fields.other_pattern_skip,
Either::Right(pattern) => pattern,
}
}
Expand Down
32 changes: 32 additions & 0 deletions src/data/fields.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use syn::{
FieldPat, FieldsNamed, FieldsUnnamed, Ident, Pat, PatIdent, PatStruct, PatTuple,
PatTupleStruct, Path, Result, Token,
};
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
use {std::iter, syn::punctuated::Punctuated, syn::PatRest};

use crate::{DeriveWhere, Field, Skip, Trait};

Expand All @@ -15,6 +17,10 @@ pub struct Fields<'a> {
pub self_pattern: Pat,
/// [Pattern](Pat) to use in a match arm to destructure `other`.
pub other_pattern: Pat,
/// [Pattern](Pat) to use in a match arm to destructure `other` but skipping
/// all fields.
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
pub other_pattern_skip: Pat,
/// [`Field`]s of this struct, union or variant.
pub fields: Vec<Field<'a>>,
}
Expand All @@ -30,11 +36,21 @@ impl<'a> Fields<'a> {
let fields = Field::from_named(derive_wheres, skip_inner, fields)?;

let self_pattern = Self::struct_pattern(path.clone(), &fields, |field| &field.self_ident);
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
let other_pattern_skip = Pat::Struct(PatStruct {
attrs: Vec::new(),
path: path.clone(),
brace_token: Brace::default(),
fields: Punctuated::new(),
dot2_token: Some(<Token![..]>::default()),
});
let other_pattern = Self::struct_pattern(path, &fields, |field| &field.other_ident);

Ok(Self {
self_pattern,
other_pattern,
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
other_pattern_skip,
fields,
})
}
Expand All @@ -49,11 +65,27 @@ impl<'a> Fields<'a> {
let fields = Field::from_unnamed(derive_wheres, skip_inner, fields)?;

let self_pattern = Self::tuple_pattern(path.clone(), &fields, |field| &field.self_ident);
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
let other_pattern_skip = Pat::TupleStruct(PatTupleStruct {
attrs: Vec::new(),
path: path.clone(),
pat: PatTuple {
attrs: Vec::new(),
paren_token: Paren::default(),
elems: iter::once(Pat::Rest(PatRest {
attrs: Vec::new(),
dot2_token: <Token![..]>::default(),
}))
.collect(),
},
});
let other_pattern = Self::tuple_pattern(path, &fields, |field| &field.other_ident);

Ok(Self {
self_pattern,
other_pattern,
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
other_pattern_skip,
fields,
})
}
Expand Down
78 changes: 16 additions & 62 deletions src/test/basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,46 +220,23 @@ fn enum_() -> Result<()> {
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
let ord = quote! {
match self {
Test::A { field: ref __field } =>
match __other {
Test::B { } => ::core::cmp::Ordering::Less,
Test::C(ref __other_0) => ::core::cmp::Ordering::Less,
Test::D() => ::core::cmp::Ordering::Less,
Test::E => ::core::cmp::Ordering::Less,
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::A { field: ref __field } => ::core::cmp::Ordering::Less,
Test::B { } =>
match __other {
Test::A { field: ref __other_field } => ::core::cmp::Ordering::Greater,
Test::C(ref __other_0) => ::core::cmp::Ordering::Less,
Test::D() => ::core::cmp::Ordering::Less,
Test::E => ::core::cmp::Ordering::Less,
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
Test::A { .. } => ::core::cmp::Ordering::Greater,
_ => ::core::cmp::Ordering::Less,
},
Test::C(ref __0) =>
match __other {
Test::A { field: ref __other_field } => ::core::cmp::Ordering::Greater,
Test::B { } => ::core::cmp::Ordering::Greater,
Test::D() => ::core::cmp::Ordering::Less,
Test::E => ::core::cmp::Ordering::Less,
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
Test::A { .. } | Test::B { .. } => ::core::cmp::Ordering::Greater,
_ => ::core::cmp::Ordering::Less,
},
Test::D() =>
match __other {
Test::A { field: ref __other_field } => ::core::cmp::Ordering::Greater,
Test::B { } => ::core::cmp::Ordering::Greater,
Test::C(ref __other_0) => ::core::cmp::Ordering::Greater,
Test::E => ::core::cmp::Ordering::Less,
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::E =>
match __other {
Test::A { field: ref __other_field } => ::core::cmp::Ordering::Greater,
Test::B { } => ::core::cmp::Ordering::Greater,
Test::C(ref __other_0) => ::core::cmp::Ordering::Greater,
Test::D() => ::core::cmp::Ordering::Greater,
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
Test::A { .. } | Test::B { .. } | Test::C(..) => ::core::cmp::Ordering::Greater,
_ => ::core::cmp::Ordering::Less,
},
Test::E => ::core::cmp::Ordering::Greater,
}
};
#[cfg(feature = "nightly")]
Expand All @@ -276,46 +253,23 @@ fn enum_() -> Result<()> {
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
let partial_ord = quote! {
match self {
Test::A { field: ref __field } =>
match __other {
Test::B { } => ::core::option::Option::Some(::core::cmp::Ordering::Less),
Test::C(ref __other_0) => ::core::option::Option::Some(::core::cmp::Ordering::Less),
Test::D() => ::core::option::Option::Some(::core::cmp::Ordering::Less),
Test::E => ::core::option::Option::Some(::core::cmp::Ordering::Less),
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::A { field: ref __field } => ::core::option::Option::Some(::core::cmp::Ordering::Less),
Test::B { } =>
match __other {
Test::A { field: ref __other_field } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
Test::C(ref __other_0) => ::core::option::Option::Some(::core::cmp::Ordering::Less),
Test::D() => ::core::option::Option::Some(::core::cmp::Ordering::Less),
Test::E => ::core::option::Option::Some(::core::cmp::Ordering::Less),
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
Test::A { .. } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
_ => ::core::option::Option::Some(::core::cmp::Ordering::Less),
},
Test::C(ref __0) =>
match __other {
Test::A { field: ref __other_field } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
Test::B { } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
Test::D() => ::core::option::Option::Some(::core::cmp::Ordering::Less),
Test::E => ::core::option::Option::Some(::core::cmp::Ordering::Less),
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
Test::A { .. } | Test::B { .. } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
_ => ::core::option::Option::Some(::core::cmp::Ordering::Less),
},
Test::D() =>
match __other {
Test::A { field: ref __other_field } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
Test::B { } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
Test::C(ref __other_0) => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
Test::E => ::core::option::Option::Some(::core::cmp::Ordering::Less),
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::E =>
match __other {
Test::A { field: ref __other_field } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
Test::B { } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
Test::C(ref __other_0) => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
Test::D() => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
Test::A { .. } | Test::B { .. } | Test::C(..) => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
_ => ::core::option::Option::Some(::core::cmp::Ordering::Less),
},
Test::E => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
}
};

Expand Down
48 changes: 8 additions & 40 deletions src/test/enum_.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,8 @@ fn two_data() -> Result<()> {
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
let partial_ord = quote! {
match self {
Test::A(ref __0) =>
match __other {
Test::B(ref __other_0) => ::core::option::Option::Some(::core::cmp::Ordering::Less),
_ => #unreachable,
},
Test::B(ref __0) =>
match __other {
Test::A(ref __other_0) => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
_ => #unreachable,
},
Test::A(ref __0) => ::core::option::Option::Some(::core::cmp::Ordering::Less),
Test::B(ref __0) => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
}
};

Expand Down Expand Up @@ -228,16 +220,8 @@ fn unit() -> Result<()> {
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
let partial_ord = quote! {
match self {
Test::A(ref __0) =>
match __other {
Test::B => ::core::option::Option::Some(::core::cmp::Ordering::Less),
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::B =>
match __other {
Test::A(ref __other_0) => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::A(ref __0) => ::core::option::Option::Some(::core::cmp::Ordering::Less),
Test::B => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
}
};

Expand Down Expand Up @@ -313,16 +297,8 @@ fn struct_unit() -> Result<()> {
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
let partial_ord = quote! {
match self {
Test::A(ref __0) =>
match __other {
Test::B { } => ::core::option::Option::Some(::core::cmp::Ordering::Less),
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::B { } =>
match __other {
Test::A(ref __other_0) => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::A(ref __0) => ::core::option::Option::Some(::core::cmp::Ordering::Less),
Test::B { } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
}
};

Expand Down Expand Up @@ -398,16 +374,8 @@ fn tuple_unit() -> Result<()> {
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
let partial_ord = quote! {
match self {
Test::A(ref __0) =>
match __other {
Test::B() => ::core::option::Option::Some(::core::cmp::Ordering::Less),
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::B() =>
match __other {
Test::A(ref __other_0) => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::A(ref __0) => ::core::option::Option::Some(::core::cmp::Ordering::Less),
Test::B() => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
}
};

Expand Down
24 changes: 4 additions & 20 deletions src/test/skip.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,8 @@ fn variants_empty() -> Result<()> {
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
let ord = quote! {
match self {
Test::A(ref __0) =>
match __other {
Test::B (ref __other_0) => ::core::cmp::Ordering::Less,
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::B(ref __0) =>
match __other {
Test::A (ref __other_0) => ::core::cmp::Ordering::Greater,
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::A(ref __0) => ::core::cmp::Ordering::Less,
Test::B(ref __0) => ::core::cmp::Ordering::Greater,
}
};

Expand Down Expand Up @@ -190,16 +182,8 @@ fn variants_partly_empty() -> Result<()> {
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
let ord = quote! {
match self {
Test::A(ref __0) =>
match __other {
Test::B (ref __other_0, ref __other_1) => ::core::cmp::Ordering::Less,
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::B(ref __0, ref __1) =>
match __other {
Test::A (ref __other_0) => ::core::cmp::Ordering::Greater,
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
Test::A(ref __0) => ::core::cmp::Ordering::Less,
Test::B(ref __0, ref __1) => ::core::cmp::Ordering::Greater,
}
};

Expand Down
66 changes: 39 additions & 27 deletions src/trait_/common_ord.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ pub fn build_ord_signature(item: &Item, trait_: &DeriveTrait, body: &TokenStream
// Safe implementation when not on nightly.
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
{
use syn::PatOr;

let mut less = quote! { ::core::cmp::Ordering::Less };
let mut greater = quote! { ::core::cmp::Ordering::Greater };

Expand All @@ -105,35 +107,45 @@ pub fn build_ord_signature(item: &Item, trait_: &DeriveTrait, body: &TokenStream
// other. The index for these variants is used to determine which
// `Ordering` to return.
for (index, variant) in variants.iter().enumerate() {
let mut arms = Vec::with_capacity(variants.len() - 1);

for (index_other, variant_other) in variants.iter().enumerate() {
// Make sure we aren't comparing the same variant with itself.
if index != index_other {
use std::cmp::Ordering::*;

let ordering = match index.cmp(&index_other) {
Less => &less,
Equal => &equal,
Greater => &greater,
};

let pattern = &variant_other.other_pattern();

arms.push(quote! {
#pattern => #ordering,
});
}
}

let pattern = &variant.self_pattern();

different.push(quote! {
#pattern => match __other {
#(#arms)*
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
},
});
// The first variant is always `Less` then everything.
if index == 0 {
different.push(quote! {
#pattern => #less,
})
}
// The last variant is always `Greater` then everything.
else if index == variants.len() - 1 {
different.push(quote! {
#pattern => #greater,
})
}
// Any variant between the first and last.
else {
// Collect all variants that are `Less`.
let cases = variants
.iter()
.enumerate()
.filter(|(index_other, _)| *index_other < index)
.map(|(_, variant_other)| variant_other.other_pattern_skip().clone())
.collect();

// Build one match arm pattern with all variants that are `Greater`.
let pattern_less = PatOr {
attrs: Vec::new(),
leading_vert: None,
cases,
};

// All other variants are `Less`.
different.push(quote! {
#pattern => match __other {
#pattern_less => #greater,
_ => #less,
},
});
}
}

quote! {
Expand Down