mclow.lists added inline comments.
================ Comment at: include/charconv:89 +_LIBCPP_BEGIN_NAMESPACE_STD + +enum class _LIBCPP_ENUM_VIS chars_format ---------------- lichray wrote: > EricWF wrote: > > We need to hide these names when `_LIBCPP_STD_VER < 17`, since we're not > > allowed to introduce new names into namespace `std` in older dialects. > But this header is backported to C++11, so I intended to not to guard it. > But this header is backported to C++11, so I intended to not to guard it. In general, we don't provide new features for old language versions. The only time we've ever done that is for `string_view`, and I'm **still** not sure I did the right thing there. ================ Comment at: src/charconv.cpp:34 + +#define APPEND1(i) \ + do \ ---------------- I'd really like to see some numbers here showing how this (somewhat awkward) code performs better than the straightforward versions. Because maintenance is forever. Note: I'm sure that this was true on older compilers - but is it true today? ================ Comment at: test/support/charconv_test_helpers.h:24 + +template <typename To, typename From> +constexpr auto ---------------- If this is supposed to be a C++17 or later header (and I'm pretty sure it is), you should add a `static_assert(TEST_STD_VER > 14, "");` to this header. Repository: rCXX libc++ https://reviews.llvm.org/D41458 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
