[libcxx-commits] [PATCH] D93414: [libc++] Adds a make_string test helper function.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D93414#2522792 <https://reviews.llvm.org/D93414#2522792>, @curdeius wrote:

> LGTM. Just 2 nits.
> Have you looked at what @mstorsjo did in his series of patches for porting filesystem to Windows? I have a vague idea that there were some converting utilities similar to this one.

Thanks for the review. I've didn't have a close look at @mstorsjo's changes. @mstorsjo do you have code in review with similar functionality?



================
Comment at: libcxx/test/support/make_string.h:23
+// Some of these std::codecvt specializations have been deprecated. At the
+// moment are no replacements in the Standard so use the deprecated facilities.
+_LIBCPP_SUPPRESS_DEPRECATED_PUSH
----------------
curdeius wrote:
> Nit: "there are no" instead of "are no".
Will fix.


================
Comment at: libcxx/test/support/make_string.h:30
+std::basic_string<CharT> make_string(const char* str) {
+  struct convertor_type : public std::codecvt<CharT, char, std::mbstate_t> {
+    using std::codecvt<CharT, char, std::mbstate_t>::codecvt;
----------------
curdeius wrote:
> Nit: convertor? Why not converter?
As far as I know they are synonyms, but I'm no native speaker. But it seems converter is more common, so will adjust the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93414