impl PartialEq<Vec<B>> for &[A], &mut [A]

[sollyucko]

rust-lang/rfcs#2917

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @withoutboats (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[shepmaster]

r? @kennytm

[marmeladema]

I am definitely not a compiler-team member and I don't want to overstep any boundaries here, so I apologize in advance if I do, just let me know. But should this kind of change requires some tests?

[shepmaster]

I would agree that some tests should be added, specifically those exercising the specific equality being added. Also, I’d have some assert_eq usages as this comes up as a common usage.

[kennytm]

@shepmaster ya know I can't start the required FCP for this insta-stable PR since I'm not in the libs team 🙃

[shepmaster]

ya know

I appreciate that you think I know anything. What I know is that you are a thorough reviewer (like one who realized that this would be insta-stable and thus needs a RFC).

[shepmaster]

r? @LukasKalbertodt

[shepmaster]

The "slice"s are not slices; that needs to be changed.

You don't have to follow this, but I would have written the tests to lean on the type checker more. Something like:

fn compare_vec_and_array(v: Vec<i32>, a: [i32; 3]) {
    let _ = v == a;
    let _ = a == v;

    assert_eq!(v, a);
    assert_eq!(a, v);
}

fn compare_vec_and_slice(v: Vec<i32>, s: &[i32]) { /* ... */ }

// Optionally make these take in `Vec<T>`?

I don't know if I would even call these functions; their existence proves that the traits exist. I might call them just to prove that they were compiled.

I also advocate for having smaller tests with better names. That way, when a test fails, you can tell what failed by the name, and it's easier to see what is being tested.

[sollyucko]

I'm still keeping the mega-test for thoroughness, but I can remove it if you'd prefer.

[dtolnay]

I reverted 73194c527519565a686e8e79d9f81ed8374a1d07 because those impls were not included in the FCP proposal. There can be a separate FCP in a different PR.

[dtolnay]

@bors r+

[bors]

📌 Commit 4896a06 has been approved by dtolnay

[bors]

⌛ Testing commit 4896a06 with merge 3de9e530d523ca4a2386e981b9e1cd89a6b9fa34...

[Dylan-DPC-zz]

included in rollup (accidentally two of them :P )

@bors retry