[PATCH] D45550: Use GetArgumentVector to retrieve the utf-8 encoded arguments on all platforms

[Stella Stamenova via Phabricator via llvm-commits]

stella.stamenova added inline comments.


================
Comment at: tools/lld/lld.cpp:129-130
+  SpecificBumpPtrAllocator<char> ArgAllocator;
+  ExitOnErr(errorCodeToError(sys::Process::GetArgumentVector(
+      Argv, makeArrayRef(Argv_, Argc_), ArgAllocator)));
+
----------------
ruiu wrote:
> stella.stamenova wrote:
> > ruiu wrote:
> > > Why is this function's signature so complex? How can it fail?
> > On Windows, this calls several functions which can fail:
> > * GetCommandLineW
> > * CommandLineToArgvW
> > * WideCharToMultiByte
> IIUC, you want to do the same thing for all llvm tools that have their own main() functions. And I think that you always exit on error. If that's the case, I'd reduce the amount of boilerplate by moving code.
We don't intend on making the change to all tools at this time - just the ones needed to make the test pass - in order to scope the work.

GetArgumentVector is used from a variety of places not all of which share the same code exit behavior (though most do). The same is true for PrintStackTraceOnErrorSignal and PrettyStackTraceProgram. Since all of these are frequently used together (and it's probably an omission of they are not), I agree it would be a great idea to create a simpler pattern than can be used by tools. To properly make the change, we would have to update all the tools to use the new pattern, which is also out of our scope right now. Perhaps we should open a bug/task to be addressed later?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D45550