Skip to content

Change impls of PartialEq and friends in libstd to be more generic #2245

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

Closed
wants to merge 1 commit into from
Closed
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
134 changes: 134 additions & 0 deletions text/0000-composable-impls.md
Original file line number Diff line number Diff line change
@@ -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<String>`, but
you cannot compare an `Option<&str>` with `Option<String>` or `Option<&[&str]>`
with `Option<Vec<String>>`.

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<String>` 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<T, U> PartialEq<Option<U>> for Option<T>
where
T: PartialEq<U>,
```

`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<T, U, E> PartialEq<Result<U, E>> for Result<T, E>
where
T: PartialEq<U>,
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<A1, A2, B1, B2> PartialEq<(A2, B2)> for (A1, B1)
where
A1: PartialEq<A2>,
B1: PartialEq<B2>,
{
```

# 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::<String>(&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?