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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
