[PATCH] Support case insensitive header searches for MSVCCompat

Saleem Abdulrasool abdulras at fb.com
Thu Mar 20 15:33:31 PDT 2014



================
Comment at: include/clang/Lex/DirectoryLookup.h:66
@@ +65,3 @@
+  /// \brief Whether the lookup should be case insensitive.  This is required
+  /// for compatibility with case-sensitive file systems and Mcirosoft mode
+  /// which does not include headers with the correct case.
----------------
Richard Smith wrote:
> Typo of "Microsoft"
Good eye.

================
Comment at: lib/Frontend/InitHeaderSearch.cpp:679
@@ -673,3 +678,3 @@
                                      const llvm::Triple &Triple) {
-  InitHeaderSearch Init(HS, HSOpts.Verbose, HSOpts.Sysroot);
+  InitHeaderSearch Init(HS, HSOpts.Verbose, HSOpts.Sysroot, Lang.MSVCCompat);
 
----------------
Richard Smith wrote:
> We don't want to pay the cost of this if the underlying file system is already case-insensitive. On Windows, `GetVolumeInformation` can be used to determine this. I don't think there's a good way to determine it for Linux, sadly.
> 
> (We should also use this in `FileManager` rather than making an assumption that Windows is case-insensitive and nothing else is there.)
Agreed.  What volume would you say that we query that on?  We could be compiling on a volume different from the compiler.  The headers could come from various volumes as well.

================
Comment at: lib/Lex/HeaderSearch.cpp:36-42
@@ +35,9 @@
+
+  for (PI = llvm::sys::path::rbegin(LowerPath),
+       PE = llvm::sys::path::rend(LowerPath),
+       FI = llvm::sys::path::rbegin(LowerFilename),
+       FE = llvm::sys::path::rend(LowerFilename);
+       FI != FE && PI != PE; ++PI, ++FI)
+    if (FI->str() != PI->str())
+      return false;
+
----------------
Richard Smith wrote:
> Do you need to compare anything other than the last component here?
for a multi-component file path, a non-terminal component may mismatch.  If a multi-component path has a matching terminal-component, that would also get incorrectly included otherwise.

================
Comment at: lib/Lex/HeaderSearch.cpp:257-265
@@ +256,11 @@
+    if (CaseInsensitive && !llvm::sys::fs::exists(TmpDir.str())) {
+      llvm::error_code EC;
+      std::string LowerFilename = Filename.lower();
+      for (llvm::sys::fs::directory_iterator DI(getDir()->getName(), EC), DE;
+           !EC && DI != DE; DI = DI.increment(EC)) {
+        if (IsEquivalent(DI->path(), LowerFilename)) {
+          TmpDir = DI->path();
+          break;
+        }
+      }
+    }
----------------
Richard Smith wrote:
> Please factor this out. Should you do something better with `EC` than ignoring it?
I dont think I can do much with EC.  I suppose I could print out a warning (but that seems like it would clutter the output).  But, I will pull out the loop into a function.


http://llvm-reviews.chandlerc.com/D2972



More information about the cfe-commits mailing list