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

[Aaron Ballman via llvm-commits]

aaron.ballman added a comment.

Some small nits, but basically looking good.


================
Comment at: llvm/trunk/lib/Support/CommandLine.cpp:793
@@ +792,3 @@
+  if (!llvm::ConvertUTF8toWide(envVar, wenvVar)) {
+    assert(false);
+    return;
----------------
Can you turn them into `assert(false && "Message about what went wrong");`?

================
Comment at: llvm/trunk/lib/Support/ConvertUTFWrapper.cpp:190
@@ +189,3 @@
+  const UTF8 *ErrorPtr;
+  if (!ConvertUTF8toWide(sizeof(wchar_t), Source, ResultPtr, ErrorPtr))
+  {
----------------
You should run clang-format over this as well, the formatting isn't correct (here and elsewhere in the file).

================
Comment at: llvm/trunk/lib/Support/ConvertUTFWrapper.cpp:243
@@ +242,3 @@
+  else {
+    // Unreachable code
+    return false;
----------------
Use `llvm_unreachable`?


http://reviews.llvm.org/D17549