about summary refs log tree commit diff
path: root/library/core
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-03-22 04:06:25 +0000
committerbors <bors@rust-lang.org>2024-03-22 04:06:25 +0000
commitcdb683f6e4b0774b85c60eebe12af87f29d8ee4d (patch)
treee54736c22d842a591a8f768e7f784806b67b9ac0 /library/core
parentb57a10c39d2a2da7389d029595d7fff6ac9cbf5a (diff)
parentf8fd23a2add24de796b2fab197920d3fd349941a (diff)
downloadrust-cdb683f6e4b0774b85c60eebe12af87f29d8ee4d.tar.gz
rust-cdb683f6e4b0774b85c60eebe12af87f29d8ee4d.zip
Auto merge of #122024 - clubby789:remove-spec-option-pe, r=jhpratt
Remove SpecOptionPartialEq

With the recent LLVM bump, the specialization for Option::partial_eq on types with niches is no longer necessary. I kept the manual implementation as it still gives us better codegen than the derive (will look at this seperately).

Also implemented PartialOrd/Ord by hand as it _somewhat_ improves codegen for #49892: https://godbolt.org/z/vx5Y6oW4Y
Diffstat (limited to 'library/core')
-rw-r--r--library/core/src/option.rs92
1 files changed, 30 insertions, 62 deletions
diff --git a/library/core/src/option.rs b/library/core/src/option.rs
index 5027e326a9d..0083d15efae 100644
--- a/library/core/src/option.rs
+++ b/library/core/src/option.rs
@@ -558,17 +558,16 @@ use crate::panicking::{panic, panic_str};
 use crate::pin::Pin;
 use crate::{
     cmp, convert, hint, mem,
-    num::NonZero,
     ops::{self, ControlFlow, Deref, DerefMut},
     slice,
 };
 
 /// The `Option` type. See [the module level documentation](self) for more.
-#[derive(Copy, PartialOrd, Eq, Ord, Debug, Hash)]
+#[derive(Copy, Eq, Debug, Hash)]
 #[rustc_diagnostic_item = "Option"]
 #[lang = "Option"]
 #[stable(feature = "rust1", since = "1.0.0")]
-#[allow(clippy::derived_hash_with_manual_eq)] // PartialEq is specialized
+#[allow(clippy::derived_hash_with_manual_eq)] // PartialEq is manually implemented equivalently
 pub enum Option<T> {
     /// No value.
     #[lang = "None"]
@@ -2146,83 +2145,52 @@ impl<'a, T> From<&'a mut Option<T>> for Option<&'a mut T> {
     }
 }
 
