Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce default discriminant type for all representations and across editions #81

Merged
merged 3 commits into from
Sep 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- Use safe casting in `PartialOrd` and `Ord` implementations in more cases when
possible.
- Avoid unnecessarily validating the discriminant type of enums with C
representations in some cases.
- Avoid unnecessarily validating the default discriminant type in some cases.
- 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
153 changes: 97 additions & 56 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 @@ -346,14 +377,15 @@ fn repr_c() -> Result<()> {
};
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
#[repr(C)]
enum EnsureReprCIsIsize { Test = 0_isize }

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 @@ -445,8 +477,9 @@ fn repr_c_clone() -> Result<()> {
};
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
#[repr(C)]
enum EnsureReprCIsIsize { Test = 0_isize }
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 @@ -550,8 +583,9 @@ fn repr_c_copy() -> Result<()> {
};
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
#[repr(C)]
enum EnsureReprCIsIsize { Test = 0_isize }
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 @@ -637,14 +671,15 @@ fn repr_c_reverse() -> Result<()> {
};
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
#[repr(C)]
enum EnsureReprCIsIsize { Test = 0_isize }

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 @@ -688,15 +723,17 @@ fn repr_c_mix() -> Result<()> {
};
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
#[repr(C)]
enum EnsureReprCIsIsize { Test = 0_isize }

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 @@ -741,16 +778,19 @@ fn repr_c_skip() -> Result<()> {
};
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
#[repr(C)]
enum EnsureReprCIsIsize { Test = 0_isize }

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 @@ -796,14 +836,15 @@ fn repr_c_expr() -> Result<()> {
};
#[cfg(not(feature = "nightly"))]
let partial_ord = quote! {
#[repr(C)]
enum EnsureReprCIsIsize { Test = 0_isize }

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 => u64::MAX - 2,
Test::B => (u64::MAX - 2) + 1,
Test::C => (u64::MAX - 2) + 2
Test::A => __VALIDATE_ISIZE_A,
Test::B => __VALIDATE_ISIZE_B,
Test::C => __VALIDATE_ISIZE_C
}
}

Expand All @@ -815,7 +856,7 @@ fn repr_c_expr() -> Result<()> {
#[derive_where(PartialOrd)]
#[repr(C)]
enum Test {
A = u64::MAX - 2,
A = isize::MAX - 2,
B,
#[derive_where(incomparable)]
C,
Expand Down
Loading