[libcxx-commits] [PATCH] D125704: [libc++] Make to_chars base 10 header only.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/include/__charconv/tables.h:25
+
+static constexpr char __digits_base_10[200] = {
+    // clang-format off
----------------
ldionne wrote:
> What we want is essentially `inline constexpr char __digits_base_10[200]`, but we can't have that because we need to support C++11. IIRC, the canonical way to do this pre-C++17 was:
> 
> ```
> template <class = void>
> struct __digits_base_10 {
>   static constexpr char __value[200];
> };
> 
> template <class _Tp>
> char __digits_base_10<_Tp>::__value[200] = {
>   // ...
> };
> ```
> 
> Then, you access it as `__digits_base_10<>::__value`. It's not as pretty, but it gets you the `linkonce_odr` semantics that you are after even before C++17.
> 
> 
> Note from live review: We may want to apply the same kind of transformation to the other places where we have this pattern in `<charconv>`. Using `static constexpr` might lead to code bloat.
I did this place only for now. I'll have a look at the other places in a separated commit.


================
Comment at: libcxx/include/__charconv/tables.h:37
+    '9', '0', '9', '1', '9', '2', '9', '3', '9', '4', '9', '5', '9', '6', '9', '7', '9', '8', '9', '9'};
+// clang-format on
+
----------------
ldionne wrote:
> Nit: it's a bit weird that this `clang-format` comment is not aligned with the previous one.
Agreed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125704