[PATCH] Windows: Add support for unicode command lines
Reid Kleckner
rnk at google.com
Fri Oct 4 06:54:29 PDT 2013
================
Comment at: lib/Support/Windows/Program.inc:51
@@ +50,3 @@
+
+retry_search_path:
+ DWORD len = SearchPathW(NULL, progNameUnicode.data(), L".exe",
----------------
I'd rather not loop with goto. There are a couple easy ways to rewrite this with a loop construct.
================
Comment at: lib/Support/Windows/Windows.h:27
@@ -26,1 +26,3 @@
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/OwningPtr.h"
----------------
You should IWYU. This header doesn't use either of these, does it?
================
Comment at: lib/Support/Windows/Process.inc:191
@@ +190,3 @@
+ SmallVector<size_t, 256> StringOffsets;
+ StringOffsets.reserve(NewArgCount);
+
----------------
Rather than doing this dance with StringOffsets to keep stable pointers into a std::vector, you could use a caller-allocated BumpPtrAllocator to allocate the strings. That way you get stable strings by default.
================
Comment at: lib/Support/Windows/Process.inc:178-181
@@ -172,1 +177,6 @@
+Optional<ArrayRef<const char *> >
+Process::GetArgumentVector(ArrayRef<const char *>,
+ OwningArrayPtr<char *> &NewArgVectorPointers,
+ std::vector<char> &NewArgVectorStrings) {
+ int NewArgCount;
----------------
It seems silly to return the new args twice, once by return value and once by out param. Maybe it would be better to return an error_code?
http://llvm-reviews.chandlerc.com/D1834
More information about the llvm-commits
mailing list