[all-commits] [llvm/llvm-project] fc027e: linear_congruential_engine: Fixes for __lce_alg_pi...

[LRFLEW via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: fc027e10ba3d343b583264776c8f4f8e82ad4560
      https://github.com/llvm/llvm-project/commit/fc027e10ba3d343b583264776c8f4f8e82ad4560
  Author: LRFLEW <LRFLEW at aol.com>
  Date:   2024-02-15 (Thu, 15 Feb 2024)

  Changed paths:
    M libcxx/include/__random/linear_congruential_engine.h
    M libcxx/test/std/numerics/rand/rand.eng/rand.eng.lcong/alg.pass.cpp
    M libcxx/test/std/numerics/rand/rand.eng/rand.eng.lcong/assign.pass.cpp
    M libcxx/test/std/numerics/rand/rand.eng/rand.eng.lcong/copy.pass.cpp
    M libcxx/test/std/numerics/rand/rand.eng/rand.eng.lcong/default.pass.cpp
    M libcxx/test/std/numerics/rand/rand.eng/rand.eng.lcong/values.pass.cpp

  Log Message:
  -----------
  linear_congruential_engine: Fixes for __lce_alg_picker (#81080)

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