On Thu, Sep 2, 2021 at 5:10 PM Go Kudo <zeriyo...@gmail.com> wrote: > Hi Internals. > > Expanded from the previous RFC and changed it to an RFC that organizes the > whole PHP random number generator. Also, the target version has been > changed to 8.2. > > https://wiki.php.net/rfc/rng_extension > https://github.com/php/php-src/pull/7453 > > Hopefully you will get some good responses. >
This looks pretty nice to me. A few bits of feedback: * Unlike the previous versions of the RFC, this one also moves all of the existing RNG-related functionality from ext/standard to ext/random. Why does it do this? This doesn't really seem related to the problem this RFC is solving. * Regarding the abstract class Random\NumberGenerator: You currently need the abstract class, because php_random_ng is tied to the object. An alternative design that would allow Random\NumberGenerator to be an interface would be to make it independent of the object, such that the structure can be allocated in the Random constructor for userland implementations. Of course, internal implementations could still embed it as part of their objects -- just don't make it a requirement. * The future scope mentions: "Changes random source to php_random_int() a shuffle(), str_shuffle(), and array_rand()". I don't think it makes sense to switch these functions to use cryptographic randomness by default... Regards, Nikita
