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

Functions in std should accept &[T; N] whenever they accept &[T] #21725

Closed
petrochenkov opened this issue Jan 28, 2015 · 4 comments
Closed

Functions in std should accept &[T; N] whenever they accept &[T] #21725

petrochenkov opened this issue Jan 28, 2015 · 4 comments
Labels
A-DSTs Area: Dynamically-sized types (DSTs) A-typesystem Area: The type system

Comments

@petrochenkov
Copy link
Contributor

Most of the time it happens automatically due to DST coercions &[T; N] => &[T], but sometimes additional care is required.

One example, where it currently doesn't work, is comparison operators between slices/arrays/vectors:

fn main() {
    let a: &[u8; 3] = &[1, 2, 3];
    let b: &[u8] = &[1, 2, 3];
    // Both comparisons doesn't compile
    a == b;
    b == a;
}

When #18465 is implemented, the code comparing byte string literals with slices will break. A lot of such code can be found, for example, in libstd/path.

@steveklabnik steveklabnik added the A-typesystem Area: The type system label Jan 29, 2015
@huonw
Copy link
Member

huonw commented Jan 29, 2015

cc @nick29581, maybe something to with coercions? It seems at least b == a should compile by coercing the RHS to &[u8]?

(I tried to implement #18465 and met this exact problem)

@nrc nrc added the A-DSTs Area: Dynamically-sized types (DSTs) label Jan 29, 2015
@nrc
Copy link
Member

nrc commented Jan 29, 2015

I'll have to have a look at the code in the morning to be sure of what's going on. I think the problem though is that we do not coerce when searching for impls, the == means we have to find an implementation of Eq (or is it PartialEq?) with Self = &[u8] and a type param bound to &[u8; 3], which I assume will not work. If that is the case, I'm not sure what we can do about it - I think doing coercions when doing trait matching is impractical, though I don't exactly recall why. I guess it could work if the RHS of PartialEq were an associated type, but I assume there are reasons we don't do that.

OTOH, I could be wrong and we should coerce and this is a bug...

@petrochenkov
Copy link
Contributor Author

If there's no bug in coercions and impl lookup, then I'm ready to write additional impls for the comparison traits to cover references to fixed-size arrays.

bors added a commit that referenced this issue Feb 23, 2015
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
bors added a commit that referenced this issue Mar 17, 2015
This patch changes the type of byte string literals from `&[u8]` to `&[u8; N]`.
It also implements some necessary traits (`IntoBytes`, `Seek`, `Read`, `BufRead`) for fixed-size arrays (also related to #21725) and adds test for #17233, which seems to be resolved.

Fixes #18465
[breaking-change]
bors added a commit that referenced this issue Mar 18, 2015
This patch changes the type of byte string literals from `&[u8]` to `&[u8; N]`.
It also implements some necessary traits (`IntoBytes`, `Seek`, `Read`, `BufRead`) for fixed-size arrays (also related to #21725) and adds test for #17233, which seems to be resolved.

Fixes #18465
[breaking-change]
@petrochenkov
Copy link
Contributor Author

I believe this is fixed - functions in std take either &[T] (and DST coercions apply), or AsRef<[T]> (and arrays implement AsRef), or Borrow<[T]> (and arrays implement Borrow). Tricky cases like PartialEq are also covered.

"Bound-targeting coercions" as a languge feature should have a separate issue in the RFCs repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-DSTs Area: Dynamically-sized types (DSTs) A-typesystem Area: The type system
Projects
None yet
Development

No branches or pull requests

4 participants