[PATCH] Support case insensitive header searches for MSVCCompat

Richard Smith richard at metafoo.co.uk
Thu Mar 20 15:49:04 PDT 2014



================
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);
 
----------------
Saleem Abdulrasool wrote:
> 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.
We should presumably make the decision on a per-directory basis. I'd expect it can change across junction points, not just across volumes.

================
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;
+
----------------
Saleem Abdulrasool wrote:
> 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.
But `directory_iterator` only walks a single level at a time, right? If you meant to perform a recursive walk looking for missing directory components, not just the last level, then I think that

 1) you're missing the code to do that (or maybe I've just overlooked how you're achieving that), and
 2) you still only want to compare the last component at each stage in your tree walk (rather than wasting time walking through the files in directories that don't match).

================
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;
+        }
+      }
+    }
----------------
Saleem Abdulrasool wrote:
> 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.
I'd be inclined to think that a `FatalError` is the right way to respond to a directory operation that fails, unless we have a reason to continue past one? Hmm, maybe we want to ignore permissions problems?


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



More information about the cfe-commits mailing list