[PATCH] Windows: Add support for unicode command lines

David Majnemer david.majnemer at gmail.com
Fri Oct 4 07:45:58 PDT 2013



================
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;
----------------
Reid Kleckner wrote:
> 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?
The intent is not to provide the caller with meaningful state in the last two parameters, they may not be used by an implementation; for example, the POSIX implementation doesn't use the second and third arguments at all, it just returns the first parameter.

I suppose I could use ErrorOr<> instead of Optional<>.

================
Comment at: lib/Support/Windows/Program.inc:51
@@ +50,3 @@
+
+retry_search_path:
+  DWORD len = SearchPathW(NULL, progNameUnicode.data(), L".exe",
----------------
Reid Kleckner wrote:
> I'd rather not loop with goto.  There are a couple easy ways to rewrite this with a loop construct.
What's wrong with the goto? I don't see an issue. Would the other ways result in more or less code?

================
Comment at: lib/Support/Windows/Process.inc:191
@@ +190,3 @@
+  SmallVector<size_t, 256> StringOffsets;
+  StringOffsets.reserve(NewArgCount);
+
----------------
Reid Kleckner wrote:
> 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.
This can be done.


http://llvm-reviews.chandlerc.com/D1834



More information about the llvm-commits mailing list