[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