mirror of
https://github.com/intel/llvm.git
synced 2026-02-01 08:56:15 +08:00
fc027e10ba
This fixes two major mistakes in the implementation of `linear_congruential_engine` that allowed it to produce incorrect output. Specifically, these mistakes are in `__lce_alg_picker`, which is used to determine whether Schrage's algorithm is valid and needed. The first mistake is in the definition of `_OverflowOK`. The code comment and the description of [D65041](https://reviews.llvm.org/D65041) both indicate that it's supposed to be true iff `m` is a power of two. However, the definition used does not work out to that, and instead is true whenever `m` is even. This could result in `linear_congruential_engine` using an invalid implementation, as it would incorrectly assume that any integer overflow can't change the result. I changed the implementation to one that accurately checks if `m` is a power of two. Technically, this implementation has an edge case where it considers `0` to be a power of two, but in this case this is actually accurate behavior, as `m = 0` indicates a modulus of 2^w where w is the size of `result_type` in bits, which *is* a power of two. The second mistake is in the static assert. The original static assert erroneously included an unnecessary `a != 0 || m != 0`. Combined with the `|| !_MightOverflow`, this actually resulted in the static assert being impossible to fail. Applying De Morgan's law and expanding `_MightOverflow` gives that the only way this static assert can be triggered is if `a == 0 && m == 0 && a != 0 && m != 0 && ...`, which clearly cannot be true. I simply removed the explicit checks against `a` and `m`, as the intended checks are already included in `_MightOverflow` and `_SchrageOK`, and their inclusion doesn't provide any obvious semantic benefit. This should fix all the current instances where `linear_congruential_engine` uses an invalid implementation. This technically isn't a complete implementation, though, since the static assert will cause some instantiations of `linear_congruential_engine` not disallowed by the standard from compiling. However, this should still be an improvement, as all compiling instantiations of `linear_congruential_engine` should use a valid implementation. Fixing the cases where the static assert triggers will require adding additional implementations, some of which will be fairly non-trivial, so I'd rather leave those for another PR so they don't hold up these more important fixes. Fixes #33554