[PATCH] D102339: [clang] On Windows, ignore case and separators when discarding duplicate dependency file paths.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: clang/lib/Frontend/DependencyFile.cpp:145
+  StringRef SearchPath;
+#ifdef _WIN32
+  // Make the search insensitive to case and separators.
----------------
I feel like this should somehow be a property of the input virtual filesystem: we should be able to ask the VFS to do the path canonicalization for us, or ask if the FS is case insensitive and do the lower-casing if so.


================
Comment at: clang/lib/Frontend/DependencyFile.cpp:148
+  llvm::SmallString<256> TmpPath = Filename;
+  std::replace(TmpPath.begin(), TmpPath.end(), '/', '\\');
+  std::transform(TmpPath.begin(), TmpPath.end(), TmpPath.begin(), ::tolower);
----------------
nit: using `sys::path::native` here seems nicer. It replaces `~` with the home dir, but that doesn't seem important.


================
Comment at: clang/test/Frontend/dependency-gen-windows-duplicates.c:12
+// CHECK-NEXT: \test.c
+// CHECK-NEXT: \subdir\x.h
+// File x.h must appear only once (case insensitive check).
----------------
Please reorder the includes and adjust the test to look for SubDir/X.h with capitalization to verify that clang isn't simply producing all lower case output. Your code does this correctly, it canonicalizes the path to check for duplicates, and then prints the filename as written, I just want the test case to reflect that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102339