Skip to content

Commit ddad3aa

Browse files
authored
Improve codegen for enums (#80)
1 parent 1a80132 commit ddad3aa

File tree

3 files changed

+170
-21
lines changed

3 files changed

+170
-21
lines changed

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [Unreleased]
9+
10+
### Changed
11+
- Use safe casting in `PartialOrd` and `Ord` implementations in more cases when
12+
possible.
13+
- Avoid unnecessarily validating the discriminant type of enums with C
14+
representations in some cases.
15+
816
## [1.2.4] - 2023-09-01
917

1018
### Fixed

src/test/discriminant.rs

Lines changed: 143 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,54 @@ fn repr_c() -> Result<()> {
386386
)
387387
}
388388

389+
#[test]
390+
fn repr_c_without_discriminant() -> Result<()> {
391+
#[cfg(feature = "nightly")]
392+
let partial_ord = quote! {
393+
::core::cmp::PartialOrd::partial_cmp(
394+
&::core::intrinsics::discriminant_value(self),
395+
&::core::intrinsics::discriminant_value(__other),
396+
)
397+
};
398+
#[cfg(not(feature = "nightly"))]
399+
let partial_ord = quote! {
400+
fn __discriminant(__this: &Test) -> isize {
401+
match __this {
402+
Test::A => 0,
403+
Test::B => 1,
404+
Test::C => 2
405+
}
406+
}
407+
408+
::core::cmp::PartialOrd::partial_cmp(&__discriminant(self), &__discriminant(__other))
409+
};
410+
411+
test_derive(
412+
quote! {
413+
#[derive_where(PartialOrd)]
414+
#[repr(C)]
415+
enum Test {
416+
A,
417+
B,
418+
#[derive_where(incomparable)]
419+
C,
420+
}
421+
},
422+
quote! {
423+
impl ::core::cmp::PartialOrd for Test {
424+
#[inline]
425+
fn partial_cmp(&self, __other: &Self) -> ::core::option::Option<::core::cmp::Ordering> {
426+
if ::core::matches!(self, Test::C) || ::core::matches!(__other, Test::C) {
427+
return ::core::option::Option::None;
428+
}
429+
430+
#partial_ord
431+
}
432+
}
433+
},
434+
)
435+
}
436+
389437
#[test]
390438
fn repr_c_clone() -> Result<()> {
391439
#[cfg(feature = "nightly")]
@@ -440,6 +488,57 @@ fn repr_c_clone() -> Result<()> {
440488
)
441489
}
442490

491+
#[test]
492+
fn repr_c_clone_without_discriminant() -> Result<()> {
493+
#[cfg(feature = "nightly")]
494+
let partial_ord = quote! {
495+
::core::cmp::PartialOrd::partial_cmp(
496+
&::core::intrinsics::discriminant_value(self),
497+
&::core::intrinsics::discriminant_value(__other),
498+
)
499+
};
500+
#[cfg(not(feature = "nightly"))]
501+
let partial_ord = quote! {
502+
::core::cmp::PartialOrd::partial_cmp(&(::core::clone::Clone::clone(self) as isize), &(::core::clone::Clone::clone(__other) as isize))
503+
};
504+
505+
test_derive(
506+
quote! {
507+
#[derive_where(Clone, PartialOrd)]
508+
#[repr(C)]
509+
enum Test {
510+
A,
511+
B,
512+
#[derive_where(incomparable)]
513+
C,
514+
}
515+
},
516+
quote! {
517+
impl ::core::clone::Clone for Test {
518+
#[inline]
519+
fn clone(&self) -> Self {
520+
match self {
521+
Test::A => Test::A,
522+
Test::B => Test::B,
523+
Test::C => Test::C,
524+
}
525+
}
526+
}
527+
528+
impl ::core::cmp::PartialOrd for Test {
529+
#[inline]
530+
fn partial_cmp(&self, __other: &Self) -> ::core::option::Option<::core::cmp::Ordering> {
531+
if ::core::matches!(self, Test::C) || ::core::matches!(__other, Test::C) {
532+
return ::core::option::Option::None;
533+
}
534+
535+
#partial_ord
536+
}
537+
}
538+
},
539+
)
540+
}
541+
443542
#[test]
444543
fn repr_c_copy() -> Result<()> {
445544
#[cfg(feature = "nightly")]
@@ -485,6 +584,48 @@ fn repr_c_copy() -> Result<()> {
485584
)
486585
}
487586

