Implement binary_search_limit_* on slice

[sicking]

This implements the proposal in #52529.

I tried to make it clear in the documentation exactly how to use these functions in a way that hopefully will make it easier to implement the use cases described in #52529.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[sicking]

See also #52529 (comment) for possible improvement on the API to allow both inclusive and exclusive limits. And also possibly improving performance by not calling into binary_search_by.

[sicking]

I did some perf tests and I wasn't able to do anything measurably faster than what's here already.

[sicking]

I added a second commit to implement using Bound<T> as argument for binary_search_limit and binary_search_limit_by_key. I kept it as a separate commit so that we can choose whether to use it or not.

[alexcrichton]

Thanks for the PR! It looks like this has gone through quite a number of iterations though, so are we sure this is ready for the standard library? This seems like functionality that may wish to bake on crates.io first before being added to libstd? (even if unstable)

[sicking]

API-wise the only revision has been regarding if we should use Bound<T> as limit argument or not. That's what was discussed in #52529 (comment), and what I added a second commit for.

I also looked into a few implementation strategies to see if we could get better performance, but turns out the simple implementation was also the fastest one (which I think is a testament to the current implementation of binary_search_by).

However rfc 2184 does suggest a pretty different approach which can be used to solve the same use cases. I personally feel that the names and signature there make that API is more suited for searching for items with a particular value (I.e. find the set of entries that contain the value "5"), and so to me doesn't seem much better suited for the use case that I was hoping to address (I.e. find the first entry greater than "5", or less than "5", or greater than or equal to "5", etc) than binary_search_by is.

That said, I'm not particularly familiar with the process for making additions to stdlib. If there's a preference for creating an RFC or a standalone crate, I'll do that instead.

[alexcrichton]

Hm ok, but my point is that there's not only been revisions to this PR and while it was discussed on the issue but there's also discussion on the rfc issue tracker of alternate APIs. In that sense I don't think we're ready to add this to libstd.

We don't require an RFC for all API additions to the standard library but additions need to be broadly in the space of "we for sure want to at least have functionality like this stable one day". This is somewhat questionable in that it's such a simple definition (or so it looks) so I'm not sure we'd want it included per se.

In the meantime though you can make an extension crate on crates.io for sure!

[sicking]

Ok, so if I understand what you're saying, we don't want to merge this both because the discussion in the RFC indicates that there's not agreement on the shape of the API, and because the API is simple enough that it's not obvious that it's needed?

I'll look into creating a crate, but honestly I don't see anyone bothering to look for a crate for something as small as this.

What is proper protocol for this PR? Should I close it?