[PATCH] Windows: Add support for unicode command lines

Reid Kleckner rnk at google.com
Fri Oct 4 10:08:18 PDT 2013



================
Comment at: lib/Support/Windows/Process.inc:191
@@ +190,3 @@
+  SmallVector<size_t, 256> StringOffsets;
+  StringOffsets.reserve(NewArgCount);
+
----------------
David Majnemer wrote:
> David Majnemer wrote:
> > 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.
> Does the following interface seem reasonable to you?
> ```
>   error_code
>   GetArgumentVector(SmallVectorImpl<const char *> &NewArgs,
>                     ArrayRef<const char *> ArgsFromMain,
>                     SpecificBumpPtrAllocator<char> &ArgAllocator);
> ```
Looks good.

If we wanted to get even more minimal, we could use a MutableArrayRef<> to modify the original argv array in place!  =D  If the arg counts don't match we'd return an error and leave the array untouched.

================
Comment at: include/llvm/Support/Process.h:175
@@ +174,3 @@
+  /// for program execution.
+  static Optional<ArrayRef<const char *> >
+  GetArgumentVector(ArrayRef<const char *> OriginalMainArgs,
----------------
Rui Ueyama wrote:
> In Unix this function does nothing and does not use any argument. In Windows the first argument (OriginalMainArgs) is ignored. It feels like they are really different, and they don't not necessarily have to have the same interface.
> 
> I'd probably rather make this a function defined only when LLVM_ON_WIN32 is defined, and guard the call of this function in LLVM's main() with LLVM_ON_WIN32.
I'd rather avoid ifdefs in main.

================
Comment at: lib/Support/Windows/Program.inc:51
@@ +50,3 @@
+
+retry_search_path:
+  DWORD len = SearchPathW(NULL, progNameUnicode.data(), L".exe",
----------------
Rui Ueyama wrote:
> David Majnemer wrote:
> > 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?
> This can easily be re-written with for(;;) { ...; if (len <= buffer.capacity()) break; ...}. It seems that's more normal way to write this kind of logic than goto.
I was thinking:

  DWORD len = progNameUnicode.capacity();
  do {
    progNameUnicode.resize(len);  // should be a nop on the first iteration.
    len = SearchPathW(...);
  } while (len > buffer.capacity());


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



More information about the llvm-commits mailing list