[libcxx-commits] [PATCH] D131317: [libc++] Implements constexpr <charconv>.

Mon Aug 8 11:38:37 PDT 2022

Mordante planned changes to this revision.
Mordante marked 3 inline comments as done.
Mordante added inline comments.

================
Comment at: libcxx/include/__config:852-856
+#  if _LIBCPP_STD_VER > 20
+#    define _LIBCPP_CONSTEXPR_AFTER_CXX20 constexpr
+#  else
+#    define _LIBCPP_CONSTEXPR_AFTER_CXX20
+#  endif
----------------
philnik wrote:
> Mordante wrote:
> > philnik wrote:
> > > I've named this `_LIBCPP_CONSTEXPR_CXX23` in D131218 because I find it a lot more intuitive. I'll open  a discussion on Discord whether we want this change or not.
> > Yeah it seems we manage to get 3 reviews that basically add this macro at the same time.
> > I keep it for now until we finished the discussion which direction to go.
> Sounds good to me. Although there seems to be no opposition to the change.
True, but I expect one of the other two patches to land before this one, so then I can remove this hunk, rebase, and fix the other places. There are some changes here that shouldn't be hiding in this review; but I really want to get a CI run before making these changes.

================
Comment at: libcxx/test/libcxx/clang_tidy.sh.cpp:11

+// UNSUPPORTED: gcc
+
----------------
philnik wrote:
> Mordante wrote:
> > @philnik does running this in GCC have any benefit for this test?
> > I see there are work-arounds for unsupported diagnostics and now I have an unsupported compiler option `-fconstexpr-ops-limit`. I even wonder should this be limited to the clang compiler?
> It also works for people who primarily use GCC, but I guess that's about zero people. So it's probably fine to disable it entirely for GCC. Could you also remove the compatibility stuff in that case?
Yes I intend to do that, however I want to move these changes to their own review.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131317