[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Zhihao Yuan via Phabricator via cfe-commits]

lichray marked 3 inline comments as done.
lichray added inline comments.


================
Comment at: include/charconv:234
+to_chars(char* __first, char* __last, _Tp __value, int __base)
+    -> to_chars_result
+{
----------------
mclow.lists wrote:
> Why use the trailing return type here?
> I don't see any advantage - it doesn't depend on the parameters (template or runtime).
> 
> 
Because clang-format doesn't distinguish storage specifiers and simple-type-specifiers, and I want to format return types along with function signatures rather than letting hanging somewhere.


================
Comment at: src/support/itoa/itoa.cpp:35
+
+#define APPEND1(i)                              \
+    do                                          \
----------------
mclow.lists wrote:
> lichray wrote:
> > EricWF wrote:
> > > Any reason these can't be `static` functions? The compiler should optimize them away nicely.
> > Although yes, but that's what the author provides.  It's an implementation file, so it doesn't matter I guess.
> It *does* matter, since we'll have to maintain this.
> 
> It would also be nice if they had meaningful names.
I tried (template + forceinline), and the binary was optimized differently (emitted ~100 more instructions).  The author have tuned these really well.  A syntax like `buffer = append1(buffer, v)` may work better, I guess, but that adds lots of noises.

These macros are not complex.  They just emit a integer of width 1234 to digits.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458