Skip to content

Commit

Permalink
Apply validation to default representation as well
Browse files Browse the repository at this point in the history
  • Loading branch information
daxpedda committed Sep 3, 2023
1 parent bc16da3 commit 22fdeff
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 105 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
possible.
- Avoid unnecessarily validating the discriminant type of enums with C
representations in some cases.
- Make sure to validate that enums with C representation use `isize`
discriminants even in the case of an edition change.
- Apply default enum discriminant type validation to all representations and
make it cross-edition safe.

## [1.2.4] - 2023-09-01

Expand Down
8 changes: 3 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -275,11 +275,9 @@ 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.
function correctly. To protect against a potential future change to the
default discriminant type, some compile-time validation is inserted to
ascertain that the type remains `isize`.

### `no_std` support

Expand Down
24 changes: 8 additions & 16 deletions src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,13 @@ impl Item<'_> {
pub enum Discriminant {
/// The enum has only a single variant.
Single,
/// The enum uses the default or C representation and has only unit
/// variants.
Unit {
/// `true` if this is using the C representation.
c: bool,
},
/// The enum uses the default or C representation but has a non-unit
/// variant.
/// The enum has only unit variants.
Unit,
/// The enum has a non-unit variant.
Data,
/// The enum uses a non-default representation and has only unit variants.
/// The enum has only unit variants.
UnitRepr(Representation),
/// The enum uses a non-default representation and has a non-unit variant.
/// The enum has a non-unit variant.
DataRepr(Representation),
}

Expand All @@ -122,7 +117,6 @@ impl Discriminant {
}

let mut has_repr = None;
let mut is_c = false;

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

for ident in list {
if ident == "C" {
is_c = true;
} else if let Some(repr) = Representation::parse(&ident) {
if let Some(repr) = Representation::parse(&ident) {
has_repr = Some(repr);
break;
} else if ident != "Rust" && ident != "align" {
} else if ident != "C" && ident != "Rust" && ident != "align" {
return Err(Error::repr_unknown(ident.span()));
}
}
Expand All @@ -155,7 +147,7 @@ impl Discriminant {
Self::DataRepr(repr)
}
} else if is_unit {
Self::Unit { c: is_c }
Self::Unit
} else {
let discriminant = variants
.iter()
Expand Down
8 changes: 3 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,9 @@
//! 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.
//! function correctly. To protect against a potential future change to the
//! default discriminant type, some compile-time validation is inserted to
//! ascertain that the type remains `isize`.
//!
//! ## `no_std` support
//!
Expand Down
151 changes: 91 additions & 60 deletions src/test/discriminant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,14 @@ 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 => 0,
Test::B => (0) + 1,
Test::C => (0) + 2
Test::A => __VALIDATE_ISIZE_A,
Test::B => __VALIDATE_ISIZE_B,
Test::C => __VALIDATE_ISIZE_C
}
}

Expand Down Expand Up @@ -61,6 +65,10 @@ 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))
};

Expand Down Expand Up @@ -111,6 +119,10 @@ 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))
};

Expand Down Expand Up @@ -153,10 +165,14 @@ 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 => 2,
Test::B => 1,
Test::C => 0
Test::A => __VALIDATE_ISIZE_A,
Test::B => __VALIDATE_ISIZE_B,
Test::C => __VALIDATE_ISIZE_C
}
}

Expand Down Expand Up @@ -200,11 +216,16 @@ 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 => 1,
Test::B => 0,
Test::C => 2,
Test::D => (2) + 1
Test::A => __VALIDATE_ISIZE_A,
Test::B => __VALIDATE_ISIZE_B,
Test::C => __VALIDATE_ISIZE_C,
Test::D => __VALIDATE_ISIZE_D
}
}

Expand Down Expand Up @@ -249,12 +270,18 @@ 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 => 0,
Test::B => 3,
Test::C => (3) + 1,
Test::D => (3) + 2,
Test::E => (3) + 3
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
}
}

Expand Down Expand Up @@ -300,10 +327,14 @@ 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 => isize::MAX - 2,
Test::B => (isize::MAX - 2) + 1,
Test::C => (isize::MAX - 2) + 2
Test::A => __VALIDATE_ISIZE_A,
Test::B => __VALIDATE_ISIZE_B,
Test::C => __VALIDATE_ISIZE_C
}
}

