[libcxx-commits] [PATCH] D97115: [libc++] Make chars_format a bitmask type.

Tue Feb 23 00:17:48 PST 2021

curdeius added inline comments.

================
Comment at: libcxx/include/charconv:111

+using __chars_format_underlying_type = underlying_type<chars_format>::type;
+
----------------
Mordante wrote:
> Mordante wrote:
> > curdeius wrote:
> > > Would adding `to_underlying` helper be cleaner?
> > > Something along the lines of the proposed http://open-std.org/JTC1/SC22/WG21/docs/papers/2019/p1682r1.html. WDYT?
> > I hadn't seen that paper yet, but it seems very useful. According to to https://github.com/cplusplus/papers/issues/460 it seems to be on track to land in C++23. I'm somewhat tempted to implement `__to_underlying` in `utility` for C++11 for now and also adjust the other bitmask types. Then we can switch to `to_underlying` or add a C++17 `to_underlying` once the paper lands. @ldionne What's your opinion?
> This comment aged fast; according to Herb P1682 has been adopted [1]. I'll create a patch to implement it. I think it makes sense to allow its usage in C++11 or do we prefer C++23 only? In that case I'll add a private function for C++11 which is used by the C++23 public version.
> 
> [1] https://herbsutter.com/2021/02/22/trip-report-winter-2021-iso-c-standards-meeting-virtual/
@Mordante, I have already an almost ready patch for `to_underlying`. I'll finish it up this evening or tomorrow and send for a review, so don't bother for the tests of your private function.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97115