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

Christopher Di Bella via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed May 11 11:13:58 PDT 2022


cjdb added a subscriber: CrystalSplitter.
cjdb added a comment.

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

> http://eel.is/c++draft/rand.req.genl#1 says that the effect of instantiating one of these templates with a type that isn't in <list-of-allowed-types> is undefined. That means it's undefined behavior, and we strive to catch cases where users rely on undefined behavior for various reasons, notably to make their code more portable (https://godbolt.org/z/E6oGjfjhP), but also because we can't guarantee what the behavior is for such inputs.

Yes, it's undefined, which means that the standard places no requirements on an implementation. That means that implementations are allowed to treat this as an extension point. libc++ can absolutely guarantee the behaviour for such inputs, because it is the implementation. Choosing to not support it is certainly a valid option, but choosing to not support it after years of having done so is incredibly user-hostile.

As for portability:

- libstdc++ accepts this code. Have you considered trying to get Microsoft/STL to adopt supporting all integer types instead of limiting the usability in libc++? That would be far more productive and would not be user-adverse.
- libc++ has plenty of extension points, and if I recall correctly, some of them aren't even opt-outable (such as choosing to continue permitting `uniform_int_distribution<__int128>`).

>> broken a fair amount of user code.
>
> When I tried this change internally, I did see a few breakages, but all of them were really easy to fix and made the code better. I'm curious to hear about your experience.
>
> If you'd like to pursue this change, what I'd support is adding a escape hatch such that `__libcpp_random_is_valid_inttype<T>::value` is always `true`, and removing that after LLVM 15 has been shipped. This won't really change the fact that user's code needs to be fixed, but it will give them a bit more time to procrastinate on doing it. If that helps, I'm OK with that approach.

@CrystalSplitter can provide you with data points on how ChromeOS has been affected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125283



More information about the libcxx-commits mailing list