Fix Issue 20550 - Use fixed seeds for treaps in GC #2919
Conversation
n8sh
requested review from
andralex,
DmitryOlshansky,
MartinNowak,
rainers,
WalterBright and
wilzbach
as code owners
|
Thanks for your pull request, @n8sh! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + druntime#2919" |
thewilsonator
added
the
72h no objection -> merge
| { | ||
| rand48.defaultSeed(); | ||
| Rand48 _rand48 = { randSeed }; | ||
| rand48 = _rand48; |
There was a problem hiding this comment.
Doesn't rand48 = Rand48(randSeed); work here?
There was a problem hiding this comment.
no it doesn't because of the opCall override
There was a problem hiding this comment.
ok (though it seems a hole in @safe if you are allowed to initialize any private members of a struct like this.) I'm not sure why the compiler tries to invoke a non-static opCall. Adding this(ulong) to Rand48 allows to use the constructor syntax.
| @@ -27,9 +27,10 @@ nothrow: | |||
| removeAll(); | |||
| } | |||
|
|
|||
| void initialize() | |||
| void initialize(ulong randSeed) | |||
There was a problem hiding this comment.
Would it be a problem to keep the existing initialize() method, but also allow passing the seed explicitly in an overload? I'm not expecting Treap to be used often, but having to come up with a magic number can be annoying.
There was a problem hiding this comment.
Maybe define rand() as the default parameter value?
thewilsonator
added
auto-merge
and removed
72h no objection -> merge
Discussion started from ldc-developers#168.