lichray marked 7 inline comments as done.
lichray added a comment.
Herald added a subscriber: christof.
Pending patch update due to poor network.
================
Comment at: include/charconv:90
+
+enum class _LIBCPP_ENUM_VIS chars_format
+{
----------------
EricWF wrote:
> enum types should have their underlying integer type explicitly specified.
> Otherwise their size and ABI can break when users specify `-short-enums`
The standard requires `enum class` to default to `int`; IIUC `enum class` is
not affected by `-fshort-enums`.
================
Comment at: include/support/itoa/itoa.h:1
+// -*- C++ -*-
+//===--------------------- support/itoa/itoa.h
----------------------------===//
----------------
EricWF wrote:
> I would rather not introduce another header for this. I think it should go
> directly in the `charconv` header.
We may have separated floating point headers coming as well...
================
Comment at: include/support/itoa/itoa.h:47
+
+static constexpr uint32_t __pow10_32[] = {
+ UINT32_C(0), UINT32_C(10), UINT32_C(100),
----------------
EricWF wrote:
> I suspect we can use `__pow10_64` in the 32 bit case as well to avoid
> emitting another static global.
I have functions returning them as array references.
================
Comment at: include/support/itoa/itoa.h:54
+
+#if __has_builtin(__builtin_clzll)
+
----------------
EricWF wrote:
> Use `__clz` from `<algorithm>`. You can assume we always have that.
Factor `__clz` out of `algorithm` may be too much for this review. I would
like to see a subsequent patch to take care of all the related portability
fixes.
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
