[libcxx-commits] [PATCH] D121223: [libc++][NFC] Cleanups in <charconv>.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision.
Quuxplusone added a comment.
This revision is now accepted and ready to land.

LGTM if CI is happy!



================
Comment at: libcxx/include/charconv:107-108
 namespace __itoa {
-_LIBCPP_AVAILABILITY_TO_CHARS _LIBCPP_FUNC_VIS char* __u64toa(uint64_t __value, char* __buffer) _NOEXCEPT;
-_LIBCPP_AVAILABILITY_TO_CHARS _LIBCPP_FUNC_VIS char* __u32toa(uint32_t __value, char* __buffer) _NOEXCEPT;
+_LIBCPP_AVAILABILITY_TO_CHARS _LIBCPP_FUNC_VIS char* __u64toa(uint64_t __value, char* __buffer) noexcept;
+_LIBCPP_AVAILABILITY_TO_CHARS _LIBCPP_FUNC_VIS char* __u32toa(uint32_t __value, char* __buffer) noexcept;
 } // namespace __itoa
----------------
philnik wrote:
> I think this fails in C++03, but I don't know why it is available in C++03 at all.
+1; could this be moved under the following `#if`?


================
Comment at: libcxx/include/charconv:171
 struct _LIBCPP_HIDDEN
     __traits_base<_Tp, decltype(void(uint32_t{declval<_Tp>()}))>
 {
----------------
Pre-existing, tangential: I notice that here the SFINAE is testing explicit curly-brace-initialization from `_Tp` to `uint32_t`, but what's actually used on line 184 is implicit conversion (and ADL on `_Tp`). If `_Tp` is always a built-in integer type, this is probably all fine.


================
Comment at: libcxx/include/charconv:323
 template <typename _Tp>
-_LIBCPP_AVAILABILITY_TO_CHARS _LIBCPP_INLINE_VISIBILITY int __to_chars_integral_width(_Tp __value, unsigned __base) {
+_LIBCPP_AVAILABILITY_TO_CHARS _LIBCPP_HIDE_FROM_ABI int __to_chars_integral_width(_Tp __value, unsigned __base) {
   _LIBCPP_ASSERT(__value >= 0, "The function requires a non-negative value.");
----------------
Consider breaking after `int`, to match lines 308-309.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121223