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

Filipe Cabecinhas via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 17 13:38:15 PST 2016


filcab added inline comments.


================
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(), '\\', '/');
----------------
filcab wrote:
> filcab wrote:
> > ruiu wrote:
> > > davide wrote:
> > > > davide wrote:
> > > > > 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.
> > > > Just checked, and noticed we're inconsistent here.
> > > > 
> > > > ```
> > > > [davide at cupiditate lib]$ grep -R 'if LLVM_ON_WIN32' * | wc -l
> > > > 8
> > > > [davide at cupiditate lib]$ grep -R 'ifdef LLVM_ON_WIN32' * | wc -l
> > > > 29
> > > > ```
> > > > So, Filipe, you prefer the other spelling? If so, I can change, but I'd rather change it everywhere.
> > > I was thinking that Filipe suggested use C's `if` instead of C preprocessor's #ifdef (which I don't agree).
> > I'd prefer the `if (LLVM_ON_WIN32)` variant, as it doesn't allow for code that doesn't compile. But if the regular contributors have objections, I don't mind it too much.
> ruiu: It's very common in the sanitizers, and it ends up being cleaner and safer.
Since Rui doesn't seem to agree, I'll withdraw this comment.


Repository:
  rL LLVM

https://reviews.llvm.org/D26734





More information about the llvm-commits mailing list