Copy impl for random number generators


#1

A few weeks ago I suggested removing Copy trait derivation from random number generators that currently implement it.

This is my motivation from now-closed PR:

Copying is an implicit operation and the copy shares its internal state with the original generator. This means the the two generators will yield exactly the same sequence of values. This behaviour is considerably easy to trigger by, for example, passing the generator by value to a function. Clone trait, on the other hand, does no implicit copying. The user will only be able to either move the generator or ask for a copy explicitly, via the clone method. The only downside to this patch is the fact that the implementations of Clone::clone methods do not optimise down to a single memcpy, like the derived Copy implementations did.

The PR received some conflicting feedback from some core team members. Summary:

  • Copy is used as a bound on some functions/structs and has useful properties;
  • Copy as a bound is only used in a few APIs in Rust standard library;
  • Copy implementation can always be added later, but not removed.

Clone implementation has already been merged for most RNGs (#20483), and the fate of Copy needs to be decided. Should the Copy implementation for ChaChaRng, Isaac64Rng, IsaacRng and StdRng be removed?

cc @aturon as per his request.


#2

Removing Copy makes sense to me for the reasons you outline. Usually, one doesn’t want more than one random number generator creating the same sequence of numbers, and doing so unintentionally could enable certain attacks. It makes sense to have to be explicit about wanting it. Also, as noted, not having it is the conservative choice, and it can be added back later, if for some reason it were necessary, but it can not be later removed.

As for optimizing to a memcpy, it doesn’t seem like RNGs would be cloned enough for this to matter, but if necessary, wouldn’t it be possible to implement clone using an explicit unsafe memcpy?


#3

+1, leaving copy would have the risk of some unintended security bugs over witch you might skim often without noticing them. But I wonder if it would remove some other useful traits implicitly implemented for all Copy types.


#4

I believe the real solution to this would be to invent a new trait ImplicitCopy that can implicitely copy the type, while Copy needs an explicit annotation.


#5

Isn’t Copy and Clone just that?


#6

Sometimes you want to copy RNG, or even replay them, for instance when doing replays, or even as a sophisticated crash reproduction method if you checkpoint regularly and store inputs.


#7

No. Clone is allowed to do stuff other than just bit-copying.


#8

+1 for removal of Copy. RNGs state objects are both stateful (by definition!) and large in size (IsaacRng, for example, is 80 bytes long.) Implicit copying doesn’t fit with them.