[PATCH] D68033: [llvm-ar] Make paths case insensitive when on windows

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

I sympathize MaskRay, but in practice I think we want this because on Windows filenames are case-insensitive. That is what users expect, and I personally don't care too much about cases when passing a filename to a command. If (only) ar distinguishes pathnames in different cases, I'd be surprised.



================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:491
+
+  if (Triple(sys::getProcessTriple()).isOSWindows())
+    llvm::transform(NormalizedPath, NormalizedPath.begin(), ::toupper);
----------------
Instead of detecting the OS at runtime, you should use `#ifdef _WIN32` because once you compile this for Windows, the run-time OS must be Windows.

This new code needs a comment justifying why we are doing this.


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:492
+  if (Triple(sys::getProcessTriple()).isOSWindows())
+    llvm::transform(NormalizedPath, NormalizedPath.begin(), ::toupper);
+
----------------
This is perhaps my personal preference, but I prefer making it lowercase rather than uppercase when normalizing.


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

https://reviews.llvm.org/D68033