Expand Down Expand Up @@ -347,14 +378,14 @@ fn repr_c() -> Result<()> {
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
const fn __discriminant(__this: &Test) -> isize {
const __ENSURE_REPR_C_IS_ISIZE_A: isize = 0;
const __ENSURE_REPR_C_IS_ISIZE_B: isize = (0) + 1;
const __ENSURE_REPR_C_IS_ISIZE_C: isize = (0) + 2;
const __VALIDATE_ISIZE_A: isize = 0;
const __VALIDATE_ISIZE_B: isize = (0) + 1;
const __VALIDATE_ISIZE_C: isize = (0) + 2;

match __this {
Test::A => __ENSURE_REPR_C_IS_ISIZE_A,
Test::B => __ENSURE_REPR_C_IS_ISIZE_B,
Test::C => __ENSURE_REPR_C_IS_ISIZE_C
Test::A => __VALIDATE_ISIZE_A,
Test::B => __VALIDATE_ISIZE_B,
Test::C => __VALIDATE_ISIZE_C
}
}

Expand Down Expand Up @@ -446,9 +477,9 @@ fn repr_c_clone() -> Result<()> {
};
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
const __ENSURE_REPR_C_IS_ISIZE_A: isize = 0;
const __ENSURE_REPR_C_IS_ISIZE_B: isize = (0) + 1;
const __ENSURE_REPR_C_IS_ISIZE_C: isize = (0) + 2;
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))
};
Expand Down Expand Up @@ -552,9 +583,9 @@ fn repr_c_copy() -> Result<()> {
};
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
const __ENSURE_REPR_C_IS_ISIZE_A: isize = 0;
const __ENSURE_REPR_C_IS_ISIZE_B: isize = (0) + 1;
const __ENSURE_REPR_C_IS_ISIZE_C: isize = (0) + 2;
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))
};
Expand Down Expand Up @@ -641,14 +672,14 @@ fn repr_c_reverse() -> Result<()> {
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
const fn __discriminant(__this: &Test) -> isize {
const __ENSURE_REPR_C_IS_ISIZE_A: isize = 2;
const __ENSURE_REPR_C_IS_ISIZE_B: isize = 1;
const __ENSURE_REPR_C_IS_ISIZE_C: isize = 0;
const __VALIDATE_ISIZE_A: isize = 2;
const __VALIDATE_ISIZE_B: isize = 1;
const __VALIDATE_ISIZE_C: isize = 0;

match __this {
Test::A => __ENSURE_REPR_C_IS_ISIZE_A,
Test::B => __ENSURE_REPR_C_IS_ISIZE_B,
Test::C => __ENSURE_REPR_C_IS_ISIZE_C
Test::A => __VALIDATE_ISIZE_A,
Test::B => __VALIDATE_ISIZE_B,
Test::C => __VALIDATE_ISIZE_C
}
}

Expand Down Expand Up @@ -693,16 +724,16 @@ fn repr_c_mix() -> Result<()> {
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
const fn __discriminant(__this: &Test) -> isize {
const __ENSURE_REPR_C_IS_ISIZE_A: isize = 1;
const __ENSURE_REPR_C_IS_ISIZE_B: isize = 0;
const __ENSURE_REPR_C_IS_ISIZE_C: isize = 2;
const __ENSURE_REPR_C_IS_ISIZE_D: isize = (2) + 1;
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 => __ENSURE_REPR_C_IS_ISIZE_A,
Test::B => __ENSURE_REPR_C_IS_ISIZE_B,
Test::C => __ENSURE_REPR_C_IS_ISIZE_C,
Test::D => __ENSURE_REPR_C_IS_ISIZE_D
Test::A => __VALIDATE_ISIZE_A,
Test::B => __VALIDATE_ISIZE_B,
Test::C => __VALIDATE_ISIZE_C,
Test::D => __VALIDATE_ISIZE_D
}
}

Expand Down Expand Up @@ -748,18 +779,18 @@ fn repr_c_skip() -> Result<()> {
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
const fn __discriminant(__this: &Test) -> isize {
const __ENSURE_REPR_C_IS_ISIZE_A: isize = 0;
const __ENSURE_REPR_C_IS_ISIZE_B: isize = 3;
const __ENSURE_REPR_C_IS_ISIZE_C: isize = (3) + 1;
const __ENSURE_REPR_C_IS_ISIZE_D: isize = (3) + 2;
const __ENSURE_REPR_C_IS_ISIZE_E: isize = (3) + 3;
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 => __ENSURE_REPR_C_IS_ISIZE_A,
Test::B => __ENSURE_REPR_C_IS_ISIZE_B,
Test::C => __ENSURE_REPR_C_IS_ISIZE_C,
Test::D => __ENSURE_REPR_C_IS_ISIZE_D,
Test::E => __ENSURE_REPR_C_IS_ISIZE_E
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
}
}

Expand Down Expand Up @@ -806,14 +837,14 @@ fn repr_c_expr() -> Result<()> {
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
const fn __discriminant(__this: &Test) -> isize {
const __ENSURE_REPR_C_IS_ISIZE_A: isize = isize::MAX - 2;
const __ENSURE_REPR_C_IS_ISIZE_B: isize = (isize::MAX - 2) + 1;
const __ENSURE_REPR_C_IS_ISIZE_C: isize = (isize::MAX - 2) + 2;
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 => __ENSURE_REPR_C_IS_ISIZE_A,
Test::B => __ENSURE_REPR_C_IS_ISIZE_B,
Test::C => __ENSURE_REPR_C_IS_ISIZE_C
Test::A => __VALIDATE_ISIZE_A,
Test::B => __VALIDATE_ISIZE_B,
Test::C => __VALIDATE_ISIZE_C
}
}

Expand Down
Loading

0 comments on commit 22fdeff

Please sign in to comment.