Skip to content

Commit 1fd649f

Browse files
daxpeddadaxpedda
committed
Remove C validation
1 parent 02f6d8e commit 1fd649f

File tree

4 files changed

+37
-87
lines changed

4 files changed

+37
-87
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Changed
1111
- Use safe casting in `PartialOrd` and `Ord` implementations in more cases when
1212
possible.
13-
- Avoid unnecessarily validating the discriminant type of enums with C
14-
representations in some cases.
13+
- Remove unnecessary validation of discriminant type for enums with C
14+
representation.
1515

1616
## [1.2.4] - 2023-09-01
1717

‎src/item.rs‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -99,18 +99,13 @@ impl Item<'_> {
9999
pub enum Discriminant {
100100
/// The enum has only a single variant.
101101
Single,
102-
/// The enum uses the default or C representation and has only unit
103-
/// variants.
104-
Unit {
105-
/// `true` if this is using the C representation.
106-
c: bool,
107-
},
108-
/// The enum uses the default or C representation but has a non-unit
109-
/// variant.
102+
/// The enum has only unit variants.
103+
Unit,
104+
/// The enum has a non-unit variant.
110105
Data,
111-
/// The enum uses a non-default representation and has only unit variants.
106+
/// The enum has only unit variants.
112107
UnitRepr(Representation),
113-
/// The enum uses a non-default representation and has a non-unit variant.
108+
/// The enum has a non-unit variant.
114109
DataRepr(Representation),
115110
}
116111

@@ -122,7 +117,6 @@ impl Discriminant {
122117
}
123118

124119
let mut has_repr = None;
125-
let mut is_c = false;
126120

