Skip to content

Commit

Permalink
Auto merge of #116167 - RalfJung:structural-eq, r=lcnr
Browse files Browse the repository at this point in the history
remove StructuralEq trait

The documentation given for the trait is outdated: *all* function pointers implement `PartialEq` and `Eq` these days. So the `StructuralEq` trait doesn't really seem to have any reason to exist any more.

One side-effect of this PR is that we allow matching on some consts that do not implement `Eq`. However, we already allowed matching on floats and consts containing floats, so this is not new, it is just allowed in more cases now. IMO it makes no sense at all to allow float matching but also sometimes require an `Eq` instance. If we want to require `Eq` we should adjust #115893 to check for `Eq`, and rule out float matching for good.

Fixes #115881
  • Loading branch information
bors committed Jan 26, 2024
2 parents 0c1fb2a + 0df7810 commit dd2559e
Show file tree
Hide file tree
Showing 76 changed files with 266 additions and 529 deletions.
13 changes: 0 additions & 13 deletions compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,6 @@ pub fn expand_deriving_eq(
) {
let span = cx.with_def_site_ctxt(span);

let structural_trait_def = TraitDef {
span,
path: path_std!(marker::StructuralEq),
skip_path_as_bound: true, // crucial!
needs_copy_as_bound_if_packed: false,
additional_bounds: Vec::new(),
supports_unions: true,
methods: Vec::new(),
associated_types: Vec::new(),
is_const: false,
};
structural_trait_def.expand(cx, mitem, item, push);

let trait_def = TraitDef {
span,
path: path_std!(cmp::Eq),
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_codegen_cranelift/example/mini_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,6 @@ unsafe impl<T: ?Sized> Freeze for &mut T {}
#[lang = "structural_peq"]
pub trait StructuralPartialEq {}

#[lang = "structural_teq"]
pub trait StructuralEq {}

#[lang = "not"]
pub trait Not {
type Output;
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_codegen_gcc/example/mini_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,6 @@ unsafe impl<T: ?Sized> Freeze for &mut T {}
#[lang = "structural_peq"]
pub trait StructuralPartialEq {}

#[lang = "structural_teq"]
pub trait StructuralEq {}

#[lang = "not"]
pub trait Not {
type Output;
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_codegen_gcc/tests/run/static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ mod libc {
#[lang = "structural_peq"]
pub trait StructuralPartialEq {}

#[lang = "structural_teq"]
pub trait StructuralEq {}

#[lang = "drop_in_place"]
#[allow(unconditional_recursion)]
pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/valtrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ pub(crate) fn const_to_valtree_inner<'tcx>(
}
// Trait objects are not allowed in type level constants, as we have no concept for
// resolving their backing type, even if we can do that at const eval time. We may
// hypothetically be able to allow `dyn StructuralEq` trait objects in the future,
// hypothetically be able to allow `dyn StructuralPartialEq` trait objects in the future,
// but it is unclear if this is useful.
ty::Dynamic(..) => Err(ValTreeCreationError::NonSupportedType),

Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,6 @@ language_item_table! {
Unsize, sym::unsize, unsize_trait, Target::Trait, GenericRequirement::Minimum(1);
/// Trait injected by `#[derive(PartialEq)]`, (i.e. "Partial EQ").
StructuralPeq, sym::structural_peq, structural_peq_trait, Target::Trait, GenericRequirement::None;
/// Trait injected by `#[derive(Eq)]`, (i.e. "Total EQ"; no, I will not apologize).
StructuralTeq, sym::structural_teq, structural_teq_trait, Target::Trait, GenericRequirement::None;
Copy, sym::copy, copy_trait, Target::Trait, GenericRequirement::Exact(0);
Clone, sym::clone, clone_trait, Target::Trait, GenericRequirement::None;
Sync, sym::sync, sync_trait, Target::Trait, GenericRequirement::Exact(0);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1360,9 +1360,9 @@ rustc_queries! {
///
/// This is only correct for ADTs. Call `is_structural_eq_shallow` to handle all types
/// correctly.
query has_structural_eq_impls(ty: Ty<'tcx>) -> bool {
query has_structural_eq_impl(ty: Ty<'tcx>) -> bool {
desc {
"computing whether `{}` implements `PartialStructuralEq` and `StructuralEq`",
"computing whether `{}` implements `StructuralPartialEq`",
ty
}
}
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1249,19 +1249,18 @@ impl<'tcx> Ty<'tcx> {
/// Primitive types (`u32`, `str`) have structural equality by definition. For composite data
/// types, equality for the type as a whole is structural when it is the same as equality
/// between all components (fields, array elements, etc.) of that type. For ADTs, structural
/// equality is indicated by an implementation of `PartialStructuralEq` and `StructuralEq` for
/// that type.
/// equality is indicated by an implementation of `StructuralPartialEq` for that type.
///
/// This function is "shallow" because it may return `true` for a composite type whose fields
/// are not `StructuralEq`. For example, `[T; 4]` has structural equality regardless of `T`
/// are not `StructuralPartialEq`. For example, `[T; 4]` has structural equality regardless of `T`
/// because equality for arrays is determined by the equality of each array element. If you
/// want to know whether a given call to `PartialEq::eq` will proceed structurally all the way
/// down, you will need to use a type visitor.
#[inline]
pub fn is_structural_eq_shallow(self, tcx: TyCtxt<'tcx>) -> bool {
match self.kind() {
// Look for an impl of both `PartialStructuralEq` and `StructuralEq`.
ty::Adt(..) => tcx.has_structural_eq_impls(self),
// Look for an impl of `StructuralPartialEq`.
ty::Adt(..) => tcx.has_structural_eq_impl(self),

// Primitive types that satisfy `Eq`.
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Str | ty::Never => true,
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ mir_build_extern_static_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
mir_build_float_pattern = floating-point types cannot be used in patterns
mir_build_indirect_structural_match =
to use a constant of type `{$non_sm_ty}` in a pattern, `{$non_sm_ty}` must be annotated with `#[derive(PartialEq, Eq)]`
to use a constant of type `{$non_sm_ty}` in a pattern, `{$non_sm_ty}` must be annotated with `#[derive(PartialEq)]`
mir_build_inform_irrefutable = `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
Expand Down Expand Up @@ -254,7 +254,7 @@ mir_build_non_partial_eq_match =
to use a constant of type `{$non_peq_ty}` in a pattern, the type must implement `PartialEq`
mir_build_nontrivial_structural_match =
to use a constant of type `{$non_sm_ty}` in a pattern, the constant's initializer must be trivial or `{$non_sm_ty}` must be annotated with `#[derive(PartialEq, Eq)]`
to use a constant of type `{$non_sm_ty}` in a pattern, the constant's initializer must be trivial or `{$non_sm_ty}` must be annotated with `#[derive(PartialEq)]`
mir_build_pattern_not_covered = refutable pattern in {$origin}
.pattern_ty = the matched value is of type `{$pattern_ty}`
Expand Down Expand Up @@ -297,9 +297,9 @@ mir_build_trailing_irrefutable_let_patterns = trailing irrefutable {$count ->
} into the body
mir_build_type_not_structural =
to use a constant of type `{$non_sm_ty}` in a pattern, `{$non_sm_ty}` must be annotated with `#[derive(PartialEq, Eq)]`
to use a constant of type `{$non_sm_ty}` in a pattern, `{$non_sm_ty}` must be annotated with `#[derive(PartialEq)]`
mir_build_type_not_structural_more_info = see https://doc.rust-lang.org/stable/std/marker/trait.StructuralEq.html for details
mir_build_type_not_structural_more_info = see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
mir_build_type_not_structural_tip = the traits must be derived, manual `impl`s are not sufficient
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@ symbols! {
Some,
SpanCtxt,
String,
StructuralEq,
StructuralPartialEq,
SubdiagnosticMessage,
Sync,
Expand Down Expand Up @@ -1625,7 +1624,6 @@ symbols! {
struct_variant,
structural_match,
structural_peq,
structural_teq,
sub,
sub_assign,
sub_with_overflow,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub fn search_for_structural_match_violation<'tcx>(

/// This implements the traversal over the structure of a given type to try to
/// find instances of ADTs (specifically structs or enums) that do not implement
/// the structural-match traits (`StructuralPartialEq` and `StructuralEq`).
/// `StructuralPartialEq`.
struct Search<'tcx> {
span: Span,

Expand Down
16 changes: 5 additions & 11 deletions compiler/rustc_ty_utils/src/structural_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@ use rustc_infer::infer::TyCtxtInferExt;
use rustc_trait_selection::traits::{ObligationCause, ObligationCtxt};

/// This method returns true if and only if `adt_ty` itself has been marked as
/// eligible for structural-match: namely, if it implements both
/// `StructuralPartialEq` and `StructuralEq` (which are respectively injected by
/// `#[derive(PartialEq)]` and `#[derive(Eq)]`).
/// eligible for structural-match: namely, if it implements
/// `StructuralPartialEq` (which is injected by `#[derive(PartialEq)]`).
///
/// Note that this does *not* recursively check if the substructure of `adt_ty`
/// implements the traits.
fn has_structural_eq_impls<'tcx>(tcx: TyCtxt<'tcx>, adt_ty: Ty<'tcx>) -> bool {
/// implements the trait.
fn has_structural_eq_impl<'tcx>(tcx: TyCtxt<'tcx>, adt_ty: Ty<'tcx>) -> bool {
let infcx = &tcx.infer_ctxt().build();
let cause = ObligationCause::dummy();

Expand All @@ -21,11 +20,6 @@ fn has_structural_eq_impls<'tcx>(tcx: TyCtxt<'tcx>, adt_ty: Ty<'tcx>) -> bool {
let structural_peq_def_id =
infcx.tcx.require_lang_item(LangItem::StructuralPeq, Some(cause.span));
ocx.register_bound(cause.clone(), ty::ParamEnv::empty(), adt_ty, structural_peq_def_id);
// for now, require `#[derive(Eq)]`. (Doing so is a hack to work around
// the type `for<'a> fn(&'a ())` failing to implement `Eq` itself.)
let structural_teq_def_id =
infcx.tcx.require_lang_item(LangItem::StructuralTeq, Some(cause.span));
ocx.register_bound(cause, ty::ParamEnv::empty(), adt_ty, structural_teq_def_id);

// We deliberately skip *reporting* fulfillment errors (via
// `report_fulfillment_errors`), for two reasons:
Expand All @@ -40,5 +34,5 @@ fn has_structural_eq_impls<'tcx>(tcx: TyCtxt<'tcx>, adt_ty: Ty<'tcx>) -> bool {
}

pub(crate) fn provide(providers: &mut Providers) {
providers.has_structural_eq_impls = has_structural_eq_impls;
providers.has_structural_eq_impl = has_structural_eq_impl;
}
23 changes: 20 additions & 3 deletions library/core/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ pub trait Unsize<T: ?Sized> {
/// Required trait for constants used in pattern matches.
///
/// Any type that derives `PartialEq` automatically implements this trait,
/// *regardless* of whether its type-parameters implement `Eq`.
/// *regardless* of whether its type-parameters implement `PartialEq`.
///
/// If a `const` item contains some type that does not implement this trait,
/// then that type either (1.) does not implement `PartialEq` (which means the
Expand All @@ -200,7 +200,7 @@ pub trait Unsize<T: ?Sized> {
/// a pattern match.
///
/// See also the [structural match RFC][RFC1445], and [issue 63438] which
/// motivated migrating from attribute-based design to this trait.
/// motivated migrating from an attribute-based design to this trait.
///
/// [RFC1445]: https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md
/// [issue 63438]: https://github.com/rust-lang/rust/issues/63438
Expand All @@ -218,7 +218,7 @@ marker_impls! {
isize, i8, i16, i32, i64, i128,
bool,
char,
str /* Technically requires `[u8]: StructuralEq` */,
str /* Technically requires `[u8]: StructuralPartialEq` */,
(),
{T, const N: usize} [T; N],
{T} [T],
Expand Down Expand Up @@ -275,13 +275,15 @@ marker_impls! {
#[unstable(feature = "structural_match", issue = "31434")]
#[diagnostic::on_unimplemented(message = "the type `{Self}` does not `#[derive(Eq)]`")]
#[lang = "structural_teq"]
#[cfg(bootstrap)]
pub trait StructuralEq {
// Empty.
}

// FIXME: Remove special cases of these types from the compiler pattern checking code and always check `T: StructuralEq` instead
marker_impls! {
#[unstable(feature = "structural_match", issue = "31434")]
#[cfg(bootstrap)]
StructuralEq for
usize, u8, u16, u32, u64, u128,
isize, i8, i16, i32, i64, i128,
Expand Down Expand Up @@ -859,6 +861,7 @@ impl<T: ?Sized> Default for PhantomData<T> {
impl<T: ?Sized> StructuralPartialEq for PhantomData<T> {}

#[unstable(feature = "structural_match", issue = "31434")]
#[cfg(bootstrap)]
impl<T: ?Sized> StructuralEq for PhantomData<T> {}

/// Compiler-internal trait used to indicate the type of enum discriminants.
Expand Down Expand Up @@ -1038,6 +1041,20 @@ pub trait PointerLike {}
#[unstable(feature = "adt_const_params", issue = "95174")]
#[diagnostic::on_unimplemented(message = "`{Self}` can't be used as a const parameter type")]
#[allow(multiple_supertrait_upcastable)]
#[cfg(not(bootstrap))]
pub trait ConstParamTy: StructuralPartialEq + Eq {}

/// A marker for types which can be used as types of `const` generic parameters.
///
/// These types must have a proper equivalence relation (`Eq`) and it must be automatically
/// derived (`StructuralPartialEq`). There's a hard-coded check in the compiler ensuring
/// that all fields are also `ConstParamTy`, which implies that recursively, all fields
/// are `StructuralPartialEq`.
#[lang = "const_param_ty"]
#[unstable(feature = "adt_const_params", issue = "95174")]
#[rustc_on_unimplemented(message = "`{Self}` can't be used as a const parameter type")]
#[allow(multiple_supertrait_upcastable)]
#[cfg(bootstrap)]
pub trait ConstParamTy: StructuralEq + StructuralPartialEq + Eq {}

/// Derive macro generating an impl of the trait `ConstParamTy`.
Expand Down
5 changes: 3 additions & 2 deletions library/core/src/tuple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use crate::cmp::Ordering::{self, *};
use crate::marker::ConstParamTy;
use crate::marker::{StructuralEq, StructuralPartialEq};
use crate::marker::StructuralPartialEq;

// Recursive macro for implementing n-ary tuple functions and operations
//
Expand Down Expand Up @@ -64,7 +64,8 @@ macro_rules! tuple_impls {
maybe_tuple_doc! {
$($T)+ @
#[unstable(feature = "structural_match", issue = "31434")]
impl<$($T),+> StructuralEq for ($($T,)+)
#[cfg(bootstrap)]
impl<$($T),+> crate::marker::StructuralEq for ($($T,)+)
{}
}

Expand Down
2 changes: 0 additions & 2 deletions src/tools/clippy/tests/ui/crashes/ice-6254.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ fn main() {
// This used to cause an ICE (https://github.com/rust-lang/rust/issues/78071)
match FOO_REF_REF {
FOO_REF_REF => {},
//~^ ERROR: to use a constant of type `Foo` in a pattern, `Foo` must be annotated
//~| NOTE: for more information, see issue #62411 <https://github.com/rust-lang/ru
Foo(_) => {},
}
}
15 changes: 0 additions & 15 deletions src/tools/clippy/tests/ui/crashes/ice-6254.stderr

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ impl std::marker::ConstParamTy for ImplementsConstParamTy {}
struct CantParam(ImplementsConstParamTy);

impl std::marker::ConstParamTy for CantParam {}
//~^ error: the type `CantParam` does not `#[derive(Eq)]`
//~| error: the type `CantParam` does not `#[derive(PartialEq)]`
//~^ error: the type `CantParam` does not `#[derive(PartialEq)]`
//~| the trait bound `CantParam: Eq` is not satisfied

#[derive(std::marker::ConstParamTy)]
//~^ error: the type `CantParamDerive` does not `#[derive(Eq)]`
//~| error: the type `CantParamDerive` does not `#[derive(PartialEq)]`
//~^ error: the type `CantParamDerive` does not `#[derive(PartialEq)]`
//~| the trait bound `CantParamDerive: Eq` is not satisfied
struct CantParamDerive(ImplementsConstParamTy);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,8 @@ LL | impl std::marker::ConstParamTy for CantParam {}
note: required by a bound in `ConstParamTy`
--> $SRC_DIR/core/src/marker.rs:LL:COL

error[E0277]: the type `CantParam` does not `#[derive(Eq)]`
--> $DIR/const_param_ty_impl_no_structural_eq.rs:10:36
|
LL | impl std::marker::ConstParamTy for CantParam {}
| ^^^^^^^^^ the trait `StructuralEq` is not implemented for `CantParam`
|
note: required by a bound in `ConstParamTy`
--> $SRC_DIR/core/src/marker.rs:LL:COL

error[E0277]: the trait bound `CantParamDerive: Eq` is not satisfied
--> $DIR/const_param_ty_impl_no_structural_eq.rs:15:10
--> $DIR/const_param_ty_impl_no_structural_eq.rs:14:10
|
LL | #[derive(std::marker::ConstParamTy)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Eq` is not implemented for `CantParamDerive`
Expand All @@ -46,7 +37,7 @@ LL | struct CantParamDerive(ImplementsConstParamTy);
|

error[E0277]: the type `CantParamDerive` does not `#[derive(PartialEq)]`
--> $DIR/const_param_ty_impl_no_structural_eq.rs:15:10
--> $DIR/const_param_ty_impl_no_structural_eq.rs:14:10
|
LL | #[derive(std::marker::ConstParamTy)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `StructuralPartialEq` is not implemented for `CantParamDerive`
Expand All @@ -55,16 +46,6 @@ note: required by a bound in `ConstParamTy`
--> $SRC_DIR/core/src/marker.rs:LL:COL
= note: this error originates in the derive macro `std::marker::ConstParamTy` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the type `CantParamDerive` does not `#[derive(Eq)]`
--> $DIR/const_param_ty_impl_no_structural_eq.rs:15:10
|
LL | #[derive(std::marker::ConstParamTy)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `StructuralEq` is not implemented for `CantParamDerive`
|
note: required by a bound in `ConstParamTy`
--> $SRC_DIR/core/src/marker.rs:LL:COL
= note: this error originates in the derive macro `std::marker::ConstParamTy` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 6 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0277`.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![allow(incomplete_features)]
#![feature(adt_const_params, structural_match)]
#![feature(adt_const_params)]

union Union {
a: u8,
Expand All @@ -11,7 +11,6 @@ impl PartialEq for Union {
}
}
impl Eq for Union {}
impl std::marker::StructuralEq for Union {}

impl std::marker::ConstParamTy for Union {}
//~^ ERROR the type `Union` does not `#[derive(PartialEq)]`
Expand All @@ -28,7 +27,6 @@ impl PartialEq for UnionDerive {
}
}
impl Eq for UnionDerive {}
impl std::marker::StructuralEq for UnionDerive {}


fn main() {}
Loading

0 comments on commit dd2559e

Please sign in to comment.