[PATCH] Random Number Generator Refactoring (removing from Module)

[Stephen Crane]

On Thu, Jul 3, 2014 at 11:24 AM, Alp Toker <alp at nuanti.com> wrote:
>> Index: include/llvm/Support/RandomNumberGenerator.h
>> ===================================================================
>> --- include/llvm/Support/RandomNumberGenerator.h
>> +++ include/llvm/Support/RandomNumberGenerator.h
>> @@ -7,16 +7,17 @@
>>   //
>>
>> //===----------------------------------------------------------------------===//
>>   //
>> -// This file defines an abstraction for random number generation (RNG).
>> -// Note that the current implementation is not cryptographically secure
>> -// as it uses the C++11 <random> facilities.
>> +// This file defines an abstraction for deterministic random number
>> +// generation (RNG).  Note that the current implementation is not
>> +// cryptographically secure as it uses the C++11 <random> facilities.
>>   //
>>
>> //===----------------------------------------------------------------------===//
>>     #ifndef LLVM_SUPPORT_RANDOMNUMBERGENERATOR_H_
>>   #define LLVM_SUPPORT_RANDOMNUMBERGENERATOR_H_
>>     #include "llvm/ADT/StringRef.h"
>> +#include "llvm/IR/Module.h"
>
>
> Layering violation here, llvm/Support is strictly layered beneath llvm/IR
> etc. (In general, an RNG should be used by higher layers, not the other way
> round. Doing so will lead to a clearer design, and I think there's some
> clarity missing at the moment.)
>
> Alp.

Alright, fair enough, thanks. I was trying to make it easy for clients
to instantiate an RNG. We could have an RNG constructor in Module
which returns an RNG instance instead of storing it. Would this be ok?
I was trying to get this entirely out of Module, but that may not
actually be a good idea.

Also, at this point we're more or less just wrapping the C++11 RNG in
an interface, but I think this might still be useful to cleanly swap
out the RNG if desired.

- stephen