Skip to content

Commit

Permalink
Remove SpecOptionPartialEq
Browse files Browse the repository at this point in the history
  • Loading branch information
clubby789 committed Mar 19, 2024
1 parent f296c16 commit 5f254d8
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 136 deletions.
8 changes: 1 addition & 7 deletions compiler/rustc_index_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,7 @@ mod newtype;
#[proc_macro]
#[cfg_attr(
feature = "nightly",
allow_internal_unstable(
step_trait,
rustc_attrs,
trusted_step,
spec_option_partial_eq,
min_specialization
)
allow_internal_unstable(step_trait, rustc_attrs, trusted_step, min_specialization)
)]
pub fn newtype_index(input: TokenStream) -> TokenStream {
newtype::newtype(input)
Expand Down
28 changes: 0 additions & 28 deletions compiler/rustc_index_macros/src/newtype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,32 +156,6 @@ impl Parse for Newtype {
}
};

let spec_partial_eq_impl = if let Lit::Int(max) = &max {
if let Ok(max_val) = max.base10_parse::<u32>() {
quote! {
#gate_rustc_only
impl core::option::SpecOptionPartialEq for #name {
#[inline]
fn eq(l: &Option<Self>, r: &Option<Self>) -> bool {
if #max_val < u32::MAX {
l.map(|i| i.as_u32()).unwrap_or(#max_val+1) == r.map(|i| i.as_u32()).unwrap_or(#max_val+1)
} else {
match (l, r) {
(Some(l), Some(r)) => r == l,
(None, None) => true,
_ => false
}
}
}
}
}
} else {
quote! {}
}
} else {
quote! {}
};

Ok(Self(quote! {
#(#attrs)*
#[derive(Clone, Copy, PartialEq, Eq, Hash, #(#derive_paths),*)]
Expand Down Expand Up @@ -283,8 +257,6 @@ impl Parse for Newtype {

#step

#spec_partial_eq_impl

impl From<#name> for u32 {
#[inline]
fn from(v: #name) -> u32 {
Expand Down
81 changes: 10 additions & 71 deletions library/core/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,8 +557,7 @@ use crate::iter::{self, FromIterator, FusedIterator, TrustedLen};
use crate::panicking::{panic, panic_str};
use crate::pin::Pin;
use crate::{
cmp, convert, hint, mem,
num::NonZero,
convert, hint, mem,
ops::{self, ControlFlow, Deref, DerefMut},
slice,
};
Expand All @@ -568,7 +567,7 @@ use crate::{
#[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"]
Expand Down Expand Up @@ -2146,86 +2145,26 @@ 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> {
#[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())
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl SpecOptionPartialEq for cmp::Ordering {
#[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)
}
}

/////////////////////////////////////////////////////////////////////////////
// The Option Iterators
/////////////////////////////////////////////////////////////////////////////
Expand Down
27 changes: 0 additions & 27 deletions tests/assembly/option-nonzero-eq.rs

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//@ compile-flags: -O -Zmerge-functions=disabled
//@ min-llvm-version: 18
#![crate_type = "lib"]
#![feature(generic_nonzero)]

Expand All @@ -7,9 +8,6 @@ use core::cmp::Ordering;
use core::ptr::NonNull;
use core::num::NonZero;

// See also tests/assembly/option-nonzero-eq.rs, for cases with `assume`s in the
// LLVM and thus don't optimize down clearly here, but do in assembly.

// CHECK-lABEL: @non_zero_eq
#[no_mangle]
pub fn non_zero_eq(l: Option<NonZero<u32>>, r: Option<NonZero<u32>>) -> bool {
Expand All @@ -36,3 +34,42 @@ pub fn non_null_eq(l: Option<NonNull<u8>>, r: Option<NonNull<u8>>) -> bool {
// CHECK-NEXT: ret i1
l == r
}

// CHECK-lABEL: @ordering_eq
#[no_mangle]
pub fn ordering_eq(l: Option<Ordering>, r: Option<Ordering>) -> bool {
// CHECK: start:
// CHECK-NEXT: icmp eq i8
// CHECK-NEXT: ret i1
l == r
}

#[derive(PartialEq)]
pub enum EnumWithNiche {
A,
B,
C,
D,
E,
F,
G,
}

// CHECK-lABEL: @niche_eq
#[no_mangle]
pub fn niche_eq(l: Option<EnumWithNiche>, r: Option<EnumWithNiche>) -> bool {
// CHECK: start:
// CHECK-NEXT: icmp eq i8
// CHECK-NEXT: ret i1
l == r
}

// FIXME: This should work too
// // FIXME-CHECK-lABEL: @bool_eq
// #[no_mangle]
// pub fn bool_eq(l: Option<bool>, r: Option<bool>) -> bool {
// // FIXME-CHECK: start:
// // FIXME-CHECK-NEXT: icmp eq i8
// // FIXME-CHECK-NEXT: ret i1
// l == r
// }

0 comments on commit 5f254d8

Please sign in to comment.