127121
for attr in attrs {
128122
if attr.path().is_ident("repr") {
@@ -131,12 +125,13 @@ impl Discriminant {
131125
list.parse_args_with(Punctuated::<Ident, Token![,]>::parse_terminated)?;
132126

133127
for ident in list {
134-
if ident == "C" {
135-
is_c = true;
136-
} else if let Some(repr) = Representation::parse(&ident) {
128+
if let Some(repr) = Representation::parse(&ident) {
137129
has_repr = Some(repr);
138130
break;
139-
} else if ident != "Rust" && ident != "align" {
131+
} else if ident != "C"
132+
&& ident != "Rust" && ident != "align"
133+
&& ident != "packed"
134+
{
140135
return Err(Error::repr_unknown(ident.span()));
141136
}
142137
}
@@ -155,7 +150,7 @@ impl Discriminant {
155150
Self::DataRepr(repr)
156151
}
157152
} else if is_unit {
158-
Self::Unit { c: is_c }
153+
Self::Unit
159154
} else {
160155
let discriminant = variants
161156
.iter()

‎src/test/discriminant.rs‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -346,9 +346,6 @@ fn repr_c() -> Result<()> {
346346
};
347347
#[cfg(not(feature = "nightly"))]
348348
let partial_ord = quote! {
349-
#[repr(C)]
350-
enum EnsureReprCIsIsize { Test = 0_isize }
351-
352349
const fn __discriminant(__this: &Test) -> isize {
353350
match __this {
354351
Test::A => 0,
@@ -445,9 +442,6 @@ fn repr_c_clone() -> Result<()> {
445442
};
446443
#[cfg(not(feature = "nightly"))]
447444
let partial_ord = quote! {
448-
#[repr(C)]
449-
enum EnsureReprCIsIsize { Test = 0_isize }
450-
451445
::core::cmp::PartialOrd::partial_cmp(&(::core::clone::Clone::clone(self) as isize), &(::core::clone::Clone::clone(__other) as isize))
452446
};
453447

@@ -550,9 +544,6 @@ fn repr_c_copy() -> Result<()> {
550544
};
551545
#[cfg(not(feature = "nightly"))]
552546
let partial_ord = quote! {
553-
#[repr(C)]
554-
enum EnsureReprCIsIsize { Test = 0_isize }
555-
556547
::core::cmp::PartialOrd::partial_cmp(&(*self as isize), &(*__other as isize))
557548
};
558549

@@ -637,9 +628,6 @@ fn repr_c_reverse() -> Result<()> {
637628
};
638629
#[cfg(not(feature = "nightly"))]
639630
let partial_ord = quote! {
640-
#[repr(C)]
641-
enum EnsureReprCIsIsize { Test = 0_isize }
642-
643631
const fn __discriminant(__this: &Test) -> isize {
644632
match __this {
645633
Test::A => 2,
@@ -688,9 +676,6 @@ fn repr_c_mix() -> Result<()> {
688676
};
689677
#[cfg(not(feature = "nightly"))]
690678
let partial_ord = quote! {
691-
#[repr(C)]
692-
enum EnsureReprCIsIsize { Test = 0_isize }
693-
694679
const fn __discriminant(__this: &Test) -> isize {
695680
match __this {
696681
Test::A => 1,
@@ -741,9 +726,6 @@ fn repr_c_skip() -> Result<()> {
741726
};
742727
#[cfg(not(feature = "nightly"))]
743728
let partial_ord = quote! {
744-
#[repr(C)]
745-
enum EnsureReprCIsIsize { Test = 0_isize }
746-
747729
const fn __discriminant(__this: &Test) -> isize {
748730
match __this {
749731
Test::A => 0,
@@ -796,9 +778,6 @@ fn repr_c_expr() -> Result<()> {
796778
};
797779
#[cfg(not(feature = "nightly"))]
798780
let partial_ord = quote! {
799-
#[repr(C)]
800-
enum EnsureReprCIsIsize { Test = 0_isize }
801-
802781
const fn __discriminant(__this: &Test) -> isize {
803782
match __this {
804783
Test::A => u64::MAX - 2,

‎src/trait_/common_ord.rs‎

Lines changed: 24 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
//! Common implementation help for [`PartialOrd`] and [`Ord`].
22
3+
#[cfg(not(feature = "nightly"))]
4+
use std::borrow::Cow;
5+
36
use proc_macro2::TokenStream;
7+
#[cfg(not(feature = "nightly"))]
8+
use proc_macro2::{Literal, Span};
49
use quote::quote;
510
#[cfg(not(feature = "nightly"))]
6-
use syn::Path;
11+
use syn::{parse_quote, Expr, ExprLit, LitInt, Path};
712

813
#[cfg(not(feature = "nightly"))]
914
use crate::{item::Representation, Discriminant, Trait};
@@ -139,43 +144,25 @@ pub fn build_ord_signature(
139144
Discriminant::Single => {
140145
unreachable!("we should only generate this code with multiple variants")
141146
}
142-
Discriminant::Unit { c } => {
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(|| {
149-
quote! {
150-
#[repr(C)]
151-
enum EnsureReprCIsIsize {
152-
Test = 0_isize
153-
}
154-
}
155-
});
156-
147+
Discriminant::Unit => {
157148
if traits.iter().any(|trait_| trait_ == Trait::Copy) {
158149
quote! {
159-
#validate_c
160-
161150
#path::#method(&(*self as isize), &(*__other as isize))
162151
}
163152
} else if traits.iter().any(|trait_| trait_ == Trait::Clone) {
164153
let clone = DeriveTrait::Clone.path();
165154
quote! {
166-
#validate_c
167-
168155
#path::#method(&(#clone::clone(self) as isize), &(#clone::clone(__other) as isize))
169156
}
170157
} else {
171158
build_discriminant_order(
172-
None, validate_c, item, generics, variants, &path, &method,
159+
None, item, generics, variants, &path, &method,
173160
)
174161
}
175162
}
176-
Discriminant::Data => build_discriminant_order(
177-
None, None, item, generics, variants, &path, &method,
178-
),
163+
Discriminant::Data => {
164+
build_discriminant_order(None, item, generics, variants, &path, &method)
165+
}
179166
Discriminant::UnitRepr(repr) => {
180167
if traits.iter().any(|trait_| trait_ == Trait::Copy) {
181168
quote! {
@@ -188,15 +175,16 @@ pub fn build_ord_signature(
188175
}
189176
} else {
190177
#[cfg(feature = "safe")]
191-
let body_else = build_discriminant_order(
192-
Some(*repr),
193-
None,
194-
item,
195-
generics,
196-
variants,
197-
&path,
198-
&method,
199-
);
178+
let body_else = {
179+
build_discriminant_order(
180+
Some(*repr),
181+
item,
182+
generics,
183+
variants,
184+
&path,
185+
&method,
186+
)
187+
};
200188
#[cfg(not(feature = "safe"))]
201189
let body_else = quote! {
202190
#path::#method(
@@ -220,7 +208,6 @@ pub fn build_ord_signature(
220208
#[cfg(feature = "safe")]
221209
Discriminant::DataRepr(repr) => build_discriminant_order(
222210
Some(*repr),
223-
None,
224211
item,
225212
generics,
226213
variants,
@@ -267,22 +254,16 @@ pub fn build_ord_signature(
267254
}
268255
}
269256

270-
/// Builds order comparison recursively for all variants.
257+
/// Create `discriminant()` function and use it to do the comparison.
271258
#[cfg(not(feature = "nightly"))]
272259
fn build_discriminant_order(
273260
repr: Option<Representation>,
274-
validate_c: Option<TokenStream>,
275261
item: &Item,
276262
generics: &SplitGenerics<'_>,
277263
variants: &[Data<'_>],
278264
path: &Path,
279265
method: &TokenStream,
280266
) -> TokenStream {
281-
use std::{borrow::Cow, ops::Deref};
282-
283-
use proc_macro2::{Literal, Span};
284-
use syn::{parse_quote, Expr, ExprLit, LitInt};
285-
286267
let mut discriminants = Vec::<Cow<Expr>>::with_capacity(variants.len());
287268
let mut last_expression: Option<(Option<usize>, usize)> = None;
288269

@@ -328,7 +309,6 @@ fn build_discriminant_order(
328309
.zip(discriminants)
329310
.map(|(variant, discriminant)| {
330311
let pattern = variant.self_pattern();
331-
let discriminant = discriminant.deref();
332312

333313
quote! {
334314
#pattern => #discriminant
@@ -347,8 +327,6 @@ fn build_discriminant_order(
347327
} = generics;
348328

349329
quote! {
350-
#validate_c
351-
352330
const fn __discriminant #imp(__this: &#item #ty) -> #repr #where_clause {
353331
match __this {
354332
#(#variants),*
@@ -361,18 +339,16 @@ fn build_discriminant_order(
361339

362340
/// Build `match` arms for [`PartialOrd`] and [`Ord`].
363341
pub fn build_ord_body(trait_: &DeriveTrait, data: &Data) -> TokenStream {
364-
use DeriveTrait::*;
365-
366342
let path = trait_.path();
367343
let mut equal = quote! { ::core::cmp::Ordering::Equal };
368344

369345
// Add `Option` to `Ordering` if we are implementing `PartialOrd`.
370346
let method = match trait_ {
371-
PartialOrd => {
347+
DeriveTrait::PartialOrd => {
372348
equal = quote! { ::core::option::Option::Some(#equal) };
373349
quote! { partial_cmp }
374350
}
375-
Ord => quote! { cmp },
351+
DeriveTrait::Ord => quote! { cmp },
376352
_ => unreachable!("unsupported trait in `build_ord`"),
377353
};
378354

0 commit comments

Comments
 (0)