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

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added a comment.

In D125283#3535047 <https://reviews.llvm.org/D125283#3535047>, @ldionne wrote:

> In D125283#3532818 <https://reviews.llvm.org/D125283#3532818>, @cjdb wrote:
>
>> I'm not sure why there's so much resistance to reverting. libc++:
>>
>> 1. has users with broken code right now;
>> 2. actively supports `uniform_int_distribution<__int128_t>`, **which is also undefined** (I don't get why you consider `signed char`, `unsigned char`, `char`, and `bool` to be problematic, but not `__int128_t` and `unsigned __int128_t`);
>> 3. has licence to make it an extension without Microsoft/STL's support;
>>
>> You can also propose standardising this right now.
>
> I don't know whether our distributions actually work with these types. This explicit rejection was added in D114920 <https://reviews.llvm.org/D114920>, which was itself prompted by discussions on D114129 <https://reviews.llvm.org/D114129>, whose goal was to fix http://llvm.org/PR51520 (an overflow in `uniform_int_distribution<__int128_t>`). What tells us that we don't have similar problems with `uniform_int_distribution<char>` & friends. They are not tested, so anything is possible. Then, add the fact that the Standard made those undefined, which I consider a red flag; without diving into the implementation of those distributions, I'm thinking there might be a good reason why they didn't require implementations to support those types, e.g. maybe the usual algorithms don't work properly for types of that size.
>
> So, given this uncertainty, I think it's better for everyone (our users especially) to get this compile-time error. However, if this patch showed that our implementation indeed supports these types properly, it would become a simple question of "do we want to support them as an extension, like we do for `__int128_t`", and that's easier to answer.
>
> If you can add tests to show that distributions work with these types, I'd be OK with that. We still have until June 8th to cherry-pick to LLVM 14, so we could even backport to LLVM 14 and avoid breaking users for just one release (which would be very annoying indeed).

If there's a technical reason why single byte integers can't be used in the algorithms, then it's definitely possible to make specialisations that do two-byte distributions and then contract to a single-byte type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125283