From b85f91f8da2572b9a1b97b2a0ac0563238a7057b Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Thu, 29 Mar 2018 17:28:15 +0200 Subject: [PATCH] Swap the 2 variants of Option That allows us to have a better path from Result to Option in the general case, without niche-filling optimisations. --- src/libcore/option.rs | 111 ++++++++++++++++++++++++-- src/test/compile-fail/issue-2111.rs | 2 +- src/test/mir-opt/match_false_edges.rs | 6 +- 3 files changed, 110 insertions(+), 9 deletions(-) diff --git a/src/libcore/option.rs b/src/libcore/option.rs index 570f745f8330c..2e445d0ba0daf 100644 --- a/src/libcore/option.rs +++ b/src/libcore/option.rs @@ -146,7 +146,7 @@ #![stable(feature = "rust1", since = "1.0.0")] use iter::{FromIterator, FusedIterator, TrustedLen}; -use {mem, ops}; +use {cmp, intrinsics, mem, ops}; // Note that this is not a lang item per se, but it has a hidden dependency on // `Iterator`, which is one. The compiler assumes that the `next` method of @@ -154,15 +154,15 @@ use {mem, ops}; // which basically means it must be `Option`. /// The `Option` type. See [the module level documentation](index.html) for more. -#[derive(Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)] +#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)] // PartialOrd and Ord by hand below. #[stable(feature = "rust1", since = "1.0.0")] pub enum Option { - /// No value - #[stable(feature = "rust1", since = "1.0.0")] - None, /// Some value `T` #[stable(feature = "rust1", since = "1.0.0")] Some(#[stable(feature = "rust1", since = "1.0.0")] T), + /// No value + #[stable(feature = "rust1", since = "1.0.0")] + None, } ///////////////////////////////////////////////////////////////////////////// @@ -981,6 +981,107 @@ impl From for Option { } } +// The Option type used to be defined as { None, Some(T) }, but for codegen +// reasons we reversed it in #49499 to reduce the amount of work that needs +// to be done for Result <-> Option conversions. Keeping the derived +// Ord and PartialOrd implementations would make that swap a breaking change. +#[stable(feature = "rust1", since = "1.0.0")] +impl Ord for Option +where + T: Ord, +{ + #[inline] + fn cmp(&self, other: &Self) -> cmp::Ordering { + let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize; + let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize; + if self_discr == other_discr { + match (self, other) { + (&Some(ref this), &Some(ref other)) => this.cmp(other), + _ => cmp::Ordering::Equal, + } + } else { + other_discr.cmp(&self_discr) + } + } +} + +// See comment on the Ord impl above. +#[stable(feature = "rust1", since = "1.0.0")] +impl PartialOrd for Option +where + T: PartialOrd, +{ + #[inline] + fn partial_cmp(&self, other: &Self) -> Option { + let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize; + let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize; + if self_discr == other_discr { + match (self, other) { + (&Some(ref this), &Some(ref other)) => this.partial_cmp(other), + _ => Some(cmp::Ordering::Equal), + } + } else { + other_discr.partial_cmp(&self_discr) + } + } + + #[inline] + fn lt(&self, other: &Self) -> bool { + let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize; + let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize; + if self_discr == other_discr { + match (self, other) { + (&Some(ref this), &Some(ref other)) => this < other, + _ => false, + } + } else { + other_discr < self_discr + } + } + + #[inline] + fn le(&self, other: &Self) -> bool { + let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize; + let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize; + if self_discr == other_discr { + match (self, other) { + (&Some(ref this), &Some(ref other)) => this <= other, + _ => true, + } + } else { + other_discr <= self_discr + } + } + + #[inline] + fn gt(&self, other: &Self) -> bool { + let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize; + let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize; + if self_discr == other_discr { + match (self, other) { + (&Some(ref this), &Some(ref other)) => this > other, + _ => false, + } + } else { + other_discr > self_discr + } + } + + #[inline] + fn ge(&self, other: &Self) -> bool { + let self_discr = unsafe { intrinsics::discriminant_value(self) } as isize; + let other_discr = unsafe { intrinsics::discriminant_value(other) } as isize; + if self_discr == other_discr { + match (self, other) { + (&Some(ref this), &Some(ref other)) => this >= other, + _ => true, + } + } else { + other_discr >= self_discr + } + } +} + ///////////////////////////////////////////////////////////////////////////// // The Option Iterators ///////////////////////////////////////////////////////////////////////////// diff --git a/src/test/compile-fail/issue-2111.rs b/src/test/compile-fail/issue-2111.rs index 8180ce52bdbc1..7ef4bbf164c43 100644 --- a/src/test/compile-fail/issue-2111.rs +++ b/src/test/compile-fail/issue-2111.rs @@ -10,7 +10,7 @@ fn foo(a: Option, b: Option) { match (a,b) { - //~^ ERROR: non-exhaustive patterns: `(None, None)` not covered + //~^ ERROR: non-exhaustive patterns: `(Some(_), Some(_))` not covered (Some(a), Some(b)) if a == b => { } (Some(_), None) | (None, Some(_)) => { } diff --git a/src/test/mir-opt/match_false_edges.rs b/src/test/mir-opt/match_false_edges.rs index 53f178619975e..ad8279082dd53 100644 --- a/src/test/mir-opt/match_false_edges.rs +++ b/src/test/mir-opt/match_false_edges.rs @@ -55,7 +55,7 @@ fn main() { // _2 = std::option::Option::Some(const 42i32,); // _3 = discriminant(_2); // _6 = discriminant(_2); -// switchInt(move _6) -> [0isize: bb6, 1isize: bb4, otherwise: bb8]; +// switchInt(move _6) -> [0isize: bb4, 1isize: bb6, otherwise: bb8]; // } // bb1: { // resume; @@ -119,7 +119,7 @@ fn main() { // _2 = std::option::Option::Some(const 42i32,); // _3 = discriminant(_2); // _6 = discriminant(_2); -// switchInt(move _6) -> [0isize: bb5, 1isize: bb4, otherwise: bb8]; +// switchInt(move _6) -> [0isize: bb4, 1isize: bb5, otherwise: bb8]; // } // bb1: { // resume; @@ -183,7 +183,7 @@ fn main() { // _2 = std::option::Option::Some(const 1i32,); // _3 = discriminant(_2); // _8 = discriminant(_2); -// switchInt(move _8) -> [1isize: bb4, otherwise: bb5]; +// switchInt(move _8) -> [0isize: bb4, otherwise: bb5]; // } // bb1: { // resume;