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

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/src/__support/CPP/OwnedString.h:1
+//===-- Implementation of a struct to hold a string in menory -------------===//
+//
----------------
sivachandra wrote:
> This class should live outside of the `CPP` directory.
I can't move it out of `CPP` because it's included from `LibcTest.h` and `LibcTest.h` specifies that it can only include headers from `TestUtils` and `CPP`.


================
Comment at: libc/src/__support/CPP/OwnedString.h:19
+// for if that char* needs to be freed.
+class OwnedString {
+  string str;
----------------
sivachandra wrote:
> You can make this a template class like this:
> 
> ```
> template <typename T>
> class CString {
>   T str;
> public:
>   // These constructors can be implemented iff required.
>   CString() = delete;
>   CString(const CString &) = delete;
>   CString(CString &&) = delete;
> 
>   LIBC_INLINE CString(const T &s) : str(s) {}
>   LIBC_INLINE operator const char *() const { return str.data(); }
> };
> 
> template <>
> class CString<const char *> {
>   const char *str;
> public:
>   // These constructors can be implemented iff required.
>   CString() = delete;
>   CString(const CString &) = delete;
>   CString(CString &&) = delete;
> 
>   LIBC_INLINE CString(const char *s) : s(str) {}
>   LIBC_INLINE operator const char *() const { return str; }
> };
> ```
> 
> The template will work with `cpp::string_view` and `cpp::span` also.
I can't make this class a template because the function it's returning from would need to know the template arguments, since they can't be deduced in a function return type. Both the Bazel and CMake version of the function would therefore need to be the same template type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147008