Skip to content

Commit be7a2e3

Browse files
daxpeddadaxpedda
authored
Enforce default discriminant type for all representations and across editions (#81)
1 parent 02f6d8e commit be7a2e3

File tree

6 files changed

+228
-144
lines changed

6 files changed

+228
-144
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ 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+
- Avoid unnecessarily validating the default discriminant type in some cases.
14+
- Apply default enum discriminant type validation to all representations and
15+
make it cross-edition safe.
1516

1617
## [1.2.4] - 2023-09-01
1718

‎README.md‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -275,11 +275,9 @@ supported.
275275
Unions only support [`Clone`] and [`Copy`].
276276

277277
[`PartialOrd`] and [`Ord`] need to determine the discriminant type to
278-
function correctly. Unfortunately, according to the specification, the C
279-
representation without an integer representation doesn't have a
280-
platform-independent discriminant type. Therefor a check is inserted to
281-
ascertain that discriminants of enums with a C representation have the
282-
[`isize`] type, which is currently the case for all known platforms.
278+
function correctly. To protect against a potential future change to the
279+
default discriminant type, some compile-time validation is inserted to
280+
ascertain that the type remains `isize`.
283281

284282
### `no_std` support
285283

‎src/item.rs‎

Lines changed: 8 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,10 @@ 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" && ident != "Rust" && ident != "align" {
140132
return Err(Error::repr_unknown(ident.span()));
141133
}
142134
}
@@ -155,7 +147,7 @@ impl Discriminant {
155147
Self::DataRepr(repr)
156148
}
157149
} else if is_unit {
158-
Self::Unit { c: is_c }
150+
Self::Unit
159151
} else {
160152
let discriminant = variants
161153
.iter()

‎src/lib.rs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -314,11 +314,9 @@
314314
//! Unions only support [`Clone`] and [`Copy`].
315315
//!
316316
//! [`PartialOrd`] and [`Ord`] need to determine the discriminant type to
317-
//! function correctly. Unfortunately, according to the specification, the C
318-
//! representation without an integer representation doesn't have a
319-
//! platform-independent discriminant type. Therefor a check is inserted to
320-
//! ascertain that discriminants of enums with a C representation have the
321-
//! [`isize`] type, which is currently the case for all known platforms.
317+
//! function correctly. To protect against a potential future change to the
318+
//! default discriminant type, some compile-time validation is inserted to
319+
//! ascertain that the type remains `isize`.
322320
//!
323321
//! ## `no_std` support
324322
//!

‎src/test/discriminant.rs‎

Lines changed: 97 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,14 @@ fn default() -> Result<()> {
1515
#[cfg(not(feature = "nightly"))]
1616
let partial_ord = quote! {
1717
const fn __discriminant(__this: &Test) -> isize {
18+
const __VALIDATE_ISIZE_A: isize = 0;
19+
const __VALIDATE_ISIZE_B: isize = (0) + 1;
20+
const __VALIDATE_ISIZE_C: isize = (0) + 2;
21+
1822
match __this {
19-
Test::A => 0,
20-
Test::B => (0) + 1,
21-
Test::C => (0) + 2
23+
Test::A => __VALIDATE_ISIZE_A,
24+
Test::B => __VALIDATE_ISIZE_B,
25+
Test::C => __VALIDATE_ISIZE_C
2226
}
2327
}
2428

@@ -61,6 +65,10 @@ fn default_clone() -> Result<()> {
6165
};
6266
#[cfg(not(feature = "nightly"))]
6367
let partial_ord = quote! {
68+
const __VALIDATE_ISIZE_A: isize = 0;
69+
const __VALIDATE_ISIZE_B: isize = (0) + 1;
70+
const __VALIDATE_ISIZE_C: isize = (0) + 2;
71+
6472
::core::cmp::PartialOrd::partial_cmp(&(::core::clone::Clone::clone(self) as isize), &(::core::clone::Clone::clone(__other) as isize))
6573
};
6674

@@ -111,6 +119,10 @@ fn default_copy() -> Result<()> {
111119
};
112120
#[cfg(not(feature = "nightly"))]
113121
let partial_ord = quote! {
122+
const __VALIDATE_ISIZE_A: isize = 0;
123+
const __VALIDATE_ISIZE_B: isize = (0) + 1;
124+
const __VALIDATE_ISIZE_C: isize = (0) + 2;
125+
114126
::core::cmp::PartialOrd::partial_cmp(&(*self as isize), &(*__other as isize))
115127
};
116128

@@ -153,10 +165,14 @@ fn default_reverse() -> Result<()> {
153165
#[cfg(not(feature = "nightly"))]
154166
let partial_ord = quote! {
155167
const fn __discriminant(__this: &Test) -> isize {
168+
const __VALIDATE_ISIZE_A: isize = 2;
169+
const __VALIDATE_ISIZE_B: isize = 1;
170+
const __VALIDATE_ISIZE_C: isize = 0;
171+
156172
match __this {
157-
Test::A => 2,
158-
Test::B => 1,
159-
Test::C => 0
173+
Test::A => __VALIDATE_ISIZE_A,
174+
Test::B => __VALIDATE_ISIZE_B,
175+
Test::C => __VALIDATE_ISIZE_C
160176
}
161177
}
162178

@@ -200,11 +216,16 @@ fn default_mix() -> Result<()> {
200216
#[cfg(not(feature = "nightly"))]
201217
let partial_ord = quote! {
202218
const fn __discriminant(__this: &Test) -> isize {
219+
const __VALIDATE_ISIZE_A: isize = 1;
220+
const __VALIDATE_ISIZE_B: isize = 0;
221+
const __VALIDATE_ISIZE_C: isize = 2;
222+
const __VALIDATE_ISIZE_D: isize = (2) + 1;
223+
203224
match __this {
204-
Test::A => 1,
205-
Test::B => 0,
206-
Test::C => 2,
207-
Test::D => (2) + 1
225+
Test::A => __VALIDATE_ISIZE_A,
226+
Test::B => __VALIDATE_ISIZE_B,
227+
Test::C => __VALIDATE_ISIZE_C,
228+
Test::D => __VALIDATE_ISIZE_D
208229
}
209230
}
210231

@@ -249,12 +270,18 @@ fn default_skip() -> Result<()> {
249270
#[cfg(not(feature = "nightly"))]
250271
let partial_ord = quote! {
251272
const fn __discriminant(__this: &Test) -> isize {
273+
const __VALIDATE_ISIZE_A: isize = 0;
274+
const __VALIDATE_ISIZE_B: isize = 3;
275+
const __VALIDATE_ISIZE_C: isize = (3) + 1;
276+
const __VALIDATE_ISIZE_D: isize = (3) + 2;
277+
const __VALIDATE_ISIZE_E: isize = (3) + 3;
278+
252279
match __this {
253-
Test::A => 0,
254-
Test::B => 3,
255-
Test::C => (3) + 1,
256-
Test::D => (3) + 2,
257-
Test::E => (3) + 3
280+
Test::A => __VALIDATE_ISIZE_A,
281+
Test::B => __VALIDATE_ISIZE_B,
282+
Test::C => __VALIDATE_ISIZE_C,
283+
Test::D => __VALIDATE_ISIZE_D,
284+
Test::E => __VALIDATE_ISIZE_E
258285
}
259286
}
260287

@@ -300,10 +327,14 @@ fn default_expr() -> Result<()> {
300327
#[cfg(not(feature = "nightly"))]
301328
let partial_ord = quote! {
302329
const fn __discriminant(__this: &Test) -> isize {
330+
const __VALIDATE_ISIZE_A: isize = isize::MAX - 2;
331+
const __VALIDATE_ISIZE_B: isize = (isize::MAX - 2) + 1;
332+
const __VALIDATE_ISIZE_C: isize = (isize::MAX - 2) + 2;
333+
303334
match __this {
304-
Test::A => isize::MAX - 2,
305-
Test::B => (isize::MAX - 2) + 1,
306-
Test::C => (isize::MAX - 2) + 2
335+
Test::A => __VALIDATE_ISIZE_A,
336+
Test::B => __VALIDATE_ISIZE_B,
337+
Test::C => __VALIDATE_ISIZE_C
307338
}
308339
}
309340

@@ -346,14 +377,15 @@ fn repr_c() -> Result<()> {
346377
};
347378
#[cfg(not(feature = "nightly"))]
348379
let partial_ord = quote! {
349-
#[repr(C)]
350-
enum EnsureReprCIsIsize { Test = 0_isize }
351-
352380
const fn __discriminant(__this: &Test) -> isize {
381+
const __VALIDATE_ISIZE_A: isize = 0;
382+
const __VALIDATE_ISIZE_B: isize = (0) + 1;
383+
const __VALIDATE_ISIZE_C: isize = (0) + 2;
384+
353385
match __this {
354-
Test::A => 0,
355-
Test::B => (0) + 1,
356-
Test::C => (0) + 2
386+
Test::A => __VALIDATE_ISIZE_A,
387+
Test::B => __VALIDATE_ISIZE_B,
388+
Test::C => __VALIDATE_ISIZE_C
357389
}
358390
}
359391

@@ -445,8 +477,9 @@ fn repr_c_clone() -> Result<()> {
445477
};
446478
#[cfg(not(feature = "nightly"))]
447479
let partial_ord = quote! {
448-
#[repr(C)]
449-
enum EnsureReprCIsIsize { Test = 0_isize }
480+
const __VALIDATE_ISIZE_A: isize = 0;
481+
const __VALIDATE_ISIZE_B: isize = (0) + 1;
482+
const __VALIDATE_ISIZE_C: isize = (0) + 2;
450483

451484
::core::cmp::PartialOrd::partial_cmp(&(::core::clone::Clone::clone(self) as isize), &(::core::clone::Clone::clone(__other) as isize))
452485
};
@@ -550,8 +583,9 @@ fn repr_c_copy() -> Result<()> {
550583
};
551584
#[cfg(not(feature = "nightly"))]
552585
let partial_ord = quote! {
553-
#[repr(C)]
554-
enum EnsureReprCIsIsize { Test = 0_isize }
586+
const __VALIDATE_ISIZE_A: isize = 0;
587+
const __VALIDATE_ISIZE_B: isize = (0) + 1;
588+
const __VALIDATE_ISIZE_C: isize = (0) + 2;
555589

556590
::core::cmp::PartialOrd::partial_cmp(&(*self as isize), &(*__other as isize))
557591
};
@@ -637,14 +671,15 @@ fn repr_c_reverse() -> Result<()> {
637671
};
638672
#[cfg(not(feature = "nightly"))]
639673
let partial_ord = quote! {
640-
#[repr(C)]
641-
enum EnsureReprCIsIsize { Test = 0_isize }
642-
643674
const fn __discriminant(__this: &Test) -> isize {
675+
const __VALIDATE_ISIZE_A: isize = 2;
676+
const __VALIDATE_ISIZE_B: isize = 1;
677+
const __VALIDATE_ISIZE_C: isize = 0;
678+
644679
match __this {
645-
Test::A => 2,
646-
Test::B => 1,
647-
Test::C => 0
680+
Test::A => __VALIDATE_ISIZE_A,
681+
Test::B => __VALIDATE_ISIZE_B,
682+
Test::C => __VALIDATE_ISIZE_C
648683
}
649684
}
650685

@@ -688,15 +723,17 @@ fn repr_c_mix() -> Result<()> {
688723
};
689724
#[cfg(not(feature = "nightly"))]
690725
let partial_ord = quote! {
691-
#[repr(C)]
692-
enum EnsureReprCIsIsize { Test = 0_isize }
693-
694726
const fn __discriminant(__this: &Test) -> isize {
727+
const __VALIDATE_ISIZE_A: isize = 1;
728+
const __VALIDATE_ISIZE_B: isize = 0;
729+
const __VALIDATE_ISIZE_C: isize = 2;
730+
const __VALIDATE_ISIZE_D: isize = (2) + 1;
731+
695732
match __this {
696-
Test::A => 1,
697-
Test::B => 0,
698-
Test::C => 2,
699-
Test::D => (2) + 1
733+
Test::A => __VALIDATE_ISIZE_A,
734+
Test::B => __VALIDATE_ISIZE_B,
735+
Test::C => __VALIDATE_ISIZE_C,
736+
Test::D => __VALIDATE_ISIZE_D
700737
}
701738
}
702739

@@ -741,16 +778,19 @@ fn repr_c_skip() -> Result<()> {
741778
};
742779
#[cfg(not(feature = "nightly"))]
743780
let partial_ord = quote! {
744-
#[repr(C)]
745-
enum EnsureReprCIsIsize { Test = 0_isize }
746-
747781
const fn __discriminant(__this: &Test) -> isize {
782+
const __VALIDATE_ISIZE_A: isize = 0;
783+
const __VALIDATE_ISIZE_B: isize = 3;
784+
const __VALIDATE_ISIZE_C: isize = (3) + 1;
785+
const __VALIDATE_ISIZE_D: isize = (3) + 2;
786+
const __VALIDATE_ISIZE_E: isize = (3) + 3;
787+
748788
match __this {
749-
Test::A => 0,
750-
Test::B => 3,
751-
Test::C => (3) + 1,
752-
Test::D => (3) + 2,
753-
Test::E => (3) + 3
789+
Test::A => __VALIDATE_ISIZE_A,
790+
Test::B => __VALIDATE_ISIZE_B,
791+
Test::C => __VALIDATE_ISIZE_C,
792+
Test::D => __VALIDATE_ISIZE_D,
793+
Test::E => __VALIDATE_ISIZE_E
754794
}
755795
}
756796

@@ -796,14 +836,15 @@ fn repr_c_expr() -> Result<()> {
796836
};
797837
#[cfg(not(feature = "nightly"))]
798838
let partial_ord = quote! {
799-
#[repr(C)]
800-
enum EnsureReprCIsIsize { Test = 0_isize }
801-
802839
const fn __discriminant(__this: &Test) -> isize {
840+
const __VALIDATE_ISIZE_A: isize = isize::MAX - 2;
841+
const __VALIDATE_ISIZE_B: isize = (isize::MAX - 2) + 1;
842+
const __VALIDATE_ISIZE_C: isize = (isize::MAX - 2) + 2;
843+
803844
match __this {
804-
Test::A => u64::MAX - 2,
805-
Test::B => (u64::MAX - 2) + 1,
806-
Test::C => (u64::MAX - 2) + 2
845+
Test::A => __VALIDATE_ISIZE_A,
846+
Test::B => __VALIDATE_ISIZE_B,
847+
Test::C => __VALIDATE_ISIZE_C
807848
}
808849
}
809850

@@ -815,7 +856,7 @@ fn repr_c_expr() -> Result<()> {
815856
#[derive_where(PartialOrd)]
816857
#[repr(C)]
817858
enum Test {
818-
A = u64::MAX - 2,
859+
A = isize::MAX - 2,
819860
B,
820861
#[derive_where(incomparable)]
821862
C,

0 commit comments

Comments
 (0)