Fix for include file resolution on Windows

[selcott]

Summary

Fix for failure to resolve include files on Windows when the files in question are virtual, i.e. created with mapVirtualFile(). The files are searched for in a hash which is keyed on the file's normalized path as a string. Searches for a file in that hash will fail unless the search path is in the exact same textual form.

[yaron.keren]

I've previously encountered a problem fixing this:

http://reviews.llvm.org/D3686

clang/test/Misc/remap-file.c was failing since FileManager considers / different than \.
Is this test passing for you?

[• rafael]

It should be possible to add a testcase showing that we now add a \ instead of a / on windows, no?

[selcott]

Yes, you're right, remap-file.c fails with this change. FWIW, the test is flawed. It verifies the following very specific (and incorrect) matching logic, which is to accept only paths where all separators excluding the final one are backslashes, and only the final one is a forward slash. I.e. C:\path\to\the/file.h

Anyway, here's another patch that fixes that one too.

lib/Basic/FileManager.cpp

   ++NumFileLookups;
 
   // See if there is already an entry in the map.
   auto &NamedFileEnt =
       *SeenFileEntries.insert(std::make_pair(Filename, nullptr)).first;
 
   // See if there is already an entry in the map.
   if (NamedFileEnt.second && NamedFileEnt.second != NON_EXISTENT_FILE)
     return NamedFileEnt.second;
 
+  // The file name should probably be added to the map in its normalized form
+  // so that searches for the same file in a different form will find it. But
+  // instead of doing that, in order to maintain backwards compatibility I'll
+  // keep the original entry and add a second one in addition to it, keyed on
+  // the normalized form and pointing to the same FileEntry.
+  // Note that this is only necessary here in getVirtualFile but not getFile
+  // because in the case of getFile the key is always the path returned by the
+  // OS after stat'ing the file, which is always the same.
+  SmallString<128> NormalizedPath = Filename;
+  llvm::sys::path::native(NormalizedPath);
+  auto *NormalizedNamedFileEnt = &NamedFileEnt;
+  if (!NormalizedPath.equals(Filename)) {
+    NormalizedNamedFileEnt = &*SeenFileEntries.insert(
+      std::make_pair(NormalizedPath.c_str(), nullptr)).first;
+
+    // If this search succeeded then the file has already been added but in a
+    // different form. This will add another entry corresponding to the form
+    // that was just queried.
+    if (NormalizedNamedFileEnt->second &&
+        NormalizedNamedFileEnt->second != NON_EXISTENT_FILE) {
+      NamedFileEnt.second = NormalizedNamedFileEnt->second;
+      return NormalizedNamedFileEnt->second;
+    }
+  }
+
   ++NumFileCacheMisses;
 
   // By default, initialize it to invalid.
-  NamedFileEnt.second = NON_EXISTENT_FILE;
+  NamedFileEnt.second = NormalizedNamedFileEnt->second = NON_EXISTENT_FILE;
 
   addAncestorsAsVirtualDirs(Filename);
   FileEntry *UFE = nullptr;
 
   // Now that all ancestors of Filename are in the cache, the
   // following call is guaranteed to find the DirectoryEntry from the
   // cache.
   const DirectoryEntry *DirInfo = getDirectoryFromFile(*this, Filename,
                                                        /*CacheFailure=*/true);
   assert(DirInfo &&
          "The directory of a virtual file should already be in the cache.");
 
   // Check to see if the file exists. If so, drop the virtual file
   FileData Data;
   const char *InterndFileName = NamedFileEnt.first().data();
   if (getStatValue(InterndFileName, Data, true, nullptr) == 0) {
     Data.Size = Size;
     Data.ModTime = ModificationTime;
     UFE = &UniqueRealFiles[Data.UniqueID];
 
-    NamedFileEnt.second = UFE;
+    NamedFileEnt.second = NormalizedNamedFileEnt->second = UFE;
 
     // If we had already opened this file, close it now so we don't
     // leak the descriptor. We're not going to use the file
     // descriptor anyway, since this is a virtual file.
     if (UFE->File)
       UFE->closeFile();
 
     // If we already have an entry with this inode, return it.
     if (UFE->isValid())
       return UFE;
 
     UFE->UniqueID = Data.UniqueID;
     UFE->IsNamedPipe = Data.IsNamedPipe;
     UFE->InPCH = Data.InPCH;
   }
 
   if (!UFE) {
     UFE = new FileEntry();
     VirtualFileEntries.push_back(UFE);
-    NamedFileEnt.second = UFE;
+    NamedFileEnt.second = NormalizedNamedFileEnt->second = UFE;
   }
 
   UFE->Name    = InterndFileName;
   UFE->Size    = Size;
   UFE->ModTime = ModificationTime;
   UFE->Dir     = DirInfo;
   UFE->UID     = NextFileUID++;
   UFE->File.reset();
   return UFE;
 }

lib/Lex/HeaderSearch.cpp

       const FileEntry *Includer = IncluderAndDir.first;
 
       // Concatenate the requested file onto the directory.
-      // FIXME: Portability.  Filename concatenation should be in sys::Path.
       TmpDir = IncluderAndDir.second->getName();
-      TmpDir.push_back('/');
-      TmpDir.append(Filename.begin(), Filename.end());
+      llvm::sys::path::append(TmpDir, Filename);
 
       // FIXME: We don't cache the result of getFileInfo across the call to
       // getFileAndSuggestModule, because it's a reference to an element of

lib/Lex/PPDirectives.cpp

   SmallString<128> NormalizedPath;
   if (LangOpts.MSVCCompat) {
     NormalizedPath = Filename.str();
-#ifndef LLVM_ON_WIN32
     llvm::sys::path::native(NormalizedPath);
-#endif
   }
   const FileEntry *File = LookupFile(
       FilenameLoc, LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename,