mclow.lists added a comment.
Sorry I've let this lie fallow for so long.
================
Comment at: include/charconv:234
+to_chars(char* __first, char* __last, _Tp __value, int __base)
+ -> to_chars_result
+{
----------------
Why use the trailing return type here?
I don't see any advantage - it doesn't depend on the parameters (template or
runtime).
================
Comment at: src/support/itoa/itoa.cpp:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
lichray wrote:
> EricWF wrote:
> > This file should be renamed `charconv.cpp` and moved into the main source
> > directory.
> We are going to have floating point cpp files so I don't think that one
> charconv.cpp is enough.
We can put both integral and floating point routines in the same source file ;-)
What Eric said - there should be just `charconv.cpp`, and no subdirectory.
Also, if this is not your work, then I need some notice (an email is fine) by
the author saying that they're OK with contributing this under the LLVM license.
================
Comment at: src/support/itoa/itoa.cpp:35
+
+#define APPEND1(i) \
+ do \
----------------
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.
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
