Improve std.sort.binarySearch
's return type, fix std.sort
functions to adhere to their documentation
#22300
+304
−243
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes a couple of changes in
std.sort
, for the binary search functions family:std.sort.binarySearch
's return type has been changed from?usize
tostruct { usize, bool }
- that is because there is still meaningful information to return to the caller, even if no acceptable item was found, namely: the index where an acceptable item could be inserted, while preserving the input slice's order (such index is unique).The other changes made are to correct the implementations of the search functions that accept a trisection callback; those were out of sync with their documentation ever since #21373 was implemented:
Trisection callbacks are expected to classify the slice's values as either too large (
.gt
), too small (.lt
), or within target (.eq
). This formulation is independent of any concrete key that may or may not exist and be compared against, and so is the most reasonable formulation in the generic case. This has been exemplified in the documentation since #20927:However, #20927's formulation turned out to be quite confusing for the most common, specific case, where in fact there is a concrete key to compare the items to, passed via the
context
argument that the trisection callbacks accept. Forwarding the arguments tostd.math.order
to get an ascending order will require them to be swapped, making it appear like the values' relation is backwards.#21373 introduced changes to the functions' implementations, like adding
.invert()
calls tolowerBound
andupperBound
's generated predicates - these superficially fixed the issues, and indeed resolved the confusion around concrete-key use cases; however, the changes also broke the generic case, making it expect inverted inputs. The proper solution is to maintain the internal logic of the functions, and instead swap theT
andcontext
arguments in the trisection callbacks (This was not originally done in #20927 as the precedent for callbacks is to takecontext
as the first argument. Retrospectively, the confusion this led to vastly outweighs the consistency benefit).The changes in this PR mean that:
context
as a key (the changes to such calls in this PR only rename arguments).std.math.Order
values directly, instead of forwarding tostd.math.order
) will have to switch.lt
s and.gt
s around, again making their behaviour consistent with the functions' documentation and intent.The PR also tweaks the doc comments somewhat, explaining the functions' main goals and preconditions in the head sentence.
compareFn
is renamed totrisectFn
for cases where a comparison, implying an order, is not the most natural way to think of the sections (e.g. three sections for red, green, and blue values). The type annotations oftrisectFn
get aprobed:
argument name, to instil that the first argument is now the one taken from the slice, in case the types themselves weren't an obvious indication.