[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