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

SliceExt::binary_search has a different signature than slice::binary_search #41561

Closed
brson opened this issue Apr 26, 2017 · 2 comments
Closed
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Apr 26, 2017

SliceExt parameterizes Self::Item over Borrow, but slices don't have that parameterization. This makes the core slice API different from the std slice API, which is not supposed to be the case.

PR where this change was made.

@brson brson added I-wrong T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Apr 26, 2017
@alexcrichton
Copy link
Member

This was attempted to get generalized in libstd in #41590, but unfortunately it caused too much breakage so the libs team has decided to not land that PR. An alternate route to fixing this though would be to de-generalize the signature in libcore. This is of course a breaking change but we may be lucky and have few users of this function in libcore, so we may be able to get away with it. The libs team would be interested in seeing a PR to de-generalize in libcore (make it the same as libstd's signature) and then have a crater run to evaluate the impact. From that point we can evaluate what to do next.

@alexcrichton
Copy link
Member

cc #32632

@Mark-Simulacrum Mark-Simulacrum added C-bug Category: This is a bug. and removed I-wrong labels Jul 27, 2017
bors added a commit that referenced this issue Aug 20, 2017
Remove Borrow bound from SliceExt::binary_search

#37761 added a Borrow bound to `binary_search` and `binary_search_by_key` in `core::SliceExt`, but did not add it to the methods in `std::slice`. #41590 attempted to add this bound to `std::slice` but was not merged due to breakage. This PR removes the bound in `core::SliceExt`, so that these methods will have the same signature in `core` and `std`.

Fixes #41561
alexcrichton added a commit to alexcrichton/rust that referenced this issue Sep 16, 2017
…ig, r=alexcrichton

Remove Borrow bound from SliceExt::binary_search

rust-lang#37761 added a Borrow bound to `binary_search` and `binary_search_by_key` in `core::SliceExt`, but did not add it to the methods in `std::slice`. rust-lang#41590 attempted to add this bound to `std::slice` but was not merged due to breakage. This PR removes the bound in `core::SliceExt`, so that these methods will have the same signature in `core` and `std`.

Fixes rust-lang#41561
bors added a commit that referenced this issue Sep 16, 2017
…richton

Remove Borrow bound from SliceExt::binary_search

#37761 added a Borrow bound to `binary_search` and `binary_search_by_key` in `core::SliceExt`, but did not add it to the methods in `std::slice`. #41590 attempted to add this bound to `std::slice` but was not merged due to breakage. This PR removes the bound in `core::SliceExt`, so that these methods will have the same signature in `core` and `std`.

Fixes #41561
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants