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

impl PartialEq<Vec<T>> for &'_ [T] / [T; N] #2917

Closed
sollyucko opened this issue Apr 28, 2020 · 11 comments
Closed

impl PartialEq<Vec<T>> for &'_ [T] / [T; N] #2917

sollyucko opened this issue Apr 28, 2020 · 11 comments

Comments

@sollyucko
Copy link

Currently:

But neither [T; N], &'_ [T], &'_ mut [T], nor [T] implement PartialEq<Vec<T>>. This means that equality tests have to be pointlessly flipped. Is there any reason for this behavior? Would it be possible to fix this?

@shepmaster
Copy link
Member

Is there any reason for this behavior?

Yes, the orphan rules. One is defined in libcore, the other is not.

Would it be possible to fix this?

Not really.


The RFCs repo isn't really intended for questions. I encourage you to instead search rust-lang/rust repo or the general internet, then post questions on the user's forum or Discord.

@shepmaster
Copy link
Member

You can also check out the Rust source code and make the change yourself to see the compiler errors.

@sollyucko
Copy link
Author

sollyucko commented Apr 29, 2020

My change seems to build & pass the tests...:

diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs
index b4a9da84787..78dab89be79 100644
--- a/src/liballoc/vec.rs
+++ b/src/liballoc/vec.rs
@@ -2340,6 +2340,8 @@ macro_rules! __impl_slice_eq1 {
 __impl_slice_eq1! { [] Vec<A>, Vec<B>, }
 __impl_slice_eq1! { [] Vec<A>, &[B], }
 __impl_slice_eq1! { [] Vec<A>, &mut [B], }
+__impl_slice_eq1! { [] &[A], Vec<B>, }
+__impl_slice_eq1! { [] &mut [A], Vec<B>, }
 __impl_slice_eq1! { [] Cow<'_, [A]>, &[B], A: Clone }
 __impl_slice_eq1! { [] Cow<'_, [A]>, &mut [B], A: Clone }
 __impl_slice_eq1! { [] Cow<'_, [A]>, Vec<B>, A: Clone }

@shepmaster
Copy link
Member

Hmm. You know, there were changes in Rust 1.41 that changed some code in this area. See RFC 2451 for more details. Perhaps it is possible now? If your code compiles, passes tests, and you can write the equality tests you wanted to, it's worth opening a PR to rust-lang/rust.

@sollyucko
Copy link
Author

Sounds good!

@mbrubeck
Copy link
Contributor

Would it also be possible to impl PartialEq<[B]> for Vec<A> and PartialEq<Vec<B> for [A]>?

@shepmaster
Copy link
Member

@mbrubeck when I tried similar for Option, it caused a massive amount of fallout due to type inference failures. For example, assert_eq!(foo(), None) is no longer unique and requires a turbofish. I assume something similar would happen here.

@mbrubeck
Copy link
Contributor

I can't see any reason that an impl for Vec<A> and [B] would cause inference problems while the existing impl for Vec<A> and &[B] does not.

@sollyucko
Copy link
Author

sollyucko commented May 25, 2020

Also, there should be working assert_eq/ne! tests for each possibility, so that shouldn't be an issue.

@mbrubeck
Copy link
Contributor

Would it also be possible to impl PartialEq<[B]> for Vec<A> and PartialEq<Vec<B> for [A]>?

Adding these impls to liballoc/vec.rs appears to work fine, without causing any regressions in the compiler code or tests.

Manishearth added a commit to Manishearth/rust that referenced this issue Jun 21, 2020
Manishearth added a commit to Manishearth/rust that referenced this issue Jun 21, 2020
Manishearth added a commit to Manishearth/rust that referenced this issue Jun 22, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jun 22, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jun 22, 2020
@jplatte
Copy link
Contributor

jplatte commented Jul 1, 2020

This can be closed now, right?

@dtolnay dtolnay closed this as completed Jul 1, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 7, 2020
Add PartialEq impls for Vec <-> slice

This is a follow-up to rust-lang#71660 and rust-lang/rfcs#2917 to add two more missing vec/slice PartialEq impls:

```
impl<A, B> PartialEq<[B]> for Vec<A> where A: PartialEq<B> { .. }
impl<A, B> PartialEq<Vec<B>> for [A] where A: PartialEq<B> { .. }
```

Since this is insta-stable, it should go through the `@rust-lang/libs` FCP process.  Note that I used version 1.47.0 for the `stable` attribute because I assume this will not merge before the 1.46.0 branch is cut next week.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants