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

Cameron via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 8 15:28:32 PST 2016


cameron314 added inline comments.

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

================
Comment at: llvm/trunk/lib/Support/ConvertUTFWrapper.cpp:190
@@ +189,3 @@
+  const UTF8 *ErrorPtr;
+  if (!ConvertUTF8toWide(sizeof(wchar_t), Source, ResultPtr, ErrorPtr))
+  {
----------------
aaron.ballman wrote:
> You should run clang-format over this as well, the formatting isn't correct (here and elsewhere in the file).
Will do. I was hoping to avoid this since clang-format changes a lot of code that was already present as well. I'll extract just the parts that apply to my patch, I suppose.

================
Comment at: llvm/trunk/lib/Support/ConvertUTFWrapper.cpp:243
@@ +242,3 @@
+  else {
+    // Unreachable code
+    return false;
----------------
aaron.ballman wrote:
> Use `llvm_unreachable`?
Aha, didn't know it existed :-)


http://reviews.llvm.org/D17549





More information about the llvm-commits mailing list