From 8afd840e73752b2573a40c9a0180a17f8bdb570b Mon Sep 17 00:00:00 2001 From: daxpedda Date: Sun, 3 Sep 2023 11:52:32 +0200 Subject: [PATCH] Remove C validation --- CHANGELOG.md | 5 ++ src/test/discriminant.rs | 134 +++++++++++---------------------------- src/trait_/common_ord.rs | 126 +++++++----------------------------- 3 files changed, 65 insertions(+), 200 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d6ddd7..1581603 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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] + +### Changed +- Remove unnecessary validation of the default discriminant type for enums. + ## [1.2.5] - 2023-09-03 ### Changed diff --git a/src/test/discriminant.rs b/src/test/discriminant.rs index 827f269..ea8f6ee 100644 --- a/src/test/discriminant.rs +++ b/src/test/discriminant.rs @@ -15,14 +15,10 @@ fn default() -> Result<()> { #[cfg(not(feature = "nightly"))] let partial_ord = quote! { const fn __discriminant(__this: &Test) -> isize { - const __VALIDATE_ISIZE_A: isize = 0; - const __VALIDATE_ISIZE_B: isize = (0) + 1; - const __VALIDATE_ISIZE_C: isize = (0) + 2; - match __this { - Test::A => __VALIDATE_ISIZE_A, - Test::B => __VALIDATE_ISIZE_B, - Test::C => __VALIDATE_ISIZE_C + Test::A => 0, + Test::B => (0) + 1, + Test::C => (0) + 2 } } @@ -65,10 +61,6 @@ fn default_clone() -> Result<()> { }; #[cfg(not(feature = "nightly"))] let partial_ord = quote! { - const __VALIDATE_ISIZE_A: isize = 0; - const __VALIDATE_ISIZE_B: isize = (0) + 1; - const __VALIDATE_ISIZE_C: isize = (0) + 2; - ::core::cmp::PartialOrd::partial_cmp(&(::core::clone::Clone::clone(self) as isize), &(::core::clone::Clone::clone(__other) as isize)) }; @@ -119,10 +111,6 @@ fn default_copy() -> Result<()> { }; #[cfg(not(feature = "nightly"))] let partial_ord = quote! { - const __VALIDATE_ISIZE_A: isize = 0; - const __VALIDATE_ISIZE_B: isize = (0) + 1; - const __VALIDATE_ISIZE_C: isize = (0) + 2; - ::core::cmp::PartialOrd::partial_cmp(&(*self as isize), &(*__other as isize)) }; @@ -165,14 +153,10 @@ fn default_reverse() -> Result<()> { #[cfg(not(feature = "nightly"))] let partial_ord = quote! { const fn __discriminant(__this: &Test) -> isize { - const __VALIDATE_ISIZE_A: isize = 2; - const __VALIDATE_ISIZE_B: isize = 1; - const __VALIDATE_ISIZE_C: isize = 0; - match __this { - Test::A => __VALIDATE_ISIZE_A, - Test::B => __VALIDATE_ISIZE_B, - Test::C => __VALIDATE_ISIZE_C + Test::A => 2, + Test::B => 1, + Test::C => 0 } } @@ -216,16 +200,11 @@ fn default_mix() -> Result<()> { #[cfg(not(feature = "nightly"))] let partial_ord = quote! { const fn __discriminant(__this: &Test) -> isize { - const __VALIDATE_ISIZE_A: isize = 1; - const __VALIDATE_ISIZE_B: isize = 0; - const __VALIDATE_ISIZE_C: isize = 2; - const __VALIDATE_ISIZE_D: isize = (2) + 1; - match __this { - Test::A => __VALIDATE_ISIZE_A, - Test::B => __VALIDATE_ISIZE_B, - Test::C => __VALIDATE_ISIZE_C, - Test::D => __VALIDATE_ISIZE_D + Test::A => 1, + Test::B => 0, + Test::C => 2, + Test::D => (2) + 1 } } @@ -270,18 +249,12 @@ fn default_skip() -> Result<()> { #[cfg(not(feature = "nightly"))] let partial_ord = quote! { const fn __discriminant(__this: &Test) -> isize { - const __VALIDATE_ISIZE_A: isize = 0; - const __VALIDATE_ISIZE_B: isize = 3; - const __VALIDATE_ISIZE_C: isize = (3) + 1; - const __VALIDATE_ISIZE_D: isize = (3) + 2; - const __VALIDATE_ISIZE_E: isize = (3) + 3; - match __this { - Test::A => __VALIDATE_ISIZE_A, - Test::B => __VALIDATE_ISIZE_B, - Test::C => __VALIDATE_ISIZE_C, - Test::D => __VALIDATE_ISIZE_D, - Test::E => __VALIDATE_ISIZE_E + Test::A => 0, + Test::B => 3, + Test::C => (3) + 1, + Test::D => (3) + 2, + Test::E => (3) + 3 } } @@ -327,14 +300,10 @@ fn default_expr() -> Result<()> { #[cfg(not(feature = "nightly"))] let partial_ord = quote! { const fn __discriminant(__this: &Test) -> isize { - const __VALIDATE_ISIZE_A: isize = isize::MAX - 2; - const __VALIDATE_ISIZE_B: isize = (isize::MAX - 2) + 1; - const __VALIDATE_ISIZE_C: isize = (isize::MAX - 2) + 2; - match __this { - Test::A => __VALIDATE_ISIZE_A, - Test::B => __VALIDATE_ISIZE_B, - Test::C => __VALIDATE_ISIZE_C + Test::A => isize::MAX - 2, + Test::B => (isize::MAX - 2) + 1, + Test::C => (isize::MAX - 2) + 2 } } @@ -378,14 +347,10 @@ fn repr_c() -> Result<()> { #[cfg(not(feature = "nightly"))] let partial_ord = quote! { const fn __discriminant(__this: &Test) -> isize { - const __VALIDATE_ISIZE_A: isize = 0; - const __VALIDATE_ISIZE_B: isize = (0) + 1; - const __VALIDATE_ISIZE_C: isize = (0) + 2; - match __this { - Test::A => __VALIDATE_ISIZE_A, - Test::B => __VALIDATE_ISIZE_B, - Test::C => __VALIDATE_ISIZE_C + Test::A => 0, + Test::B => (0) + 1, + Test::C => (0) + 2 } } @@ -477,10 +442,6 @@ fn repr_c_clone() -> Result<()> { }; #[cfg(not(feature = "nightly"))] let partial_ord = quote! { - const __VALIDATE_ISIZE_A: isize = 0; - const __VALIDATE_ISIZE_B: isize = (0) + 1; - const __VALIDATE_ISIZE_C: isize = (0) + 2; - ::core::cmp::PartialOrd::partial_cmp(&(::core::clone::Clone::clone(self) as isize), &(::core::clone::Clone::clone(__other) as isize)) }; @@ -583,10 +544,6 @@ fn repr_c_copy() -> Result<()> { }; #[cfg(not(feature = "nightly"))] let partial_ord = quote! { - const __VALIDATE_ISIZE_A: isize = 0; - const __VALIDATE_ISIZE_B: isize = (0) + 1; - const __VALIDATE_ISIZE_C: isize = (0) + 2; - ::core::cmp::PartialOrd::partial_cmp(&(*self as isize), &(*__other as isize)) }; @@ -672,14 +629,10 @@ fn repr_c_reverse() -> Result<()> { #[cfg(not(feature = "nightly"))] let partial_ord = quote! { const fn __discriminant(__this: &Test) -> isize { - const __VALIDATE_ISIZE_A: isize = 2; - const __VALIDATE_ISIZE_B: isize = 1; - const __VALIDATE_ISIZE_C: isize = 0; - match __this { - Test::A => __VALIDATE_ISIZE_A, - Test::B => __VALIDATE_ISIZE_B, - Test::C => __VALIDATE_ISIZE_C + Test::A => 2, + Test::B => 1, + Test::C => 0 } } @@ -724,16 +677,11 @@ fn repr_c_mix() -> Result<()> { #[cfg(not(feature = "nightly"))] let partial_ord = quote! { const fn __discriminant(__this: &Test) -> isize { - const __VALIDATE_ISIZE_A: isize = 1; - const __VALIDATE_ISIZE_B: isize = 0; - const __VALIDATE_ISIZE_C: isize = 2; - const __VALIDATE_ISIZE_D: isize = (2) + 1; - match __this { - Test::A => __VALIDATE_ISIZE_A, - Test::B => __VALIDATE_ISIZE_B, - Test::C => __VALIDATE_ISIZE_C, - Test::D => __VALIDATE_ISIZE_D + Test::A => 1, + Test::B => 0, + Test::C => 2, + Test::D => (2) + 1 } } @@ -779,18 +727,12 @@ fn repr_c_skip() -> Result<()> { #[cfg(not(feature = "nightly"))] let partial_ord = quote! { const fn __discriminant(__this: &Test) -> isize { - const __VALIDATE_ISIZE_A: isize = 0; - const __VALIDATE_ISIZE_B: isize = 3; - const __VALIDATE_ISIZE_C: isize = (3) + 1; - const __VALIDATE_ISIZE_D: isize = (3) + 2; - const __VALIDATE_ISIZE_E: isize = (3) + 3; - match __this { - Test::A => __VALIDATE_ISIZE_A, - Test::B => __VALIDATE_ISIZE_B, - Test::C => __VALIDATE_ISIZE_C, - Test::D => __VALIDATE_ISIZE_D, - Test::E => __VALIDATE_ISIZE_E + Test::A => 0, + Test::B => 3, + Test::C => (3) + 1, + Test::D => (3) + 2, + Test::E => (3) + 3 } } @@ -837,14 +779,10 @@ fn repr_c_expr() -> Result<()> { #[cfg(not(feature = "nightly"))] let partial_ord = quote! { const fn __discriminant(__this: &Test) -> isize { - const __VALIDATE_ISIZE_A: isize = isize::MAX - 2; - const __VALIDATE_ISIZE_B: isize = (isize::MAX - 2) + 1; - const __VALIDATE_ISIZE_C: isize = (isize::MAX - 2) + 2; - match __this { - Test::A => __VALIDATE_ISIZE_A, - Test::B => __VALIDATE_ISIZE_B, - Test::C => __VALIDATE_ISIZE_C + Test::A => isize::MAX - 2, + Test::B => (isize::MAX - 2) + 1, + Test::C => (isize::MAX - 2) + 2 } } diff --git a/src/trait_/common_ord.rs b/src/trait_/common_ord.rs index b70b0a1..b8e848d 100644 --- a/src/trait_/common_ord.rs +++ b/src/trait_/common_ord.rs @@ -1,13 +1,11 @@ //! Common implementation help for [`PartialOrd`] and [`Ord`]. #[cfg(not(feature = "nightly"))] -use std::{borrow::Cow, ops::Deref}; +use std::borrow::Cow; use proc_macro2::TokenStream; #[cfg(not(feature = "nightly"))] use proc_macro2::{Literal, Span}; -#[cfg(not(feature = "nightly"))] -use quote::format_ident; use quote::quote; #[cfg(not(feature = "nightly"))] use syn::{parse_quote, Expr, ExprLit, LitInt, Path}; @@ -147,72 +145,23 @@ pub fn build_ord_signature( unreachable!("we should only generate this code with multiple variants") } Discriminant::Unit => { - let mut discriminants = None; - - // Validation is only needed if custom discriminants are defined. - let validate = (variants - .iter() - .any(|variant| variant.discriminant.is_some())) - .then(|| { - let discriminants = - discriminants.insert(build_discriminants(variants)); - let discriminants = discriminants.iter().zip(variants).map( - |(discriminant, variant)| { - let name = - format_ident!("__VALIDATE_ISIZE_{}", variant.ident); - let discriminant = discriminant.deref(); - - quote! { - const #name: isize = #discriminant; - } - }, - ); - - quote! { - #(#discriminants)* - } - }); - if traits.iter().any(|trait_| trait_ == Trait::Copy) { quote! { - #validate - #path::#method(&(*self as isize), &(*__other as isize)) } } else if traits.iter().any(|trait_| trait_ == Trait::Clone) { let clone = DeriveTrait::Clone.path(); quote! { - #validate - #path::#method(&(#clone::clone(self) as isize), &(#clone::clone(__other) as isize)) } } else { - let discriminants = discriminants - .get_or_insert_with(|| build_discriminants(variants)); - build_discriminant_comparison( - None, - validate, - item, - generics, - variants, - discriminants, - &path, - &method, + build_discriminant_order( + None, item, generics, variants, &path, &method, ) } } Discriminant::Data => { - let discriminants = build_discriminants(variants); - build_discriminant_comparison( - None, - None, - item, - generics, - variants, - &discriminants, - &path, - &method, - ) + build_discriminant_order(None, item, generics, variants, &path, &method) } Discriminant::UnitRepr(repr) => { if traits.iter().any(|trait_| trait_ == Trait::Copy) { @@ -227,14 +176,11 @@ pub fn build_ord_signature( } else { #[cfg(feature = "safe")] let body_else = { - let discriminants = build_discriminants(variants); - build_discriminant_comparison( + build_discriminant_order( Some(*repr), - None, item, generics, variants, - &discriminants, &path, &method, ) @@ -260,19 +206,14 @@ pub fn build_ord_signature( } } #[cfg(feature = "safe")] - Discriminant::DataRepr(repr) => { - let discriminants = build_discriminants(variants); - build_discriminant_comparison( - Some(*repr), - None, - item, - generics, - variants, - &discriminants, - &path, - &method, - ) - } + Discriminant::DataRepr(repr) => build_discriminant_order( + Some(*repr), + item, + generics, + variants, + &path, + &method, + ), }; if let Some(body_equal) = body_equal { @@ -313,9 +254,16 @@ pub fn build_ord_signature( } } -/// Builds list of discriminant values for all variants. +/// Create `discriminant()` function and use it to do the comparison. #[cfg(not(feature = "nightly"))] -fn build_discriminants<'a>(variants: &'a [Data<'_>]) -> Vec> { +fn build_discriminant_order( + repr: Option, + item: &Item, + generics: &SplitGenerics<'_>, + variants: &[Data<'_>], + path: &Path, + method: &TokenStream, +) -> TokenStream { let mut discriminants = Vec::>::with_capacity(variants.len()); let mut last_expression: Option<(Option, usize)> = None; @@ -356,38 +304,14 @@ fn build_discriminants<'a>(variants: &'a [Data<'_>]) -> Vec> { discriminants.push(discriminant); } - discriminants -} - -/// Uses list of discriminant values to compare variants. -#[cfg(not(feature = "nightly"))] -#[allow(clippy::too_many_arguments)] -fn build_discriminant_comparison( - repr: Option, - validate: Option, - item: &Item, - generics: &SplitGenerics<'_>, - variants: &[Data<'_>], - discriminants: &[Cow<'_, Expr>], - path: &Path, - method: &TokenStream, -) -> TokenStream { let variants = variants .iter() .zip(discriminants) .map(|(variant, discriminant)| { let pattern = variant.self_pattern(); - if validate.is_some() { - let discriminant = format_ident!("__VALIDATE_ISIZE_{}", variant.ident); - - quote! { - #pattern => #discriminant - } - } else { - quote! { - #pattern => #discriminant - } + quote! { + #pattern => #discriminant } }); @@ -404,8 +328,6 @@ fn build_discriminant_comparison( quote! { const fn __discriminant #imp(__this: &#item #ty) -> #repr #where_clause { - #validate - match __this { #(#variants),* }