From f730bdc95179fb91cc85d30ce257cf9b9d524039 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 12 Dec 2017 10:31:26 -0700 Subject: [PATCH] Change impls of `PartialEq` and friends in libstd to be more generic I can compare `&[&str]` with `Vec`. I should be able to compare `Option<&str>` with `Option` and `Option<&[&str]>` with `Option>`. --- text/0000-composable-impls.md | 134 ++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 text/0000-composable-impls.md diff --git a/text/0000-composable-impls.md b/text/0000-composable-impls.md new file mode 100644 index 00000000000..c517d2af3d7 --- /dev/null +++ b/text/0000-composable-impls.md @@ -0,0 +1,134 @@ +- Feature Name: more\_generic\_impls +- Start Date: 2017-12-12 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +Change most generic types in libstd and libcore to manually implement +`PartialEq` with more generic bounds than those currently generated by +`#[derive(PartialEq)]`. + + + +# Motivation +[motivation]: #motivation + +Currently traits like `PartialEq` don't compose as well as they could. It's +possible to compare `&str` with `String`, and `&[&str]` with `Vec`, but +you cannot compare an `Option<&str>` with `Option` or `Option<&[&str]>` +with `Option>`. + +This can be extremely frustrating for users. It forces them to either perform +unnecessary pattern matching, or litter their code with unnecessary (and +potentially expensive) conversions. Consider the amount of work required to +convert a `&[&str]` to a `Vec` just to do an equality comparison. + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +Most publicly exposed types in libstd with at least one type parameter would be +changed to no longer derive `PartialEq`, `Eq`, `PartialOrd` or `Ord`. Instead, +these types would be given manual impls of these traits, which compose on their +type parameters similar to the impl for `Vec`. + +Types which already manually implement these traits, but require type parameters +to be the same on both sides (such as `Box` and tuples) will have their +constraints loosened as well. + +From a brief audit of libcore and libstd, the list of types which would be +affected includes: + +- Option +- Result +- Range +- RangeFrom +- RangeTo +- RangeInclusive +- RangeToInclusive +- Tuples +- Box + +Generic types which currently derive `PartialEq`, but would not likely benefit +from this change include: + +- Iterator adapters +- NonZero +- Wrapping + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +All types which we decide should have their impls broadened would be given the following impls (using `Option` as +an example) + +```rust +impl PartialEq> for Option +where + T: PartialEq, +``` + +`Result` is a special case here, as it has more than one type parameter. In this +case, the impls will still require that the error be the same type. For example: + +```rust +impl PartialEq> for Result +where + T: PartialEq, + E: PartialEq, +{ +``` + +The reason for this being a special case is that this code is fairly common in +the wild: `assert_eq!(Ok(1), some_result);`. This code is relying on type +inference for `E`. It is much less common to compare errors. + +Tuples are another case with multiple type parameters. In this case, all type +parameters would be allowed to be separate. For example: + +```rust +impl PartialEq<(A2, B2)> for (A1, B1) +where + A1: PartialEq, + B1: PartialEq, +{ +``` + +# Drawbacks +[drawbacks]: #drawbacks + +This will affect type inference for code which is relying on a `PartialEq` impl +for type inference. For example, both of these lines would no longer compile: + +```rust +assert_eq!(Ok(String::from("Sean")), users.select(name).first(&conn)); +assert_eq!(Ok("Sean".into()), users.select(name).first::(&conn)); +``` + +This only affects types which have more than one `PartialEq` impl, such as +`String`. If the code were using virtually any other type that has only one impl +(such as integers, or anything using `derive`, it would continue to compile. +Code relying on `PartialEq` impls for type inference can already break due to +new impls being added in other crates. This is considered a semver compatible +change by [RFC #1105]. + +[RFC #1105]: https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md#minor-change-implementing-any-non-fundamental-trait + +# Rationale and alternatives +[alternatives]: #alternatives + +The most obvious alternative would be to change the code generated by +`#[derive(PartialEq)]` to allow type parameters to differ. This is almost always +what you want, but that change has a much higher potential to cause breakage. +At minimum, a crater run for both approaches would need to be done to consider +it. Even with the change to the code generated by derive, there are still +several manual impls in libstd which would need to be loosened. Finally, it is +not specified by [RFC #1105] whether changing code generated by derive is +considered a semver compatible change or not. For these reasons, I think the +conservative approach of only touching types in libstd is the best option. + +# Unresolved questions +[unresolved]: #unresolved-questions + +- Are there additional types that should be given this treatment?