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

Nikolas Klauser via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sat Aug 6 03:12:49 PDT 2022


philnik added inline comments.


================
Comment at: libcxx/include/__charconv/tables.h:30
 /// charconv for integrals in C++11 mode.
+#if _LIBCPP_STD_VER > 14
+// TODO(mordante) look at consolidating the tables for C++11/14 and C++17/20/2b
----------------
What is the difference between the versions? Why can't the arrays be constexpr before C++17?


================
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
----------------
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.


================
Comment at: libcxx/include/charconv:33
 
-  to_chars_result to_chars(char* first, char* last, see below value,
-                           int base = 10);
+  constexpr to_chars_result to_chars(char* first, char* last, see below value,
+                                     int base = 10);                                  // constexpr since C++23
----------------
I think we don't add the `constexpr` if it's "constexpr since C++ab".


================
Comment at: libcxx/include/charconv:720-729
+/*
+#include <cmath>
+#include <iostream>
+#include <format>
+
+int main() {
+  for (int i = 2; i <= 36; ++i)
----------------
What does this do here?


================
Comment at: libcxx/include/charconv:752
+#if _LIBCPP_STD_VER > 14
+             auto __digits = __tl::digits / __from_chars_log2f_lut[__b - 2];
+#else
----------------
Is there a reason this isn't in C++11 or 14?


================
Comment at: libcxx/test/support/charconv_test_helpers.h:99
         assert(r.ec == std::errc{});
-        assert(memcmp(buf, expect, len) == 0);
+        assert(__builtin_memcmp(buf, expect, len) == 0);
     }
----------------
Maybe make this a runtime-only test or implement it a constexpr `memcmp` for portability?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131317



More information about the libcxx-commits mailing list