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

[Will Wilson via Phabricator via llvm-commits]

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:
> 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).


================
Comment at: COFF/PDB.cpp:1075-1076
+    bool HasWS = Arg.find(' ') != StringRef::npos;
+    if (HasWS)
+      ArgStr.push_back('"');
+    ArgStr.append(Arg);
----------------
zturner wrote:
> In addition to what rui suggested, can you only do this if the string is not already quoted?  Furthermore, you may need to escape embedded quotes (is that something we need to worry about?)
The arguments always appear to have all double-quotes removed. This applies for both the MS linker and lld-link (by the time this code sees the arguments anyhow).

I've also tried escaping embedded double-quotes but they seem to be eaten by the argument parser and never appear in the tokenized args. Not to mention that there's really no place embedded quotes could be in any valid linker command-line (to the best of my knowledge).

So I can't see either case being worth worrying about.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D46427