Build manual discriminant order if discriminant isn't available (#77)

Co-authored-by: Roland Fredenhagen <[email protected]>

Author: daxpedda

.config/topic.dic

@@ -4,6 +4,7 @@ Changelog
 CHANGELOG
 derive_bounded
 destructure/G
+discriminants
 enum/S
 FQS
 invariants

CHANGELOG.md

@@ -8,9 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Fixed
+- Take representation into account when determining the discriminant type.
+
+### Changed
 - Use stable methods to retrieve the discriminant in `PartialOrd` and `Ord`
-  implementations. Safe methods are now also used with the `safe` crate
-  feature, significantly improving the performance there.
+  implementations. Safe methods used with the `safe` crate feature don't use
+  recursive matches anymore, significantly improving the performance.
 
 ## [1.2.3] - 2023-08-23
 

README.md

@@ -274,6 +274,13 @@ supported.
 
 Unions only support [`Clone`] and [`Copy`].
 
+[`PartialOrd`] and [`Ord`] need to determine the discriminant type to
+function correctly. Unfortunately, according to the specification, the C
+representation without an integer representation doesn't have a
+platform-independent discriminant type. Therefor a check is inserted to
+ascertain that discriminants of enums with a C representation have the
+[`isize`] type, which is currently the case for all known platforms.
+
 ### `no_std` support
 
 `no_std` support is provided by default.
@@ -282,12 +289,9 @@ Unions only support [`Clone`] and [`Copy`].
 
 - `nightly`: Implements [`Ord`] and [`PartialOrd`] with the help of
   [`core::intrinsics::discriminant_value`], which is what Rust does by
-  default too. Without this feature [`transmute`] is
-  used to convert [`Discriminant`] to a [`i32`],
-  which is the underlying type.
+  default too. This requires a nightly version of the Rust compiler.
 - `safe`: `safe`: Uses only safe ways to access the discriminant of the enum
-  for [`Ord`] and [`PartialOrd`]. This is much slower, but might be
-  preferred if you don't trust derive-where. It also replaces all cases of
+  for [`Ord`] and [`PartialOrd`]. It also replaces all cases of
   [`core::hint::unreachable_unchecked`] in [`Ord`], [`PartialEq`] and
   [`PartialOrd`], which is what std uses, with [`unreachable`].
 - `zeroize`: Allows deriving [`Zeroize`] and [`zeroize`][method@zeroize] on
@@ -349,6 +353,7 @@ conditions.
 [`Drop`]: https://doc.rust-lang.org/core/ops/trait.Drop.html
 [`Eq`]: https://doc.rust-lang.org/core/cmp/trait.Eq.html
 [`i32`]: https://doc.rust-lang.org/core/primitive.i32.html
+[`isize`]: https://doc.rust-lang.org/core/primitive.isize.html
 [`Ord`]: https://doc.rust-lang.org/core/cmp/trait.Ord.html
 [`PartialEq`]: https://doc.rust-lang.org/core/cmp/trait.PartialEq.html
 [`PartialOrd`]: https://doc.rust-lang.org/core/cmp/trait.PartialOrd.html

src/data.rs

@@ -4,7 +4,7 @@ mod field;
 mod fields;
 
 use proc_macro2::Span;
-use syn::{FieldsNamed, Ident, Pat, PatPath, Path, Result, Variant};
+use syn::{Expr, FieldsNamed, Ident, Pat, PatPath, Path, Result, Variant};
 
 pub use self::{
 	field::{Field, Member},
@@ -27,6 +27,8 @@ pub struct Data<'a> {
 	pub path: Path,
 	/// [Type](DataType) of this struct, union or variant.
 	pub type_: DataType<'a>,
+	/// Discriminant of this variant.
+	pub discriminant: Option<&'a Expr>,
 }
 
 /// Type of this data.
@@ -98,6 +100,7 @@ impl<'a> Data<'a> {
 						ident,
 						path,
 						type_: DataType::Struct(fields),
+						discriminant: None,
 					})
 				}
 			}
@@ -114,6 +117,7 @@ impl<'a> Data<'a> {
 						ident,
 						path,
 						type_: DataType::Tuple(fields),
+						discriminant: None,
 					})
 				}
 			}
@@ -127,6 +131,7 @@ impl<'a> Data<'a> {
 					qself: None,
 					path,
 				})),
+				discriminant: None,
 			}),
 			syn::Fields::Unit => Err(Error::item_empty(span)),
 		}
@@ -153,6 +158,7 @@ impl<'a> Data<'a> {
 				ident,
 				path,
 				type_: DataType::Union(fields),
+				discriminant: None,
 			})
 		}
 	}
@@ -185,6 +191,7 @@ impl<'a> Data<'a> {
 						default,
 						type_: VariantType::Struct(fields),
 					},
+					discriminant: variant.discriminant.as_ref().map(|(_, expr)| expr),
 				})
 			}
 			syn::Fields::Unnamed(fields) => {
@@ -200,6 +207,7 @@ impl<'a> Data<'a> {
 						default,
 						type_: VariantType::Tuple(fields),
 					},
+					discriminant: variant.discriminant.as_ref().map(|(_, expr)| expr),
 				})
 			}
 			syn::Fields::Unit => {
@@ -218,6 +226,7 @@ impl<'a> Data<'a> {
 						default,
 						type_: VariantType::Unit(pattern),
 					},
+					discriminant: variant.discriminant.as_ref().map(|(_, expr)| expr),
 				})
 			}
 		}
