[PATCH] D17549: [PATCH] More UTF string conversion wrappers

[Aaron Ballman via llvm-commits]

aaron.ballman added inline comments.

================
Comment at: llvm/trunk/include/llvm/Support/ConvertUTF.h:204
@@ +203,3 @@
+*/
+bool ConvertUTF8toWide(llvm::StringRef Source, std::wstring& Result);
+
----------------
& and * bind to the identifier instead of the type in our coding conventions (here and elsewhere). Run the patch through clang-format and it should fix all of those up automatically, as well as other formatting issues in the patch.

================
Comment at: llvm/trunk/include/llvm/Support/ConvertUTF.h:216
@@ +215,3 @@
+*/
+bool convertWideToUTF8(std::wstring const& Source, std::string& Result);
+
----------------
Inconsistent capitalization of the function name.

================
Comment at: llvm/trunk/lib/Support/CommandLine.cpp:793
@@ +792,3 @@
+  if (!llvm::ConvertUTF8toWide(envVar, wenvVar))
+    return;
+  const wchar_t *wenvValue = _wgetenv(wenvVar.c_str());
----------------
I feel like maybe we shouldn't silently eat this sort of failure, but should either report a fatal error or assert. I don't feel strongly about it, however.

================
Comment at: llvm/trunk/lib/Support/CommandLine.cpp:799
@@ +798,3 @@
+  if (!llvm::convertWideToUTF8(wenvValue, envValueBuffer))
+    return;
+  const char *envValue = envValueBuffer.c_str();
----------------
Same for this failure.

================
Comment at: llvm/trunk/lib/Support/ConvertUTFWrapper.cpp:140
@@ +139,3 @@
+
+bool convertUTF16ToUTF8String(const UTF16 *Src, std::string &Out)
+{
----------------
This function overload does not appear to be used, is it required?


http://reviews.llvm.org/D17549