r352593 - Simplify and modernize this code a little.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 29 18:23:34 PST 2019


Author: rsmith
Date: Tue Jan 29 18:23:34 2019
New Revision: 352593

URL: http://llvm.org/viewvc/llvm-project?rev=352593&view=rev
Log:
Simplify and modernize this code a little.

No functionality change intended.

Modified:
    cfe/trunk/lib/Basic/FileManager.cpp

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=352593&r1=352592&r2=352593&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Tue Jan 29 18:23:34 2019
@@ -35,14 +35,6 @@
 
 using namespace clang;
 
-/// NON_EXISTENT_DIR - A special value distinct from null that is used to
-/// represent a dir name that doesn't exist on the disk.
-#define NON_EXISTENT_DIR reinterpret_cast<DirectoryEntry*>((intptr_t)-1)
-
-/// NON_EXISTENT_FILE - A special value distinct from null that is used to
-/// represent a filename that doesn't exist on the disk.
-#define NON_EXISTENT_FILE reinterpret_cast<FileEntry*>((intptr_t)-1)
-
 //===----------------------------------------------------------------------===//
 // Common logic.
 //===----------------------------------------------------------------------===//
@@ -95,14 +87,13 @@ void FileManager::addAncestorsAsVirtualD
   if (DirName.empty())
     DirName = ".";
 
-  auto &NamedDirEnt =
-      *SeenDirEntries.insert(std::make_pair(DirName, nullptr)).first;
+  auto &NamedDirEnt = *SeenDirEntries.insert({DirName, nullptr}).first;
 
   // When caching a virtual directory, we always cache its ancestors
   // at the same time.  Therefore, if DirName is already in the cache,
   // we don't need to recurse as its ancestors must also already be in
-  // the cache.
-  if (NamedDirEnt.second && NamedDirEnt.second != NON_EXISTENT_DIR)
+  // the cache (or it's a known non-virtual directory).
+  if (NamedDirEnt.second)
     return;
 
   // Add the virtual directory to the cache.
@@ -136,19 +127,17 @@ const DirectoryEntry *FileManager::getDi
 #endif
 
   ++NumDirLookups;
-  auto &NamedDirEnt =
-      *SeenDirEntries.insert(std::make_pair(DirName, nullptr)).first;
 
   // See if there was already an entry in the map.  Note that the map
   // contains both virtual and real directories.
-  if (NamedDirEnt.second)
-    return NamedDirEnt.second == NON_EXISTENT_DIR ? nullptr
-                                                  : NamedDirEnt.second;
+  auto SeenDirInsertResult = SeenDirEntries.insert({DirName, nullptr});
+  if (!SeenDirInsertResult.second)
+    return SeenDirInsertResult.first->second;
 
+  // We've not seen this before. Fill it in.
   ++NumDirCacheMisses;
-
-  // By default, initialize it to invalid.
-  NamedDirEnt.second = NON_EXISTENT_DIR;
+  auto &NamedDirEnt = *SeenDirInsertResult.first;
+  assert(!NamedDirEnt.second && "should be newly-created");
 
   // Get the null-terminated directory name as stored as the key of the
   // SeenDirEntries map.
@@ -184,18 +173,14 @@ const FileEntry *FileManager::getFile(St
   ++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)
-    return NamedFileEnt.second == NON_EXISTENT_FILE ? nullptr
-                                                    : NamedFileEnt.second;
+  auto SeenFileInsertResult = SeenFileEntries.insert({Filename, nullptr});
+  if (!SeenFileInsertResult.second)
+    return SeenFileInsertResult.first->second;
 
+  // We've not seen this before. Fill it in.
   ++NumFileCacheMisses;
-
-  // By default, initialize it to invalid.
-  NamedFileEnt.second = NON_EXISTENT_FILE;
+  auto &NamedFileEnt = *SeenFileInsertResult.first;
+  assert(!NamedFileEnt.second && "should be newly-created");
 
   // Get the null-terminated file name as stored as the key of the
   // SeenFileEntries map.
