[libc-commits] [PATCH] D147231: [libc] Adds string and TestLogger classes, use them in LibcTest

[Guillaume Chatelet via Phabricator via libc-commits]

gchatelet marked 5 inline comments as done.
gchatelet added inline comments.


================
Comment at: libc/src/__support/CPP/string.h:29
+private:
+  static constexpr char kNullCharacter = '\0';
+  static constexpr char *getEmptyString() {
----------------
sivachandra wrote:
> Nit: `NULL_CHARACTER`
> 
Thx, I keep on forgetting the style guide.
We should try to enforce it via linter.


================
Comment at: libc/test/UnitTest/TestLogger.cpp:5
+
+#include <stdio.h> // fwrite, fputs, stdout
+
----------------
sivachandra wrote:
> gchatelet wrote:
> > sivachandra wrote:
> > > We want this to work on really minimal systems on which `fwrite`, `fputs` `stdout` etc are not [yet] available. So, we should use a simple function like https://github.com/llvm/llvm-project/blob/main/libc/src/__support/OSUtil/linux/io.h which does a raw write to stdout.
> > It would be helpful to provide a version of `__llvm_libc::write_to_stderr` that takes the length as an argument. Would that be OK?
> > As is, the call is suboptimal for single `char` and `string_view`.
> A lot of those utils are improved as required. So, if you think this patch will benefit, either add more args or add overloads. Totally fine.
Will do in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147231