[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
----------------
Mordante wrote:
> philnik wrote:
> > jloser wrote:
> > > 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.
> > I want to avoid the mental burden of forming `AFTER_CXX20` to `CXX23`. I'd actually prefer it a lot if we renamed the other macros. It's also the same naming scheme that we use in the tests. I don't really see how `AFTER_CXXab` is a better name when we have C++26. IMO it's pretty obvious that `_LIBCPP_CONSTEXPR_CXX23` is "it's constexpr since C++23".
> Thanks for discussing this on Discord.
Thanks for the explanation/rationale and discussion on Discord. I'm +1 for this naming now.


================
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>());
----------------
philnik wrote:
> jloser wrote:
> > 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?
> `test_right_shift<1000>()` reaches the constexpr step limit. The other tests combined do the same thing.
Got it - thanks for clarifying.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131218