Skip to content

Commit 09df883

Browse files
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 1b8f2e4 commit 09df883

12 files changed

Lines changed: 45 additions & 26 deletions

File tree

compiler/rustc_abi/src/lib.rs

Lines changed: 10 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,15 @@ 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+
if count == 1 {
2083+
// We only need one, so just pick the one closest to zero.
2084+
// Not only does that obviously use zero if it's possible, but it also
2085+
// simplifies testing things like `Option<char>`, since looking for `-1`
2086+
// is easier than looking for `1114112` (and matches clang's `WEOF`).
2087+
let next_up = size.sign_extend(v.end.wrapping_add(1)).unsigned_abs();
2088+
let next_down = size.sign_extend(v.start.wrapping_sub(1)).unsigned_abs();
2089+
if next_down <= next_up { move_start(v) } else { move_end(v) }
2090+
} else if v.start > v.end {
20842091
// zero is unavailable because wrapping occurs
20852092
move_end(v)
20862093
} else if v.start <= distance_end_zero {

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

Lines changed: 13 additions & 1 deletion
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;
@@ -20,7 +21,7 @@ fn make_some_bool(x: bool) -> Option<bool> {
2021
fn make_none_bool() -> Option<bool> {
2122
// CHECK-LABEL: i8 @make_none_bool()
2223
// CHECK-NEXT: start:
23-
// CHECK-NEXT: ret i8 2
24+
// CHECK-NEXT: ret i8 -1
2425
None
2526
}
2627

@@ -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: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ pub enum Giant {
2525
#[unsafe(no_mangle)]
2626
pub fn opt_bool_eq_discr(a: Option<bool>, b: Option<bool>) -> bool {
2727
// CHECK-LABEL: @opt_bool_eq_discr(
28-
// CHECK: %[[A:.+]] = icmp ne i8 %a, 2
29-
// CHECK: %[[B:.+]] = icmp eq i8 %b, 2
28+
// CHECK: %[[A:.+]] = icmp ne i8 %a, -1
29+
// CHECK: %[[B:.+]] = icmp eq i8 %b, -1
3030
// CHECK: %[[R:.+]] = xor i1 %[[A]], %[[B]]
3131
// CHECK: ret i1 %[[R]]
3232

@@ -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-match.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub enum Enum0 {
1717

1818
// CHECK-LABEL: define{{( dso_local)?}} noundef{{( range\(i8 [0-9]+, [0-9]+\))?}} i8 @match0(i8{{.+}}%0)
1919
// CHECK-NEXT: start:
20-
// CHECK-NEXT: %[[IS_B:.+]] = icmp eq i8 %0, 2
20+
// CHECK-NEXT: %[[IS_B:.+]] = icmp eq i8 %0, -1
2121
// CHECK-NEXT: %[[TRUNC:.+]] = and i8 %0, 1
2222
// CHECK-NEXT: %[[R:.+]] = select i1 %[[IS_B]], i8 13, i8 %[[TRUNC]]
2323
// CHECK-NEXT: ret i8 %[[R]]

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ pub fn result_match(x: Result<u64, i64>) -> u16 {
6060
#[no_mangle]
6161
pub fn option_bool_match(x: Option<bool>) -> char {
6262
// CHECK: %[[RAW:.+]] = load i8, ptr %x
63-
// CHECK: %[[IS_NONE:.+]] = icmp eq i8 %[[RAW]], 2
63+
// CHECK: %[[IS_NONE:.+]] = icmp eq i8 %[[RAW]], -1
6464
// CHECK: %[[OPT_DISCR:.+]] = select i1 %[[IS_NONE]], i64 0, i64 1
6565
// CHECK: %[[OPT_DISCR_T:.+]] = trunc nuw i64 %[[OPT_DISCR]] to i1
6666
// CHECK: br i1 %[[OPT_DISCR_T]], label %[[BB_SOME:.+]], label %[[BB_NONE:.+]]
@@ -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/function-arguments.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ pub fn return_slice(x: &[u16]) -> &[u16] {
297297
x
298298
}
299299

300-
// CHECK: { i16, i16 } @enum_id_1(i16 noundef{{( range\(i16 0, 3\))?}} %x.0, i16 %x.1)
300+
// CHECK: { i16, i16 } @enum_id_1(i16 noundef{{( range\(i16 -1, 2\))?}} %x.0, i16 %x.1)
301301
#[no_mangle]
302302
pub fn enum_id_1(x: Option<Result<u16, u16>>) -> Option<Result<u16, u16>> {
303303
x

tests/codegen-llvm/intrinsics/cold_path2.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ pub fn test(x: Option<bool>) {
2626
}
2727

2828
// CHECK-LABEL: void @test(i8{{.+}}%x)
29-
// CHECK: %[[IS_NONE:.+]] = icmp eq i8 %x, 2
29+
// CHECK: %[[IS_NONE:.+]] = icmp eq i8 %x, -1
3030
// CHECK: br i1 %[[IS_NONE]], label %bb2, label %bb1, !prof ![[NUM:[0-9]+]]
3131
// CHECK: bb1:
3232
// CHECK: path_a

tests/codegen-llvm/range-attribute.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ pub fn nonzero_int(x: NonZero<u128>) -> NonZero<u128> {
2323
x
2424
}
2525

26-
// CHECK: noundef range(i8 0, 3) i8 @optional_bool(i8{{.*}} range(i8 0, 3) %x)
26+
// CHECK: noundef range(i8 -1, 2) i8 @optional_bool(i8{{.*}} range(i8 -1, 2) %x)
2727
#[no_mangle]
2828
pub fn optional_bool(x: Option<bool>) -> Option<bool> {
2929
x

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/transmutability/enums/niche_optimization.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ fn bool() {
5454
assert::is_transmutable::<bool, OptionLike>();
5555
assert::is_transmutable::<V0, OptionLike>();
5656
assert::is_transmutable::<V1, OptionLike>();
57-
assert::is_transmutable::<V2, OptionLike>();
57+
assert::is_transmutable::<V255, OptionLike>();
5858
}
5959

6060
fn one_niche() {

0 commit comments

Comments
 (0)