[libcxx-commits] [PATCH] D102328: [libc++] Fix constraint for [bit.pow.two] functions

[Jonathan Wakely via Phabricator via libcxx-commits]

jwakely added inline comments.


================
Comment at: libcxx/include/bit:89
         _IsNotSame<typename remove_cv<_Tp>::type, char16_t>::value &&
         _IsNotSame<typename remove_cv<_Tp>::type, char32_t>::value
     >;
----------------
jwakely wrote:
> Quuxplusone wrote:
> > jwakely wrote:
> > > Quuxplusone wrote:
> > > > Seems like we should add `char8_t` while we're here, yeah? (It is also integral and unsigned but not an unsigned integer type.)  For bonus clarity, we could make a type trait `__is_integer_type<_Tp>` and then this whole constraint on line 97 would be simply
> > > > ```
> > > > static_assert(__is_integer_type<_Tp>::value && is_unsigned<_Tp>::value, "__rotl requires an unsigned integer type");
> > > > ```
> > > > ...except that a static_assert is not a "constraint," so we're just totally nonconforming here.
> > > > @jwakely, thoughts? I'd volunteer to commandeer if you like.
> > > Yes, that should be there too. Feel free to take this, I only submitted it here because it seemed better to put the patch here than in bugzilla.
> > Blech.
> > @mclow.lists, @ldionne: libc++ currently supports `__uint128_t` for these functions; do we want to keep supporting it?
> > libstdc++ does not support it: https://godbolt.org/z/jK9q8z6oT
> > 
> > I'm going to operate on the assumption that we want to keep it (as a non-conforming extension)...
> If you want non-conforming extensions then you need to compile with `-std=gnu++20` not `-std=c++20` and if you do that, libstdc++ does support `unsigned __int128` here.
The static assert is not a problem, because you're only looking at the internal `__rotl` function which doesn't need to be constrained. The public `rotl` function is constrained correctly, using `enable_if`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102328