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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to