Skip to content

Commit ba1a33e

Browse files
scottmcmscottmcm
committed
Prefer -1 for None
Currently we pick "weird" numbers like `1114112` for `None::<char>`. While that's not *wrong*, it's kinda *unnatural* -- a human wouldn't make that choice. This PR instead picks `-1` for thinge like `None::<char>` -- like clang's `WEOF` -- and `None::<bool>` and such. Any enums with more than one niched value (so not `Result` nor `Option`) remain as they were before.
1 parent 9bc1313 commit ba1a33e

9 files changed

Lines changed: 53 additions & 21 deletions

File tree

‎compiler/rustc_abi/src/lib.rs‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2051,8 +2051,7 @@ impl Niche {
20512051
assert!(size.bits() <= 128);
20522052
let max_value = size.unsigned_int_max();
20532053

2054-
let niche = v.end.wrapping_add(1)..v.start;
2055-
let available = niche.end.wrapping_sub(niche.start) & max_value;
2054+
let available = v.start.wrapping_sub(v.end).wrapping_sub(1) & max_value;
20562055
if count > available {
20572056
return None;
20582057
}
@@ -2080,7 +2079,17 @@ impl Niche {
20802079
Some((start, Scalar::Initialized { value, valid_range: v.with_end(end) }))
20812080
};
20822081
let distance_end_zero = max_value - v.end;
2083-
if v.start > v.end {
2082+
// FIXME: this ought to work for `bool` too, but that seems to be hitting a miscompilation
2083+
// <https://github.com/rust-lang/rust/pull/155473#issuecomment-4302036343>
2084+
if count == 1 && v != (WrappingRange { start: 0, end: 1 }) {
2085+
// We only need one, so just pick the one closest to zero.
2086+
// Not only does that obviously use zero if it's possible, but it also
2087+
// simplifies testing things like `Option<char>`, since looking for `-1`
2088+
// is easier than looking for `1114112` (and matches clang's `WEOF`).
2089+
let next_up = size.sign_extend(v.end.wrapping_add(1)).unsigned_abs();
2090+
let next_down = size.sign_extend(v.start.wrapping_sub(1)).unsigned_abs();
2091+
if next_down <= next_up { move_start(v) } else { move_end(v) }
2092+
} else if v.start > v.end {
20842093
// zero is unavailable because wrapping occurs
20852094
move_end(v)
20862095
} else if v.start <= distance_end_zero {

‎tests/codegen-llvm/enum/enum-aggregate.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
//@ only-64bit
33

44
#![crate_type = "lib"]
5+
#![feature(pattern_types, pattern_type_macro)]
56

67
use std::cmp::Ordering;
78
use std::num::NonZero;
@@ -123,3 +124,14 @@ fn make_fully_uninhabited_result(v: u32, n: Never) -> Result<(u32, Never), (Neve
123124
}
124125

125126
enum Never {}
127+
128+
#[repr(transparent)]
129+
struct NewtypeIndex(std::pat::pattern_type!(u32 is 0..=0xFFFFFF00));
130+
131+
#[no_mangle]
132+
pub fn make_none_newtype_index() -> Option<NewtypeIndex> {
133+
// CHECK-LABEL: @make_none_newtype_index
134+
// CHECK-NEXT: start:
135+
// CHECK-NEXT: ret i32 -1
136+
None
137+
}

‎tests/codegen-llvm/enum/enum-discriminant-eq.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ pub fn opt_bool_eq_discr(a: Option<bool>, b: Option<bool>) -> bool {
3636
#[unsafe(no_mangle)]
3737
pub fn opt_ord_eq_discr(a: Option<Ordering>, b: Option<Ordering>) -> bool {
3838
// CHECK-LABEL: @opt_ord_eq_discr(
39-
// CHECK: %[[A:.+]] = icmp ne i8 %a, 2
40-
// CHECK: %[[B:.+]] = icmp eq i8 %b, 2
39+
// CHECK: %[[A:.+]] = icmp ne i8 %a, -2
40+
// CHECK: %[[B:.+]] = icmp eq i8 %b, -2
4141
// CHECK: %[[R:.+]] = xor i1 %[[A]], %[[B]]
4242
// CHECK: ret i1 %[[R]]
4343

@@ -58,8 +58,8 @@ pub fn opt_nz32_eq_discr(a: Option<NonZero<u32>>, b: Option<NonZero<u32>>) -> bo
5858
#[unsafe(no_mangle)]
5959
pub fn opt_ac_eq_discr(a: Option<AC>, b: Option<AC>) -> bool {
6060
// CHECK-LABEL: @opt_ac_eq_discr(
61-
// CHECK: %[[A:.+]] = icmp ne i8 %a, -128
62-
// CHECK: %[[B:.+]] = icmp eq i8 %b, -128
61+
// CHECK: %[[A:.+]] = icmp ne i8 %a, -1
62+
// CHECK: %[[B:.+]] = icmp eq i8 %b, -1
6363
// CHECK: %[[R:.+]] = xor i1 %[[A]], %[[B]]
6464
// CHECK: ret i1 %[[R]]
6565

‎tests/codegen-llvm/enum/enum-two-variants-match.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ use std::cmp::Ordering::{self, *};
8181
#[no_mangle]
8282
pub fn option_ordering_match(x: Option<Ordering>) -> char {
8383
// CHECK: %[[RAW:.+]] = load i8, ptr %x
84-
// CHECK: %[[IS_NONE:.+]] = icmp eq i8 %[[RAW]], 2
84+
// CHECK: %[[IS_NONE:.+]] = icmp eq i8 %[[RAW]], -2
8585
// CHECK: %[[OPT_DISCR:.+]] = select i1 %[[IS_NONE]], i64 0, i64 1
8686
// CHECK: %[[OPT_DISCR_T:.+]] = trunc nuw i64 %[[OPT_DISCR]] to i1
8787
// CHECK: br i1 %[[OPT_DISCR_T]], label %[[BB_SOME:.+]], label %[[BB_NONE:.+]]

‎tests/codegen-llvm/option-niche-eq.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,14 @@ pub fn bool_eq(l: Option<bool>, r: Option<bool>) -> bool {
8282
// CHECK-NEXT: ret i1
8383
l == r
8484
}
85+
86+
// CHECK-LABEL: @bool_ref_eq
87+
#[no_mangle]
88+
pub fn bool_ref_eq(l: &Option<bool>, r: &Option<bool>) -> bool {
89+
// CHECK: start:
90+
// CHECK-NEXT: load i8
91+
// CHECK-NEXT: load i8
92+
// CHECK-NEXT: icmp eq i8
93+
// CHECK-NEXT: ret i1
94+
l == r
95+
}

‎tests/ui/layout/debug.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -745,7 +745,7 @@ error: layout_of(Option<char>) = Layout {
745745
I32,
746746
false,
747747
),
748-
valid_range: 0..=1114112,
748+
valid_range: (..=1114111) | (4294967295..),
749749
},
750750
),
751751
fields: Arbitrary {
@@ -763,7 +763,7 @@ error: layout_of(Option<char>) = Layout {
763763
I32,
764764
false,
765765
),
766-
valid_range: 0..=1114112,
766+
valid_range: (..=1114111) | (4294967295..),
767767
},
768768
),
769769
uninhabited: false,
@@ -773,12 +773,12 @@ error: layout_of(Option<char>) = Layout {
773773
I32,
774774
false,
775775
),
776-
valid_range: 0..=1114112,
776+
valid_range: (..=1114111) | (4294967295..),
777777
},
778778
tag_encoding: Niche {
779779
untagged_variant: 1,
780780
niche_variants: 0..=0,
781-
niche_start: 1114112,
781+
niche_start: 4294967295,
782782
},
783783
tag_field: 0,
784784
variants: [

‎tests/ui/layout/zero-sized-array-enum-niche.stderr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ error: layout_of(Result<[u32; 0], Packed<U16IsZero>>) = Layout {
370370
I16,
371371
false,
372372
),
373-
valid_range: 0..=1,
373+
valid_range: (..=0) | (65535..),
374374
},
375375
),
376376
uninhabited: false,
@@ -380,12 +380,12 @@ error: layout_of(Result<[u32; 0], Packed<U16IsZero>>) = Layout {
380380
I16,
381381
false,
382382
),
383-
valid_range: 0..=1,
383+
valid_range: (..=0) | (65535..),
384384
},
385385
tag_encoding: Niche {
386386
untagged_variant: 1,
387387
niche_variants: 0..=0,
388-
niche_start: 1,
388+
niche_start: 65535,
389389
},
390390
tag_field: 0,
391391
variants: [

‎tests/ui/type/pattern_types/non_null.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ error: layout_of(Option<(*const ()) is !null>) = Layout {
5757
0,
5858
),
5959
),
60-
valid_range: (..=0) | (1..),
60+
valid_range: 0..=18446744073709551615,
6161
},
6262
),
6363
fields: Arbitrary {
@@ -77,7 +77,7 @@ error: layout_of(Option<(*const ()) is !null>) = Layout {
7777
0,
7878
),
7979
),
80-
valid_range: (..=0) | (1..),
80+
valid_range: 0..=18446744073709551615,
8181
},
8282
tag_encoding: Niche {
8383
untagged_variant: 1,

‎tests/ui/type/pattern_types/range_patterns.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ error: layout_of(Option<(u32) is 1..>) = Layout {
9999
I32,
100100
false,
101101
),
102-
valid_range: (..=0) | (1..),
102+
valid_range: 0..=4294967295,
103103
},
104104
),
105105
fields: Arbitrary {
@@ -118,7 +118,7 @@ error: layout_of(Option<(u32) is 1..>) = Layout {
118118
I32,
119119
false,
120120
),
121-
valid_range: (..=0) | (1..),
121+
valid_range: 0..=4294967295,
122122
},
123123
tag_encoding: Niche {
124124
untagged_variant: 1,
@@ -210,7 +210,7 @@ error: layout_of(Option<NonZero<u32>>) = Layout {
210210
I32,
211211
false,
212212
),
213-
valid_range: (..=0) | (1..),
213+
valid_range: 0..=4294967295,
214214
},
215215
),
216216
fields: Arbitrary {
@@ -229,7 +229,7 @@ error: layout_of(Option<NonZero<u32>>) = Layout {
229229
I32,
230230
false,
231231
),
232-
valid_range: (..=0) | (1..),
232+
valid_range: 0..=4294967295,
233233
},
234234
tag_encoding: Niche {
235235
untagged_variant: 1,

0 commit comments

Comments
 (0)