Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SpecOptionPartialEq #122024

Merged
merged 2 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
92 changes: 30 additions & 62 deletions library/core/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down Expand Up @@ -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,
}
}
}

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
jhpratt marked this conversation as resolved.
Show resolved Hide resolved
// // 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
// }
Loading