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

VecDeque<T> should implement PartialEq with &[T] #38625

Closed
Mark-Simulacrum opened this issue Dec 26, 2016 · 8 comments
Closed

VecDeque<T> should implement PartialEq with &[T] #38625

Mark-Simulacrum opened this issue Dec 26, 2016 · 8 comments

Comments

@Mark-Simulacrum
Copy link
Member

I believe this is definitely possible (comparing the iterators of each), and I suspect that there's a more efficient method as well.

See this documentation PR where it simplifies the code: https://github.com/rust-lang/rust/pull/38581/files#diff-266e22074e3b36596081857cbf5b1513R1256.

@frewsxcv frewsxcv added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-libs and removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 27, 2016
@frewsxcv
Copy link
Member

I'm working on this.

@frewsxcv frewsxcv self-assigned this Dec 28, 2016
@frewsxcv
Copy link
Member

I ran into an issue.

assert!(tester.tail < tester.cap());
assert!(tester.head < tester.cap());

These now become errors:

error[E0282]: unable to infer enough type information about `_`
    --> src/libcollections/vec_deque.rs:2497:17
     |
2497 |             let expected = (0..).take(len).collect();
     |                 ^^^^^^^^ cannot infer type for `_`
     |
     = note: type annotations or generic parameter binding required

error: aborting due to previous error

@frewsxcv
Copy link
Member

Is this inference issue allowed breakage?

@frewsxcv frewsxcv removed their assignment Dec 28, 2016
@bluss
Copy link
Member

bluss commented Dec 28, 2016

It's definitely allowed, but not every allowed type inference breakage is a good idea. So it still has to be decided carefully.

@Mark-Simulacrum
Copy link
Member Author

@frewsxcv Mind posting a PR? Perhaps we can find a way to fix the inference breakage.

@frewsxcv
Copy link
Member

Opened a PR: #38661.

@bombless
Copy link
Contributor

I think that inference failure was there long before Rust 1.0
https://is.gd/j8KMA1

@frewsxcv
Copy link
Member

It occurs because prior to this PR, the only thing you could compare VecDeque with was another VecDeque, so the compiler knew to collect into a VecDeque. Now that it can compare VecDeque with VecDeque or Vec, it doesn't know which one to choose.

frewsxcv added a commit to frewsxcv/rust that referenced this issue Jan 28, 2017
bors added a commit that referenced this issue Jan 31, 2017
Implement `PartialEq<[A]>` for `VecDeque<A>`.

Fixes #38625.
bors added a commit that referenced this issue Feb 2, 2017
Implement `PartialEq<[A]>` for `VecDeque<A>`.

Fixes #38625.
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

4 participants