[PATCH] D26734: [ELF] Don't replace path separators on *NIX

[Davide Italiano via llvm-commits]

davide added a comment.

In https://reviews.llvm.org/D26734#598802, @filcab wrote:

> I'm a bit late because I forgot to submit the comments.
>
> You're missing a Windows test which shows you're replacing the path components.
>  Probably with `REQUIRES: system-windows, x86, cpio`


There are reproduce-windows.s and reproduce-windows-2.s , no?



================
Comment at: lld/trunk/lib/Core/Reproduce.cpp:56
 static void convertToUnixPathSeparator(SmallString<128> &Path) {
+#ifdef LLVM_ON_WIN32
   std::replace(Path.begin(), Path.end(), '\\', '/');
----------------
ruiu wrote:
> filcab wrote:
> > Nit: How about `if (LLVM_ON_WIN32)`?
> > It would force you to always be able to compile the code :-)
> Is that a common practice? I believe for #define'd macros, we usually use #ifdef.
Probably, but we should change also in the Archiver at that point.


================
Comment at: lld/trunk/test/ELF/reproduce-backslash.s:1
+# REQUIRES: x86, cpio, shell
+
----------------
filcab wrote:
> Can `shell` be replaced by `UNSUPPORTED: system-windows`, or are you relying on having a "proper shell" and I'm missing something?
I've seen this used in general to constrain tests to be ran only on Windows (at least in lld).


Repository:
  rL LLVM

https://reviews.llvm.org/D26734