Skip to content

Commit 94d0008

Browse files
daxpeddadaxpedda
committed
Improve code generation of Ord and PartialOrd for variants between the first and last
1 parent e2e54f7 commit 94d0008

File tree

4 files changed

+69
-56
lines changed

4 files changed

+69
-56
lines changed

‎src/data.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,9 @@ impl<'a> Data<'a> {
231231

232232
/// Returns the destructuring `other` pattern of this [`Data`].
233233
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
234-
pub fn other_pattern(&self) -> &Pat {
234+
pub fn other_pattern_skip(&self) -> &Pat {
235235
match self.fields() {
236-
Either::Left(fields) => &fields.other_pattern,
236+
Either::Left(fields) => &fields.other_pattern_skip,
237237
Either::Right(pattern) => pattern,
238238
}
239239
}

‎src/data/fields.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use syn::{
55
FieldPat, FieldsNamed, FieldsUnnamed, Ident, Pat, PatIdent, PatStruct, PatTuple,
66
PatTupleStruct, Path, Result, Token,
77
};
8+
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
9+
use {std::iter, syn::punctuated::Punctuated, syn::PatRest};
810

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

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

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

3549
Ok(Self {
3650
self_pattern,
3751
other_pattern,
52+
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
53+
other_pattern_skip,
3854
fields,
3955
})
4056
}
@@ -49,11 +65,27 @@ impl<'a> Fields<'a> {
4965
let fields = Field::from_unnamed(derive_wheres, skip_inner, fields)?;
5066

5167
let self_pattern = Self::tuple_pattern(path.clone(), &fields, |field| &field.self_ident);
68+
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
69+
let other_pattern_skip = Pat::TupleStruct(PatTupleStruct {
70+
attrs: Vec::new(),
71+
path: path.clone(),
72+
pat: PatTuple {
73+
attrs: Vec::new(),
74+
paren_token: Paren::default(),
75+
elems: iter::once(Pat::Rest(PatRest {
76+
attrs: Vec::new(),
77+
dot2_token: <Token![..]>::default(),
78+
}))
79+
.collect(),
80+
},
81+
});
5282
let other_pattern = Self::tuple_pattern(path, &fields, |field| &field.other_ident);
5383

5484
Ok(Self {
5585
self_pattern,
5686
other_pattern,
87+
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
88+
other_pattern_skip,
5789
fields,
5890
})
5991
}

‎src/test/basic.rs

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -223,27 +223,18 @@ fn enum_() -> Result<()> {
223223
Test::A { field: ref __field } => ::core::cmp::Ordering::Less,
224224
Test::B { } =>
225225
match __other {
226-
Test::A { field: ref __other_field } => ::core::cmp::Ordering::Greater,
227-
Test::C(ref __other_0) => ::core::cmp::Ordering::Less,
228-
Test::D() => ::core::cmp::Ordering::Less,
229-
Test::E => ::core::cmp::Ordering::Less,
230-
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
226+
Test::A { .. } => ::core::cmp::Ordering::Greater,
227+
_ => ::core::cmp::Ordering::Less,
231228
},
232229
Test::C(ref __0) =>
233230
match __other {
234-
Test::A { field: ref __other_field } => ::core::cmp::Ordering::Greater,
235-
Test::B { } => ::core::cmp::Ordering::Greater,
236-
Test::D() => ::core::cmp::Ordering::Less,
237-
Test::E => ::core::cmp::Ordering::Less,
238-
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
231+
Test::A { .. } | Test::B { .. } => ::core::cmp::Ordering::Greater,
232+
_ => ::core::cmp::Ordering::Less,
239233
},
240234
Test::D() =>
241235
match __other {
242-
Test::A { field: ref __other_field } => ::core::cmp::Ordering::Greater,
243-
Test::B { } => ::core::cmp::Ordering::Greater,
244-
Test::C(ref __other_0) => ::core::cmp::Ordering::Greater,
245-
Test::E => ::core::cmp::Ordering::Less,
246-
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
236+
Test::A { .. } | Test::B { .. } | Test::C(..) => ::core::cmp::Ordering::Greater,
237+
_ => ::core::cmp::Ordering::Less,
247238
},
248239
Test::E => ::core::cmp::Ordering::Greater,
249240
}
@@ -265,27 +256,18 @@ fn enum_() -> Result<()> {
265256
Test::A { field: ref __field } => ::core::option::Option::Some(::core::cmp::Ordering::Less),
266257
Test::B { } =>
267258
match __other {
268-
Test::A { field: ref __other_field } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
269-
Test::C(ref __other_0) => ::core::option::Option::Some(::core::cmp::Ordering::Less),
270-
Test::D() => ::core::option::Option::Some(::core::cmp::Ordering::Less),
271-
Test::E => ::core::option::Option::Some(::core::cmp::Ordering::Less),
272-
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
259+
Test::A { .. } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
260+
_ => ::core::option::Option::Some(::core::cmp::Ordering::Less),
273261
},
274262
Test::C(ref __0) =>
275263
match __other {
276-
Test::A { field: ref __other_field } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
277-
Test::B { } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
278-
Test::D() => ::core::option::Option::Some(::core::cmp::Ordering::Less),
279-
Test::E => ::core::option::Option::Some(::core::cmp::Ordering::Less),
280-
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
264+
Test::A { .. } | Test::B { .. } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
265+
_ => ::core::option::Option::Some(::core::cmp::Ordering::Less),
281266
},
282267
Test::D() =>
283268
match __other {
284-
Test::A { field: ref __other_field } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
285-
Test::B { } => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
286-
Test::C(ref __other_0) => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
287-
Test::E => ::core::option::Option::Some(::core::cmp::Ordering::Less),
288-
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
269+
Test::A { .. } | Test::B { .. } | Test::C(..) => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
270+
_ => ::core::option::Option::Some(::core::cmp::Ordering::Less),
289271
},
290272
Test::E => ::core::option::Option::Some(::core::cmp::Ordering::Greater),
291273
}

‎src/trait_/common_ord.rs

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ pub fn build_ord_signature(item: &Item, trait_: &DeriveTrait, body: &TokenStream
9090
// Safe implementation when not on nightly.
9191
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
9292
{
93+
use syn::PatOr;
94+
9395
let mut less = quote! { ::core::cmp::Ordering::Less };
9496
let mut greater = quote! { ::core::cmp::Ordering::Greater };
9597

@@ -118,32 +120,29 @@ pub fn build_ord_signature(item: &Item, trait_: &DeriveTrait, body: &TokenStream
118120
different.push(quote! {
119121
#pattern => #greater,
120122
})
121-
} else {
122-
let mut arms = Vec::with_capacity(variants.len() - 1);
123-
124-
for (index_other, variant_other) in variants.iter().enumerate() {
125-
// Make sure we aren't comparing the same variant with itself.
126-
if index != index_other {
127-
use std::cmp::Ordering::*;
128-
129-
let ordering = match index.cmp(&index_other) {
130-
Less => &less,
131-
Equal => &equal,
132-
Greater => &greater,
133-
};
134-
135-
let pattern = &variant_other.other_pattern();
136-
137-
arms.push(quote! {
138-
#pattern => #ordering,
139-
});
140-
}
141-
}
142-
123+
}
124+
// Any variant between the first and last.
125+
else {
126+
// Collect all variants that are `Less`.
127+
let cases = variants
128+
.iter()
129+
.enumerate()
130+
.filter(|(index_other, _)| *index_other < index)
131+
.map(|(_, variant_other)| variant_other.other_pattern_skip().clone())
132+
.collect();
133+
134+
// Build one match arm pattern with all variants that are `Greater`.
135+
let pattern_less = PatOr {
136+
attrs: Vec::new(),
137+
leading_vert: None,
138+
cases,
139+
};
140+
141+
// All other variants are `Less`.
143142
different.push(quote! {
144143
#pattern => match __other {
145-
#(#arms)*
146-
_ => ::core::unreachable!("comparing variants yielded unexpected results"),
144+
#pattern_less => #greater,
145+
_ => #less,
147146
},
148147
});
149148
}

0 commit comments

Comments
 (0)