Skip to content

Commit b85f91f

Browse files
committed
Swap the 2 variants of Option<T>
That allows us to have a better path from Result<T, E> to Option<T> in the general case, without niche-filling optimisations.
1 parent 3615093 commit b85f91f

File tree

3 files changed

+110
-9
lines changed

3 files changed

+110
-9
lines changed

src/libcore/option.rs

+106-5
Original file line numberDiff line numberDiff line change
@@ -146,23 +146,23 @@
146146
#![stable(feature = "rust1", since = "1.0.0")]
147147

148148
use iter::{FromIterator, FusedIterator, TrustedLen};
149-
use {mem, ops};
149+
use {cmp, intrinsics, mem, ops};
150150

151151
// Note that this is not a lang item per se, but it has a hidden dependency on
152152
// `Iterator`, which is one. The compiler assumes that the `next` method of
153153
// `Iterator` is an enumeration with one type parameter and two variants,
154154
// which basically means it must be `Option`.
155155

156156
/// The `Option` type. See [the module level documentation](index.html) for more.
157-
#[derive(Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)]
157+
#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)] // PartialOrd and Ord by hand below.
158158
#[stable(feature = "rust1", since = "1.0.0")]
159159
pub enum Option<T> {
160-
/// No value
161-
#[stable(feature = "rust1", since = "1.0.0")]
162-
None,
163160
/// Some value `T`
164161
#[stable(feature = "rust1", since = "1.0.0")]
165162
Some(#[stable(feature = "rust1", since = "1.0.0")] T),
163+
/// No value
164+
#[stable(feature = "rust1", since = "1.0.0")]
165+
None,
166166
}
167167

168168
/////////////////////////////////////////////////////////////////////////////
@@ -981,6 +981,107 @@ impl<T> From<T> for Option<T> {
981981
}
982982
}
983983

984+
// The Option<T> type used to be defined as { None, Some(T) }, but for codegen
985+
// reasons we reversed it in #49499 to reduce the amount of work that needs
986+
// to be done for Result<T, ()> <-> Option<T> conversions. Keeping the derived
987+
// Ord and PartialOrd implementations would make that swap a breaking change.
988+
#[stable(feature = "rust1", since = "1.0.0")]
989+
impl<T> Ord for Option<T>
990+
where
991+
T: Ord,
992+
{
993+
#[inline]
994+
fn cmp(&self, other: &Self) -> cmp::Ordering {
995+
let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize;
996+
let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize;
997+
if self_discr == other_discr {
998+
match (self, other) {
999+
(&Some(ref this), &Some(ref other)) => this.cmp(other),
1000+
_ => cmp::Ordering::Equal,
1001+
}
1002+
} else {
1003+
other_discr.cmp(&self_discr)
1004+
}
1005+
}
1006+
}
1007+
1008+
// See comment on the Ord impl above.
1009+
#[stable(feature = "rust1", since = "1.0.0")]
1010+
impl<T> PartialOrd for Option<T>
1011+
where
1012+
T: PartialOrd,
1013+
{
1014+
#[inline]
1015+
fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
1016+
let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize;
1017+
let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize;
1018+
if self_discr == other_discr {
1019+
match (self, other) {
1020+
(&Some(ref this), &Some(ref other)) => this.partial_cmp(other),
1021+
_ => Some(cmp::Ordering::Equal),
1022+
}
1023+
} else {
1024+
other_discr.partial_cmp(&self_discr)
1025+
}
1026+
}
1027+
1028+
#[inline]
1029+
fn lt(&self, other: &Self) -> bool {
1030+
let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize;
1031+
let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize;
1032+
if self_discr == other_discr {
1033+
match (self, other) {
1034+
(&Some(ref this), &Some(ref other)) => this < other,
1035+
_ => false,
1036+
}
1037+
} else {
1038+
other_discr < self_discr
1039+
}
1040+
}
1041+
1042+
#[inline]
1043+
fn le(&self, other: &Self) -> bool {
1044+
let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize;
1045+
let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize;
1046+
if self_discr == other_discr {
1047+
match (self, other) {
1048+
(&Some(ref this), &Some(ref other)) => this <= other,
1049+
_ => true,
1050+
}
1051+
} else {
1052+
other_discr <= self_discr
1053+
}
1054+
}
1055+
1056+
#[inline]
1057+
fn gt(&self, other: &Self) -> bool {
1058+
let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize;
1059+
let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize;
1060+
if self_discr == other_discr {
1061+
match (self, other) {
1062+
(&Some(ref this), &Some(ref other)) => this > other,
1063+
_ => false,
1064+
}
1065+
} else {
1066+
other_discr > self_discr
1067+
}
1068+
}
1069+
1070+
#[inline]
1071+
fn ge(&self, other: &Self) -> bool {
1072+
let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize;
1073+
let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize;
1074+
if self_discr == other_discr {
1075+
match (self, other) {
1076+
(&Some(ref this), &Some(ref other)) => this >= other,
1077+
_ => true,
1078+
}
1079+
} else {
1080+
other_discr >= self_discr
1081+
}
1082+
}
1083+
}
1084+
9841085
/////////////////////////////////////////////////////////////////////////////
9851086
// The Option Iterators
9861087
/////////////////////////////////////////////////////////////////////////////

src/test/compile-fail/issue-2111.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
fn foo(a: Option<usize>, b: Option<usize>) {
1212
match (a,b) {
13-
//~^ ERROR: non-exhaustive patterns: `(None, None)` not covered
13+
//~^ ERROR: non-exhaustive patterns: `(Some(_), Some(_))` not covered
1414
(Some(a), Some(b)) if a == b => { }
1515
(Some(_), None) |
1616
(None, Some(_)) => { }

src/test/mir-opt/match_false_edges.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ fn main() {
5555
// _2 = std::option::Option<i32>::Some(const 42i32,);
5656
// _3 = discriminant(_2);
5757
// _6 = discriminant(_2);
58-
// switchInt(move _6) -> [0isize: bb6, 1isize: bb4, otherwise: bb8];
58+
// switchInt(move _6) -> [0isize: bb4, 1isize: bb6, otherwise: bb8];
5959
// }
6060
// bb1: {
6161
// resume;
@@ -119,7 +119,7 @@ fn main() {
119119
// _2 = std::option::Option<i32>::Some(const 42i32,);
120120
// _3 = discriminant(_2);
121121
// _6 = discriminant(_2);
122-
// switchInt(move _6) -> [0isize: bb5, 1isize: bb4, otherwise: bb8];
122+
// switchInt(move _6) -> [0isize: bb4, 1isize: bb5, otherwise: bb8];
123123
// }
124124
// bb1: {
125125
// resume;
@@ -183,7 +183,7 @@ fn main() {
183183
// _2 = std::option::Option<i32>::Some(const 1i32,);
184184
// _3 = discriminant(_2);
185185
// _8 = discriminant(_2);
186-
// switchInt(move _8) -> [1isize: bb4, otherwise: bb5];
186+
// switchInt(move _8) -> [0isize: bb4, otherwise: bb5];
187187
// }
188188
// bb1: {
189189
// resume;

0 commit comments

Comments
 (0)