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

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.





================
Comment at: libc/src/__support/CPP/string.h:116
 
+  LIBC_INLINE explicit operator char *() const { return buffer_; }
+
----------------
Is this part of `std::string`, with the `explicit` tag?


================
Comment at: libc/utils/testutils/OwnedString.h:14
+
+template <typename Str_T> class OwnedString {
+  Str_T str;
----------------
michaelrj wrote:
> 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`.
So, two things you say:

1. Doesn't get deallocated.
2. Implicitly castable to `char *`.

I get #2 but can you explain #1 more?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147008