Rollup merge of rust-lang#144330 - gewitternacht:document-clone-eq, r=Amanieu

document assumptions about `Clone` and `Eq` traits

Most standard library collections break if `Clone` has a non-standard implementation which violates `x.clone() == x`. [Here](https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=b7fc6dfa8410cbb673eb8d38393d81de) the resulting broken behaviour of different collections is shown. I originally created an issue at rust-lang/hashbrown#629, but the conclusion there was that `x.clone()` resulting in an object that compares equal to the original one is probably a very universal assumption. However, this assumption is (to my knowledge) not documented anywhere.

I propose to make this assumption explicit in the `Clone` trait documentation. The property that seems the most reasonable to me is the following: When implementing both `Clone` and `PartialEq`, then
```text
x == x -> x.clone() == x
```
is expected to hold. This way, when also implementing `Eq`, it automatically follows that `x.clone() == x` has to hold, which should be enough for the collections to not break. At the same time, the property also works for the "normal" elements of a type with `PartialEq`. For the wording, I tried to follow the [`Hash` and `Eq`](https://doc.rust-lang.org/std/hash/trait.Hash.html#hash-and-eq) documentation.

As I am fairly new to Rust, it might well be that this property cannot be generally expected – it seems reasonable to me, but any counter-examples or critique, both content- and wording-wise, would be very welcome. If the property turns out to be too general, I would suggest to at least document the assumption of `x.clone() == x` for the collections somehow.

An additional thought of mine:
If it is indeed generally expected that `x == x -> x.clone() == x`, then, for the sake of completeness, one could also define that `x != x -> y != y for y = x.clone()` should hold, i.e., that an object that did not compare equal to itself before cloning, should also not compare equal to itself afterwards.

Author: Zalathar

library/core/src/clone.rs

@@ -139,6 +139,34 @@ mod uninit;
 /// // Note: With the manual implementations the above line will compile.
 /// ```
 ///
+/// ## `Clone` and `PartialEq`/`Eq`
+/// `Clone` is intended for the duplication of objects. Consequently, when implementing
+/// both `Clone` and [`PartialEq`], the following property is expected to hold:
+/// ```text
+/// x == x -> x.clone() == x
+/// ```
+/// In other words, if an object compares equal to itself,
+/// its clone must also compare equal to the original.
+///
+/// For types that also implement [`Eq`] – for which `x == x` always holds –
+/// this implies that `x.clone() == x` must always be true.
+/// Standard library collections such as
+/// [`HashMap`], [`HashSet`], [`BTreeMap`], [`BTreeSet`] and [`BinaryHeap`]
+/// rely on their keys respecting this property for correct behavior.
+/// Furthermore, these collections require that cloning a key preserves the outcome of the
+/// [`Hash`] and [`Ord`] methods. Thankfully, this follows automatically from `x.clone() == x`
+/// if `Hash` and `Ord` are correctly implemented according to their own requirements.
+///
+/// When deriving both `Clone` and [`PartialEq`] using `#[derive(Clone, PartialEq)]`
+/// or when additionally deriving [`Eq`] using `#[derive(Clone, PartialEq, Eq)]`,
+/// then this property is automatically upheld – provided that it is satisfied by
+/// the underlying types.
+///
+/// Violating this property is a logic error. The behavior resulting from a logic error is not
+/// specified, but users of the trait must ensure that such logic errors do *not* result in
+/// undefined behavior. This means that `unsafe` code **must not** rely on this property
+/// being satisfied.
+///
 /// ## Additional implementors
 ///
 /// In addition to the [implementors listed below][impls],
@@ -152,6 +180,11 @@ mod uninit;
 ///   (even if the referent doesn't),
 ///   while variables captured by mutable reference never implement `Clone`.
 ///
+/// [`HashMap`]: ../../std/collections/struct.HashMap.html
+/// [`HashSet`]: ../../std/collections/struct.HashSet.html
+/// [`BTreeMap`]: ../../std/collections/struct.BTreeMap.html
+/// [`BTreeSet`]: ../../std/collections/struct.BTreeSet.html
+/// [`BinaryHeap`]: ../../std/collections/struct.BinaryHeap.html
 /// [impls]: #implementors
 #[stable(feature = "rust1", since = "1.0.0")]
 #[lang = "clone"]