@@ -254,15 +263,6 @@ impl<'a> Data<'a> {
 		}
 	}
 
-	/// Returns the destructuring `other` pattern of this [`Data`].
-	#[cfg(not(feature = "nightly"))]
-	pub fn other_pattern_skip(&self) -> &Pat {
-		match self.fields() {
-			Either::Left(fields) => &fields.other_pattern_skip,
-			Either::Right(pattern) => pattern,
-		}
-	}
-
 	/// Returns `true` if this variant is marked as the [`struct@Default`]. If
 	/// not a variant, always returns `true`.
 	pub fn is_default(&self) -> bool {

src/data/fields.rs

@@ -1,12 +1,9 @@
 //! Storage for items or variants with data.
 
-use std::iter;
-
 use syn::{
-	punctuated::Punctuated,
 	token::{Brace, Paren},
-	FieldPat, FieldsNamed, FieldsUnnamed, Ident, Pat, PatIdent, PatRest, PatStruct, PatTupleStruct,
-	Path, Result, Token,
+	FieldPat, FieldsNamed, FieldsUnnamed, Ident, Pat, PatIdent, PatStruct, PatTupleStruct, Path,
+	Result, Token,
 };
 
 use crate::{DeriveWhere, Field, Skip, Trait};
@@ -18,9 +15,6 @@ 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.
-	pub other_pattern_skip: Pat,
 	/// [`Field`]s of this struct, union or variant.
 	pub fields: Vec<Field<'a>>,
 }
@@ -36,23 +30,11 @@ 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);
-		let other_pattern_skip = Pat::Struct(PatStruct {
-			attrs: Vec::new(),
-			qself: None,
-			path: path.clone(),
-			brace_token: Brace::default(),
-			fields: Punctuated::new(),
-			rest: Some(PatRest {
-				attrs: Vec::new(),
-				dot2_token: <Token![..]>::default(),
-			}),
-		});
 		let other_pattern = Self::struct_pattern(path, &fields, |field| &field.other_ident);
 
 		Ok(Self {
 			self_pattern,
 			other_pattern,
-			other_pattern_skip,
 			fields,
 		})
 	}
@@ -67,23 +49,11 @@ 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);
-		let other_pattern_skip = Pat::TupleStruct(PatTupleStruct {
-			attrs: Vec::new(),
-			qself: None,
-			path: path.clone(),
-			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,
-			other_pattern_skip,
 			fields,
 		})
 	}

src/error.rs

@@ -212,16 +212,21 @@ impl Error {
 		syn::Error::new(span, "duplicate trait with the same bound")
 	}
 
-	/// `repr` attribute that isn't a meta list.
-	pub fn repr_parse(span: Span) -> syn::Error {
-		syn::Error::new(span, "unable to parse `repr` attribute")
-	}
-
 	/// Unknown `repr`.
 	pub fn repr_unknown(span: Span) -> syn::Error {
 		syn::Error::new(span, "found unknown representation")
 	}
 
+	/// Invalid enum with non-empty variants and custom discriminants without an
+	/// integer representation.
+	pub fn repr_discriminant_invalid(span: Span) -> syn::Error {
+		syn::Error::new(
+			span,
+			"enums with non-empty variants and custom discriminants require a integer \
+			 representation",
+		)
+	}
+
 	/// Unsupported default option if [`Default`] isn't implemented.
 	pub fn default(span: Span) -> syn::Error {
 		syn::Error::new(

src/input.rs

@@ -1,7 +1,7 @@
 //! Parses [`DeriveInput`] into something more useful.
 
 use proc_macro2::Span;
-use syn::{DeriveInput, GenericParam, Generics, Result};
+use syn::{DeriveInput, GenericParam, Generics, ImplGenerics, Result, TypeGenerics, WhereClause};
 
 #[cfg(feature = "zeroize")]
 use crate::DeriveTrait;
@@ -12,7 +12,7 @@ pub struct Input<'a> {
 	/// `derive_where` attributes on the item.
 	pub derive_wheres: Vec<DeriveWhere>,
 	/// Generics necessary to define for an `impl`.
-	pub generics: &'a Generics,
+	pub generics: SplitGenerics<'a>,
 	/// Fields or variants of this item.
 	pub item: Item<'a>,
 }
@@ -194,10 +194,35 @@ impl<'a> Input<'a> {
 			}
 		}
 
+		let generics = SplitGenerics::new(generics);
+
 		Ok(Self {
 			derive_wheres,
 			generics,
 			item,
 		})
 	}
 }
+
+/// Stores output of [`Generics::split_for_impl()`].
+pub struct SplitGenerics<'a> {
+	/// Necessary generic definitions.
+	pub imp: ImplGenerics<'a>,
+	/// Generics on the type itself.
+	pub ty: TypeGenerics<'a>,
+	/// `where` clause.
+	pub where_clause: Option<&'a WhereClause>,
+}
+
+impl<'a> SplitGenerics<'a> {
+	/// Creates a [`SplitGenerics`] from [`Generics`].
+	fn new(generics: &'a Generics) -> Self {
+		let (imp, ty, where_clause) = generics.split_for_impl();
+
+		SplitGenerics {
+			imp,
+			ty,
+			where_clause,
+		}
+	}
+}