-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Remove Borrow bound from SliceExt::binary_search #43989
Conversation
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
@bors try -- gathering artifacts for cargobomb |
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
☀️ Test successful - status-travis |
review ping @aturon! pinging you on IRC as well! |
I think this is waiting for a cargobomb? ping @aidanhs |
Umm this seems in limbo. I think @Mark-Simulacrum is on leave? And he wants a crater run? |
I'm back now, though still catching up with backlog. I try-ed this so that we can get statistics from cargobomb on how many crates this breaks. In #41590 we tried to generalize it in std, but that was found too problematic (inference breakage, I believe) so this attempts to de-generalize core. We need a cargobomb run to determine whether that is possible. |
Typically I wait for a review and a ping from a member of the relevant team to rust-lang/infra with an explicit request so we don't cargobomb needlessly. That said, we have some free capacity right now so I'll give it a go. Could you rebase in the meantime @circuitfox? |
ac15987
to
e15a07a
Compare
Cargobomb results: http://cargobomb-reports.s3.amazonaws.com/pr-43989/index.html Blacklist (spurious failures etc) at https://github.com/rust-lang-nursery/cargobomb/blob/master/blacklist.md. If you see any spurious failures not on the list, please make a PR against that file. |
ping @aturon there are cargobomb results for you, so I think this is ready for review! |
Looks like all the failure there are spurious, so... @bors: r+ Thanks @circuitfox! |
📌 Commit e15a07a has been approved by |
…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
…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
☀️ Test successful - status-appveyor, status-travis |
#37761 added a Borrow bound to
binary_search
andbinary_search_by_key
incore::SliceExt
, but did not add it to the methods instd::slice
. #41590 attempted to add this bound tostd::slice
but was not merged due to breakage. This PR removes the bound incore::SliceExt
, so that these methods will have the same signature incore
andstd
.Fixes #41561