[libcxx-commits] [PATCH] D125283: reverts "[libc++] Explicitly reject `uniform_int_distribution<bool>` and `<char>`."

[Jordan R AW via Phabricator via libcxx-commits]

CrystalSplitter added a comment.

Hello.

I am on the ChromeOS C++ Toolchain team. I don't really interact much with the LLVM community personally, but nonetheless my work is quite affected by upstream LLVM changes. In this particular case, I'm the one who's cleaning up all the ChromeOS cases where this previous extension is used.

ChromeOS uses a lot of randomisation in tests. Notably, fuzzing frequently uses randomly generated bytes, and passing those around as bags of bytes to identify stability and security issues. It's not uncommon that `uniform_int_distribution<uint8_t>` or `uniform_int_distribution<char>` is used to generate these inputs.
Is this undefined? Yes! But it's also reliably worked up until now and is a valid extension. People are depending on this extension, regardless of whether they were aware it was an extension in the first place. They are all trivial to fix on a line-by-line difference, but it adds a burden when there are 30+ cases which all have different maintainers that one must collaborate with to get dependencies to compile. This list is still growing, because we haven't finished our investigation of how badly this breaks us.

This frustration is further fueled by the thought: Well, why isn't there support for it? There's no random algorithm which can generate a uniform random 16bit number but not a uniform random 8bit number. The implementation certainly isn't restricting us, and there's no added maintenance cost in doing so. I must then go and explain to maintainers that, while there's no technical reason this is a compiler //error//, we must change all the instantiations because this behaviour is not defined by the standard for ineffable reasons.

Some obvious public (still unfixed) examples in ChromeOS:

https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/aosp/system/update_engine/payload_generator/ab_generator_unittest.cc;l=68
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/aosp/system/update_engine/payload_generator/delta_diff_utils_unittest.cc;l=198
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform2/vm_tools/pstore_dump/persistent_ram_buffer_test.cc;l=30
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/aosp/frameworks/native/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp;l=454

This doesn't just affect ChromeOS notably. Here are some examples from Android:

https://cs.android.com/android/platform/superproject/+/master:external/ComputeLibrary/tests/validation/fixtures/ReductionOperationFixture.h;l=81
https://cs.android.com/android/platform/superproject/+/master:external/ComputeLibrary/tests/validation/fixtures/ConvolutionFixture.h;l=52
https://cs.android.com/android/platform/superproject/+/master:hardware/interfaces/biometrics/face/1.0/vts/functional/VtsHalBiometricsFaceV1_0TargetTest.cpp;l=196
https://cs.android.com/android/platform/superproject/+/master:external/pigweed/pw_tokenizer/generate_decoding_test_data.cc;l=220


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125283/new/

https://reviews.llvm.org/D125283