[libc-commits] [PATCH] D147008: [libc][bazel] add file printf targets and support

Michael Jones via Phabricator via libc-commits libc-commits at lists.llvm.org
Wed Apr 12 14:02:58 PDT 2023


michaelrj added inline comments.


================
Comment at: libc/utils/testutils/OwnedString.h:14
+
+template <typename Str_T> class OwnedString {
+  Str_T str;
----------------
sivachandra wrote:
> Why is this class required?
This class holds the `cpp::string` so that it doesn't get deallocated, and is implicitly castable to `char *`. 

In the test, the pattern is `auto FILE_PATH = libc_make_test_file_path("filepath");`, and then the `FILE_PATH` can be used as a `char *`. The actual type of `FILE_PATH` might be `char*` or `OwnedString`, but both work as `char*`. 
We can't just have `FILE_PATH` be a `cpp::string` because `cpp::string` isn't implicitly castable to `char*`, and I'm trying to avoid needing a static cast every time the `FILE_PATH` is used.
Similarly, the `FILE_PATH` can't just be a `char*` since if we don't store the string it gets deallocated.
The original intent of this class was for `FILE_PATH` to always be an `OwnedString`, which is why it's a template, but that's not necessary now.

I've also moved this to `__support/CPP/` since `LibcTest.h` says to only include headers from that folder and `testutils`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147008



More information about the libc-commits mailing list