[libcxx-commits] [PATCH] D131218: [libc++] Implement P2417R2 (A more constexpr bitset)

[Joe Loser via Phabricator via libcxx-commits]

jloser added inline comments.


================
Comment at: libcxx/include/__config:853
+#  if _LIBCPP_STD_VER > 20
+#    define _LIBCPP_CONSTEXPR_CXX23 constexpr
+#  else
----------------
I'd prefer `_LIBCPP_CONSTEXPR_AFTER_CXX20` for consistency and presumably `std::bitset` will be `constexpr` still when C++26 is around, so the macro name won't be great at that time.


================
Comment at: libcxx/test/std/utilities/template.bitset/bitset.members/right_shift_eq.pass.cpp:53
+#if TEST_STD_VER >= 23
+  static_assert(test_right_shift<0>());
+  static_assert(test_right_shift<1>());
----------------
What about `test_right_shift<1000>()` in `constexpr`?

It seems a simpler refactoring is to put the calls into a reusable function, say `test` and then do

```
test()
static_assert(test())
```

What do you think?


================
Comment at: libcxx/test/std/utilities/template.bitset/bitset.members/to_string.pass.cpp:130
+  static_assert(test_to_string<64>());
+  static_assert(test_to_string<65>());
+#endif
----------------
Similar comment about a reusable test function and why can't we test `test_to_string<1000>()` in `constexpr` branch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131218