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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__charconv/tables.h:25
+
+static constexpr char __digits_base_10[200] = {
+    // clang-format off
----------------
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.


================
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
+
----------------
Nit: it's a bit weird that this `clang-format` comment is not aligned with the previous one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125704