[PATCH] D17549: [PATCH] More UTF string conversion wrappers
Aaron Ballman via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 8 15:11:29 PST 2016
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
More information about the llvm-commits
mailing list