[PATCH] D64538: [Driver] Don't escape backslashes on Windows

Wed Jul 10 19:30:06 PDT 2019

rnk added a comment.

In D64538#1579632 <https://reviews.llvm.org/D64538#1579632>, @smeenai wrote:

> I'm not fully understanding this. If the .sh file was generated on Windows, it'll contain backslashes in any arguments that are paths, right? Before this change, those backslashes would have been doubled up, but the backslash still wouldn't work as a path separator on Linux, so those arguments would need manual adjustment. I guess the other case that matters is if you have a backslash in an argument which isn't a path, in which case the old behavior was definitely correct and the new one definitely isn't.

At least in Chromium, users pass flags like `-DFOO_EXPORT="__declspec(dllexport)"`. I can't remember a concrete instance where a user passed non-path flags with backslashes in them, but you could imagine a hypothetical argument like: `-DSOME_STRING="foo \"bar baz\""`, and if we don't escape those backslashes, the argument will be tokenized incorrectly. I guess that example is an objection to this part of the comment:

>   // double quotes only when it's followed by $, `, ", \, or a literal newline;
>   // the last three are disallowed in paths on Windows and the first two are
>   // unlikely, so this shouldn't cause issues in practice. Note that we always

I can also imagine tokenization going wrong if some inconsequential path ends in a trailing backslash: `-fdebug-compilation-dir=C:\foo\bar\` -> `"-fdebug-compilation-dir=C:\foo\bar\"`

I like what we do now because it's safe and handles the general case of backslashes in flags without any gotchas or corner cases. If the specific issue that we have is that driver tests are hard to write because of the variance in path printing, driver testing could already be greatly improved by adding a filecheck-friendly version of -###, so we should do that anyway.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64538/new/

https://reviews.llvm.org/D64538