[PATCH] D46427: [PDB] Quote linker arguments containing spaces (mimic MSVC)

Fri May 11 11:14:08 PDT 2018

lantictac added inline comments.

================
Comment at: COFF/PDB.cpp:1068
+
+  // MSVC surrounds arguments containing spaces with quotes, so we do the same.
+  SmallString<256> ArgStr;
----------------
ruiu wrote:
> lantictac wrote:
> > ruiu wrote:
> > > lantictac wrote:
> > > > ruiu wrote:
> > > > > Can you factor this code as `std::string quote(ArayRef<StringRef>)`?
> > > > > 
> > > > > I believe you also need to quote a double-quote as double double-quotes as well (but please check what the MS linker behavior is first.)
> > > > I'm not quite sure what you meant by "quote a double-quote as double double-quotes", or at least I couldn't find or reproduce anything to support this way of escaping a double-quote. Any further clues?
> > > > 
> > > > Other than that, I've factored out the code into a quote() function as requested (patch pending).
> > > I don't know if it is applicable to this field, but in Windows command line, the way to escape a single double-quote in a double-quoted string is to replace it with two double-quote characters.
> > > 
> > > For example, to get `hello "world"`, one needs to quote it as `"hello ""world"""`.
> > > 
> > > At least the way you escape a string seems ambiguous when a string contains a double-quote character.
> > It doesn't seem applicable in this case. At least I'm finding it impossible to get a single quote to show up in the args however I specify the command-line.
> > 
> > So as far as I can ascertain it shouldn't be a concern for this code.
> It is possible to pass a single double-quote to the command and as a result the double-quote character shows up in Config->Args. Here is an example:
> 
>   > lld-link "Hello ""World"""
>   C:\src\b\bin\lld-link.exe: error: could not open Hello "World": invalid argument
Thanks for clarifying. I'll see if I can get it to to repro on a Windows build with a Windows specific test case.

https://reviews.llvm.org/D46427