+// Ideally, LLVM should be able to optimize our derive code to this.
+// Once https://github.com/llvm/llvm-project/issues/52622 is fixed, we can
+// go back to deriving `PartialEq`.
 #[stable(feature = "rust1", since = "1.0.0")]
 impl<T> crate::marker::StructuralPartialEq for Option<T> {}
 #[stable(feature = "rust1", since = "1.0.0")]
 impl<T: PartialEq> PartialEq for Option<T> {
     #[inline]
     fn eq(&self, other: &Self) -> bool {
-        SpecOptionPartialEq::eq(self, other)
-    }
-}
-
-/// This specialization trait is a workaround for LLVM not currently (2023-01)
-/// being able to optimize this itself, even though Alive confirms that it would
-/// be legal to do so: <https://github.com/llvm/llvm-project/issues/52622>
-///
-/// Once that's fixed, `Option` should go back to deriving `PartialEq`, as
-/// it used to do before <https://github.com/rust-lang/rust/pull/103556>.
-#[unstable(feature = "spec_option_partial_eq", issue = "none", reason = "exposed only for rustc")]
-#[doc(hidden)]
-pub trait SpecOptionPartialEq: Sized {
-    fn eq(l: &Option<Self>, other: &Option<Self>) -> bool;
-}
-
-#[unstable(feature = "spec_option_partial_eq", issue = "none", reason = "exposed only for rustc")]
-impl<T: PartialEq> SpecOptionPartialEq for T {
-    #[inline]
-    default fn eq(l: &Option<T>, r: &Option<T>) -> bool {
-        match (l, r) {
+        // Spelling out the cases explicitly optimizes better than
+        // `_ => false`
+        match (self, other) {
             (Some(l), Some(r)) => *l == *r,
+            (Some(_), None) => false,
+            (None, Some(_)) => false,
             (None, None) => true,
-            _ => false,
         }
     }
 }
 
-macro_rules! non_zero_option {
-    ( $( #[$stability: meta] $NZ:ty; )+ ) => {
-        $(
-            #[$stability]
-            impl SpecOptionPartialEq for $NZ {
-                #[inline]
-                fn eq(l: &Option<Self>, r: &Option<Self>) -> bool {
-                    l.map(Self::get).unwrap_or(0) == r.map(Self::get).unwrap_or(0)
-                }
-            }
-        )+
-    };
-}
-
-non_zero_option! {
-    #[stable(feature = "nonzero", since = "1.28.0")] NonZero<u8>;
-    #[stable(feature = "nonzero", since = "1.28.0")] NonZero<u16>;
-    #[stable(feature = "nonzero", since = "1.28.0")] NonZero<u32>;
-    #[stable(feature = "nonzero", since = "1.28.0")] NonZero<u64>;
-    #[stable(feature = "nonzero", since = "1.28.0")] NonZero<u128>;
-    #[stable(feature = "nonzero", since = "1.28.0")] NonZero<usize>;
-    #[stable(feature = "signed_nonzero", since = "1.34.0")] NonZero<i8>;
-    #[stable(feature = "signed_nonzero", since = "1.34.0")] NonZero<i16>;
-    #[stable(feature = "signed_nonzero", since = "1.34.0")] NonZero<i32>;
-    #[stable(feature = "signed_nonzero", since = "1.34.0")] NonZero<i64>;
-    #[stable(feature = "signed_nonzero", since = "1.34.0")] NonZero<i128>;
-    #[stable(feature = "signed_nonzero", since = "1.34.0")] NonZero<isize>;
-}
-
-#[stable(feature = "nonnull", since = "1.25.0")]
-impl<T> SpecOptionPartialEq for crate::ptr::NonNull<T> {
+// Manually implementing here somewhat improves codegen for
+// https://github.com/rust-lang/rust/issues/49892, although still
+// not optimal.
+#[stable(feature = "rust1", since = "1.0.0")]
+impl<T: PartialOrd> PartialOrd for Option<T> {
     #[inline]
-    fn eq(l: &Option<Self>, r: &Option<Self>) -> bool {
-        l.map(Self::as_ptr).unwrap_or_else(|| crate::ptr::null_mut())
-            == r.map(Self::as_ptr).unwrap_or_else(|| crate::ptr::null_mut())
+    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
+        match (self, other) {
+            (Some(l), Some(r)) => l.partial_cmp(r),
+            (Some(_), None) => Some(cmp::Ordering::Greater),
+            (None, Some(_)) => Some(cmp::Ordering::Less),
+            (None, None) => Some(cmp::Ordering::Equal),
+        }
     }
 }
 
 #[stable(feature = "rust1", since = "1.0.0")]
-impl SpecOptionPartialEq for cmp::Ordering {
+impl<T: Ord> Ord for Option<T> {
     #[inline]
-    fn eq(l: &Option<Self>, r: &Option<Self>) -> bool {
-        l.map_or(2, |x| x as i8) == r.map_or(2, |x| x as i8)
+    fn cmp(&self, other: &Self) -> cmp::Ordering {
+        match (self, other) {
+            (Some(l), Some(r)) => l.cmp(r),
+            (Some(_), None) => cmp::Ordering::Greater,
+            (None, Some(_)) => cmp::Ordering::Less,
+            (None, None) => cmp::Ordering::Equal,
+        }
     }
 }