add must_use to partial_shuffle

[TheIronBorn]

• Added a CHANGELOG.md entry

Summary

partial_shuffle's returned slices are the only correct way to access the regions. Not using them is almost certainly an error.

Motivation

I noticed that some users don't use the returned subslices of partial_shuffle.

Instead they typically assume the shuffled region is at the start of the original slice and are incorrect. Or assume correctly and may silently become incorrect if the implementation changes.

Here's a Github search of examples.
About 13 out of 60 uses ignore the returned slices.
Mostly due to the borrow checker or cloning/Vec related inflexibility.

Many users are accessing the unshuffled region instead of the shuffled. When slice.len() is much larger than amount, this means practically no shuffling occurs in the data they actually use.

Alternatives

Of course we could instead just guarantee an order but that might prevent future optimizations and would break some users' code anyway.

[dhardy]

If you follow the lifetime elison rules for functions, namely:

If the receiver has type &Self or &mut Self, then the lifetime of that reference to Self is assigned to all elided output lifetime parameters.

then it is clear that the returned slices must be sub-slices of the input self. It is also specified that the first contains amount elements and the second all remaining elements. Thus, by my reading there are only two possible outputs: (&mut self[..amount], &mut self[amount..]) or (&mut self[len-amount..], &mut self[..len-amount]).

Only one implementation is provided and it does the latter (potentially confusing since the ranges are swapped in the output; it would perhaps have been more intuitive if the two output slices were in order but obviously that's not something we should change now).

Since this is an unsealed trait, foreign implementations on foreign types are possible (I don't see any in a GitHub search).

Instead they typically assume the shuffled region is at the start of the original slice and are incorrect.

From the first two pages of GitHub search results, I count:

• 8 examples incorrectly assuming the shuffled amount elements are at the start
• 2 examples correctly assuming the shuffled amount elements are at the end
• 1 example calling partial_shuffle which then appears to use the entire input slice(?)
• One example comparing the order of the output slices as pointers
• One other examples observed using the second output slice (of remaining elements), excluding test code

One of the samples linked uses:

vec.split_off(vec.len() - num_woken)

This is a neat way to gain an owned Vec of num_woken shuffled elements. Not quite as neat as the example that uses peers.truncate(20); alas the latter is wrong.

Of course we could instead just guarantee an order but that might prevent future optimizations and would break some users' code anyway.

I don't see any possible scope for optimizations, aside from for use-cases which want the amount elements to be at the beginning of the array instead of the end.

Suggestion

We document the expected output order within the input slice clearly (since many examples assumed this incorrectly) and add #[must_use] (which won't always help since many examples already use something like let (_results, _rest) = thing.partial_shuffle(...)).

Further, in the next breaking release of rand, we change this method to only output one slice (the selected elts of length amount). Nearly all users don't care about the remaining elements and those that do can find them from the input slice using the extended specification.

[vks]

I think this is superseded by #1769.