587+
#[test]
588+
fn repr_c_copy_without_discriminant() -> Result<()> {
589+
#[cfg(feature = "nightly")]
590+
let partial_ord = quote! {
591+
::core::cmp::PartialOrd::partial_cmp(
592+
&::core::intrinsics::discriminant_value(self),
593+
&::core::intrinsics::discriminant_value(__other),
594+
)
595+
};
596+
#[cfg(not(feature = "nightly"))]
597+
let partial_ord = quote! {
598+
::core::cmp::PartialOrd::partial_cmp(&(*self as isize), &(*__other as isize))
599+
};
600+
601+
test_derive(
602+
quote! {
603+
#[derive_where(Copy, PartialOrd)]
604+
#[repr(C)]
605+
enum Test {
606+
A,
607+
B,
608+
#[derive_where(incomparable)]
609+
C,
610+
}
611+
},
612+
quote! {
613+
impl ::core::marker::Copy for Test { }
614+
615+
impl ::core::cmp::PartialOrd for Test {
616+
#[inline]
617+
fn partial_cmp(&self, __other: &Self) -> ::core::option::Option<::core::cmp::Ordering> {
618+
if ::core::matches!(self, Test::C) || ::core::matches!(__other, Test::C) {
619+
return ::core::option::Option::None;
620+
}
621+
622+
#partial_ord
623+
}
624+
}
625+
},
626+
)
627+
}
628+
488629
#[test]
489630
fn repr_c_reverse() -> Result<()> {
490631
#[cfg(feature = "nightly")]
@@ -1111,14 +1252,7 @@ fn repr_clone() -> Result<()> {
11111252
&::core::intrinsics::discriminant_value(__other),
11121253
)
11131254
};
1114-
#[cfg(not(any(feature = "nightly", feature = "safe")))]
1115-
let partial_ord = quote! {
1116-
::core::cmp::PartialOrd::partial_cmp(
1117-
&unsafe { *<*const _>::from(self).cast::<u64>() },
1118-
&unsafe { *<*const _>::from(__other).cast::<u64>() },
1119-
)
1120-
};
1121-
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
1255+
#[cfg(not(feature = "nightly"))]
11221256
let partial_ord = quote! {
11231257
::core::cmp::PartialOrd::partial_cmp(&(::core::clone::Clone::clone(self) as u64), &(::core::clone::Clone::clone(__other) as u64))
11241258
};
@@ -1169,14 +1303,7 @@ fn repr_copy() -> Result<()> {
11691303
&::core::intrinsics::discriminant_value(__other),
11701304
)
11711305
};
1172-
#[cfg(not(any(feature = "nightly", feature = "safe")))]
1173-
let partial_ord = quote! {
1174-
::core::cmp::PartialOrd::partial_cmp(
1175-
&unsafe { *<*const _>::from(self).cast::<u64>() },
1176-
&unsafe { *<*const _>::from(__other).cast::<u64>() },
1177-
)
1178-
};
1179-
#[cfg(all(not(feature = "nightly"), feature = "safe"))]
1306+
#[cfg(not(feature = "nightly"))]
11801307
let partial_ord = quote! {
11811308
::core::cmp::PartialOrd::partial_cmp(&(*self as u64), &(*__other as u64))
11821309
};

src/trait_/common_ord.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,12 @@ pub fn build_ord_signature(
140140
unreachable!("we should only generate this code with multiple variants")
141141
}
142142
Discriminant::Unit { c } => {
143-
let validate_c = c.then(|| {
143+
// C validation is only needed if custom discriminants are defined.
144+
let validate_c = (*c
145+
&& variants
146+
.iter()
147+
.any(|variant| variant.discriminant.is_some()))
148+
.then(|| {
144149
quote! {
145150
#[repr(C)]
146151
enum EnsureReprCIsIsize {
@@ -171,7 +176,6 @@ pub fn build_ord_signature(
171176
Discriminant::Data => build_discriminant_order(
172177
None, None, item, generics, variants, &path, &method,
173178
),
174-
#[cfg(feature = "safe")]
175179
Discriminant::UnitRepr(repr) => {
176180
if traits.iter().any(|trait_| trait_ == Trait::Copy) {
177181
quote! {
@@ -183,19 +187,29 @@ pub fn build_ord_signature(
183187
#path::#method(&(#clone::clone(self) as #repr), &(#clone::clone(__other) as #repr))
184188
}
185189
} else {
186-
build_discriminant_order(
190+
#[cfg(feature = "safe")]
191+
let body_else = build_discriminant_order(
187192
Some(*repr),
188193
None,
189194
item,
190195
generics,
191196
variants,
192197
&path,
193198
&method,
194-
)
199+
);
200+
#[cfg(not(feature = "safe"))]
201+
let body_else = quote! {
202+
#path::#method(
203+
&unsafe { *<*const _>::from(self).cast::<#repr>() },
204+
&unsafe { *<*const _>::from(__other).cast::<#repr>() },
205+
)
206+
};
207+
208+
body_else
195209
}
196210
}
197211
#[cfg(not(feature = "safe"))]
198-
Discriminant::UnitRepr(repr) | Discriminant::DataRepr(repr) => {
212+
Discriminant::DataRepr(repr) => {
199213
quote! {
200214
#path::#method(
201215
&unsafe { *<*const _>::from(self).cast::<#repr>() },

0 commit comments

Comments
 (0)