Tweaks to equality comparisons for slices/arrays/vectors

[petrochenkov]

All types that are "slices in disguise" - [T], [T; N], Vec<T> etc and sometimes references to them (see #21725) should be equality comparable to each other.
Many of them are already comparable, but some are not and this PR patches the holes in the impls of PartialEq for slice-like entities.

Ideally, the problem could be solved by an impl roughly looking like

impl<T, U, A, B> PartialEq<U> for T where T: BorrowSlice<A>,
                  U: BorrowSlice<B>,
                  A: PartialEq<B> {
    fn eq(&self, other: &U) -> bool { self.borrow_slice() == other.borrow_slice() }
    fn ne(&self, other: &U) -> bool { self.borrow_slice() != other.borrow_slice() }
}

, but the type system would never allow such a simple and obvious solution.
Therefore, the patch follows this approach, but implements it with macros and not generics.

The applied changes are conservative, in particular, the smart pointers aren't touched at all.
But the comparisons for Box<[T]>, Rc<[T]> and Arc<[T]> seem trivial to add. (Should I do it?)

This PR partially address #21725

Technically this is a [breaking-change], because the impls for fixed-size arrays are more conservative now.

Note: There are blanket impls of PartialEq in libcore/cmp.rs (see https://github.com/rust-lang/rust/blob/master/src/libcore/cmp.rs#L432 and below), they strip an equal number of references from both operands and then compare the remains. It would be much better to strip all the references before the comparison in the blanket impls to reduce the number of concrete impls, but I haven't found a way to do it without specialization/negative bounds.

r? @aturon
cc @gankro

[alexcrichton]

I'm a little worried about the impact this has on the metadata, this seems like a huge number of implementations being thrown in, not all of which are really that necessary. Can you make sure we're not blowing up the metadata by a few megabytes?

[petrochenkov]

Just by comparing the sizes of produced rlibs before and after or is there a smarter way to compare rlibs?

[petrochenkov]

@alexcrichton
Sadly, the bloat is significant.

libcore: 36MB -> 39MB
libcollections: 11MB -> 18MB (15MB without CowVec impls)
Besides, if I purge libcore/array.rs completely, then libcore.rlib loses another 6MB and becomes 30MB.

That's one more reason, why we need something like this sooner.

I think the reasonable subsequent actions would be:

1. Keep the macros
2. Keep the "status quo" impls
3. Add the impls crucial for Byte string literals should have a type of a fixed size #18465
   Then the bloat should be tolerable

[alexcrichton]

Hm yeah this is quite unfortunate how big a blowup this is (11MB => 18MB is huge!). I suspect there is a huge trove of optimizations that we could exploit for metadata layout, but unfortunately it will take time to examine.

I think that your proposed sequence of actions should be fine.

[alexcrichton]

(thanks for analyzing by the way!)

Also, for the future, I analyze metadata by running ar x libfoo.rlib and then looking at rust.metadata.bin to see how big it is.

[alexcrichton]

I feel like a bit of explicitness when using a CowVec is somewhat OK. I would hate for the "norm" to be adding dozens of impls (via macros or not) for small utility types like Cow.

[petrochenkov]

I feel like a bit of explicitness when using a CowVec is somewhat OK. I would hate for the "norm" to be adding dozens of impls (via macros or not) for small utility types like Cow.

The status quo is "CowVec is comparable to slices &[T]/&mut [T]", I can fall back to it.

[aturon]

cc @japaric

[japaric]

@petrochenkov Have you tried using a BorrowSlice trait with zero input parameters?

As a first attempt this compiles:

#![crate_type = "lib"]

trait AsSlice {
    type Elem;

    fn as_slice(&self) -> &[Self::Elem];
}

trait PartialEq<T: ?Sized = Self> {
    fn eq(&self, &T) -> bool;
}

impl<T: AsSlice, U: AsSlice> PartialEq<T> for U where
    T::Elem: PartialEq<U::Elem>,
{
    fn eq(&self, _: &T) -> bool {
        true
    }
}

But I don't know if that may have more problem down the line because of conflicts with other PartialEq implementations.

[petrochenkov]

@japaric
I haven't tried this particular approach, but it seems like every impl of PartialEq with generic LHS and RHS, or reference LHS and generic RHS (and vice versa) will conflict with blanket impls of PartialEq for references https://github.com/rust-lang/rust/blob/master/src/libcore/cmp.rs#L432

[japaric]

@petrochenkov yeah, seems like because of those reference impls this approach will require negative bounds at some point.

Hmm, I wonder how bad it would be remove those impl for &T/&mut T. IIRC, those are mainly to allow &mut [] == &[], &[] == &[], etc. But those would be already covered by the blanket T: AsSlice implementation. I guess you wouldn't be able to do &5 == &5 though.

Yet another idea to reduce the impl bloat, is to make the == more magical and make it auto-deref/reborrow both sides until there's only one reference. As in given a: &mut [_], b: &Vec<_>, then a == b would desugar to PartialEq::eq(&*a, b). That way you would only have to focus on the base cases [T], str, etc and not bother with the &'a T implementations, and while we are at it, we could (maybe) make it apply the "unsize" step to go from [T; N] to [T]. (cc @nikomatsakis is this idea feasible?)

[nikomatsakis]

@japaric I don't know how feasible that is. That sort of magic is definitely tricky to get right and tends to produce edge cases of its own. It might work out.

[petrochenkov]

@japaric
I've made an experiment and replaced the problematic blanket impls for references with specific impls for &[T]/&mut [T]. It breaks a lot of code and sometimes there's no trivial way to fix it.
As an example look at the range comparisons in libcore/iter.rs (https://github.com/rust-lang/rust/blob/master/src/libcore/iter.rs#L2863). Those functions should be able to compare all kinds of iterators originating from iter(), iter_mut() or into_iter() but they can't be specialized to work differrently with reference iterators ("dereference and compare") and non-reference iterators ("just compare"), so our favorite reference impls do all the job for them and range comparison functions only do "just compare".

[petrochenkov]

@japaric

Yet another idea to reduce the impl bloat, is to make the == more magical and make it auto-deref/reborrow both sides until there's only one reference.

I don't like this idea for two reasons:

1. It's magic
2. It will not work in generic context, like in my previous example with range comparisons in libcore/iter.rs. For it to work references still have to implement PartialEq themselves.

[petrochenkov]

Rebased and updated with reduced code bloat.
libcore: 36.6MB
libcollections: 13.6MB

Some tests use fixed-size arrays in weird ways, if they are adjusted, then 2 x 2 x 32 more impls of PartialEq can be removed.

r? @alexcrichton

[alexcrichton]

@bors: r+ 5e616db

[bors]

⌛ Testing commit 5e616db with merge de81c9a...

[bors]

💔 Test failed - auto-linux-64-x-android-t

[alexcrichton]

@bors: retry