[PATCH] D78957: [WIP][FileCollector] isCaseSensitivePath

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: llvm/lib/Support/FileCollector.cpp:29
   // default.
+  // FIXME: Path could've already been ALL CAPS so we should also use lower().
   UpperDest = Path.upper();
----------------
Yes, optimally we should compare the realpath of `Path.lower()` with the realpath of `Path.u[per()`, but realpath is an expensive operation, so this seems like a reasonable enough heuristic. 


================
Comment at: llvm/lib/Support/FileCollector.cpp:182
   VFSWriter.setOverlayDir(OverlayRoot);
+  // FIXME: Technically even on a case-insensitive file system programs can have
+  // their own quirks in path handling. Is there actually much to gain by not
----------------
I'm not sure I fully understand your point so apologies if my answer doesn't actually address your question. The case-sensitivity is used by the RedirectingFileSystem for lookups. It ensures that `/foo/bar` and `/foo/Bar` resolve to the same file as would be the case on the real file system. You need the deduplication in the mapping because if you are traversing the directory with a recursive directory iterator, you want both `/foo/Bar/baz` and `/foo/bar/quux` to show up under `/foo/bar` and `/foo/Bar`.


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

https://reviews.llvm.org/D78957