@@ -218,7 +203,7 @@ const FileEntry *FileManager::getFile(St
   // FIXME: Use the directory info to prune this, before doing the stat syscall.
   // FIXME: This will reduce the # syscalls.
 
-  // Nope, there isn't.  Check to see if the file exists.
+  // Check to see if the file exists.
   std::unique_ptr<llvm::vfs::File> F;
   FileData Data;
   if (getStatValue(InterndFileName, Data, true, openFile ? &F : nullptr)) {
@@ -240,13 +225,9 @@ const FileEntry *FileManager::getFile(St
   // If the name returned by getStatValue is different than Filename, re-intern
   // the name.
   if (Data.Name != Filename) {
-    auto &NamedFileEnt =
-        *SeenFileEntries.insert(std::make_pair(Data.Name, nullptr)).first;
-    if (!NamedFileEnt.second)
-      NamedFileEnt.second = &UFE;
-    else
-      assert(NamedFileEnt.second == &UFE &&
-             "filename from getStatValue() refers to wrong file");
+    auto &NamedFileEnt = *SeenFileEntries.insert({Data.Name, &UFE}).first;
+    assert(NamedFileEnt.second == &UFE &&
+           "filename from getStatValue() refers to wrong file");
     InterndFileName = NamedFileEnt.first().data();
   }
 
@@ -295,19 +276,13 @@ FileManager::getVirtualFile(StringRef Fi
                             time_t ModificationTime) {
   ++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)
+  // See if there is already an entry in the map for an existing file.
+  auto &NamedFileEnt = *SeenFileEntries.insert({Filename, nullptr}).first;
+  if (NamedFileEnt.second)
     return NamedFileEnt.second;
 
+  // We've not seen this before, or the file is cached as non-existent.
   ++NumFileCacheMisses;
-
-  // By default, initialize it to invalid.
-  NamedFileEnt.second = NON_EXISTENT_FILE;
-
   addAncestorsAsVirtualDirs(Filename);
   FileEntry *UFE = nullptr;
 
@@ -343,9 +318,7 @@ FileManager::getVirtualFile(StringRef Fi
     UFE->IsNamedPipe = Data.IsNamedPipe;
     UFE->InPCH = Data.InPCH;
     fillRealPathName(UFE, Data.Name);
-  }
-
-  if (!UFE) {
+  } else {
     VirtualFileEntries.push_back(llvm::make_unique<FileEntry>());
     UFE = VirtualFileEntries.back().get();
     NamedFileEnt.second = UFE;
@@ -479,6 +452,9 @@ void FileManager::invalidateCache(const
   // FileEntry invalidation should not block future optimizations in the file
   // caches. Possible alternatives are cache truncation (invalidate last N) or
   // invalidation of the whole cache.
+  //
+  // FIXME: This is broken. We sometimes have the same FileEntry* shared
+  // betweeen multiple SeenFileEntries, so this can leave dangling pointers.
   UniqueRealFiles.erase(Entry->getUniqueID());
 }
 
@@ -491,13 +467,12 @@ void FileManager::GetUniqueIDMapping(
   for (llvm::StringMap<FileEntry*, llvm::BumpPtrAllocator>::const_iterator
          FE = SeenFileEntries.begin(), FEEnd = SeenFileEntries.end();
        FE != FEEnd; ++FE)
-    if (FE->getValue() && FE->getValue() != NON_EXISTENT_FILE)
+    if (FE->getValue())
       UIDToFiles[FE->getValue()->getUID()] = FE->getValue();
 
   // Map virtual file entries
   for (const auto &VFE : VirtualFileEntries)
-    if (VFE && VFE.get() != NON_EXISTENT_FILE)
-      UIDToFiles[VFE->getUID()] = VFE.get();
+    UIDToFiles[VFE->getUID()] = VFE.get();
 }
 
 void FileManager::modifyFileEntry(FileEntry *File,
@@ -519,7 +494,7 @@ StringRef FileManager::getCanonicalName(
   if (!FS->getRealPath(Dir->getName(), CanonicalNameBuf))
     CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
 
-  CanonicalDirNames.insert(std::make_pair(Dir, CanonicalName));
+  CanonicalDirNames.insert({Dir, CanonicalName});
   return CanonicalName;
 }
 




More information about the cfe-commits mailing list