[clang] Adopt llvm::ErrorOr in FileManager methods
authorHarlan Haskins <harlan@harlanhaskins.com>
Thu, 1 Aug 2019 21:31:49 +0000 (21:31 +0000)
committerHarlan Haskins <harlan@harlanhaskins.com>
Thu, 1 Aug 2019 21:31:49 +0000 (21:31 +0000)
Previously, the FileManager would use NULL returns to signify whether a file existed, but that doesn’t cover permissions issues or anything else that might occur while trying to stat or read a file. Instead, convert getFile and getDirectory into returning llvm::ErrorOr

Signed-off-by: Harlan Haskins <harlan@apple.com>
llvm-svn: 367615

clang/include/clang/Basic/FileManager.h
clang/lib/Basic/FileManager.cpp
clang/unittests/Basic/FileManagerTest.cpp

index f38664a..27bfb5f 100644 (file)
@@ -132,20 +132,24 @@ class FileManager : public RefCountedBase<FileManager> {
   SmallVector<std::unique_ptr<FileEntry>, 4> VirtualFileEntries;
 
   /// A cache that maps paths to directory entries (either real or
-  /// virtual) we have looked up
+  /// virtual) we have looked up, or an error that occurred when we looked up
+  /// the directory.
   ///
   /// The actual Entries for real directories/files are
   /// owned by UniqueRealDirs/UniqueRealFiles above, while the Entries
   /// for virtual directories/files are owned by
   /// VirtualDirectoryEntries/VirtualFileEntries above.
   ///
-  llvm::StringMap<DirectoryEntry*, llvm::BumpPtrAllocator> SeenDirEntries;
+  llvm::StringMap<llvm::ErrorOr<DirectoryEntry &>, llvm::BumpPtrAllocator>
+  SeenDirEntries;
 
   /// A cache that maps paths to file entries (either real or
-  /// virtual) we have looked up.
+  /// virtual) we have looked up, or an error that occurred when we looked up
+  /// the file.
   ///
   /// \see SeenDirEntries
-  llvm::StringMap<FileEntry*, llvm::BumpPtrAllocator> SeenFileEntries;
+  llvm::StringMap<llvm::ErrorOr<FileEntry &>, llvm::BumpPtrAllocator>
+  SeenFileEntries;
 
   /// The canonical names of directories.
   llvm::DenseMap<const DirectoryEntry *, llvm::StringRef> CanonicalDirNames;
@@ -164,8 +168,9 @@ class FileManager : public RefCountedBase<FileManager> {
   // Caching.
   std::unique_ptr<FileSystemStatCache> StatCache;
 
-  bool getStatValue(StringRef Path, llvm::vfs::Status &Status, bool isFile,
-                    std::unique_ptr<llvm::vfs::File> *F);
+  std::error_code getStatValue(StringRef Path, llvm::vfs::Status &Status,
+                               bool isFile,
+                               std::unique_ptr<llvm::vfs::File> *F);
 
   /// Add all ancestors of the given path (pointing to either a file
   /// or a directory) as virtual directories.
@@ -198,24 +203,27 @@ public:
   /// Lookup, cache, and verify the specified directory (real or
   /// virtual).
   ///
-  /// This returns NULL if the directory doesn't exist.
+  /// This returns a \c std::error_code if there was an error reading the
+  /// directory. If there is no error, the DirectoryEntry is guaranteed to be
+  /// non-NULL.
   ///
   /// \param CacheFailure If true and the file does not exist, we'll cache
   /// the failure to find this file.
-  const DirectoryEntry *getDirectory(StringRef DirName,
-                                     bool CacheFailure = true);
+  llvm::ErrorOr<const DirectoryEntry *>
+  getDirectory(StringRef DirName, bool CacheFailure = true);
 
   /// Lookup, cache, and verify the specified file (real or
   /// virtual).
   ///
-  /// This returns NULL if the file doesn't exist.
+  /// This returns a \c std::error_code if there was an error loading the file.
+  /// If there is no error, the FileEntry is guaranteed to be non-NULL.
   ///
   /// \param OpenFile if true and the file exists, it will be opened.
   ///
   /// \param CacheFailure If true and the file does not exist, we'll cache
   /// the failure to find this file.
-  const FileEntry *getFile(StringRef Filename, bool OpenFile = false,
-                           bool CacheFailure = true);
+  llvm::ErrorOr<const FileEntry *>
+  getFile(StringRef Filename, bool OpenFile = false, bool CacheFailure = true);
 
   /// Returns the current file system options
   FileSystemOptions &getFileSystemOpts() { return FileSystemOpts; }
@@ -243,8 +251,9 @@ public:
   /// If the path is relative, it will be resolved against the WorkingDir of the
   /// FileManager's FileSystemOptions.
   ///
-  /// \returns false on success, true on error.
-  bool getNoncachedStatValue(StringRef Path, llvm::vfs::Status &Result);
+  /// \returns a \c std::error_code describing an error, if there was one
+  std::error_code getNoncachedStatValue(StringRef Path,
+                                        llvm::vfs::Status &Result);
 
   /// Remove the real file \p Entry from the cache.
   void invalidateCache(const FileEntry *Entry);
index b6a7fde..e582eac 100644 (file)
@@ -63,14 +63,14 @@ void FileManager::clearStatCache() { StatCache.reset(); }
 
 /// Retrieve the directory that the given file name resides in.
 /// Filename can point to either a real file or a virtual file.
-static const DirectoryEntry *getDirectoryFromFile(FileManager &FileMgr,
-                                                  StringRef Filename,
-                                                  bool CacheFailure) {
+static llvm::ErrorOr<const DirectoryEntry *>
+getDirectoryFromFile(FileManager &FileMgr, StringRef Filename,
+                     bool CacheFailure) {
   if (Filename.empty())
-    return nullptr;
+    return std::errc::no_such_file_or_directory;
 
   if (llvm::sys::path::is_separator(Filename[Filename.size() - 1]))
-    return nullptr; // If Filename is a directory.
+    return std::errc::is_a_directory;
 
   StringRef DirName = llvm::sys::path::parent_path(Filename);
   // Use the current directory if file has no path component.
@@ -87,7 +87,8 @@ void FileManager::addAncestorsAsVirtualDirs(StringRef Path) {
   if (DirName.empty())
     DirName = ".";
 
-  auto &NamedDirEnt = *SeenDirEntries.insert({DirName, nullptr}).first;
+  auto &NamedDirEnt = *SeenDirEntries.insert(
+        {DirName, std::errc::no_such_file_or_directory}).first;
 
   // When caching a virtual directory, we always cache its ancestors
   // at the same time.  Therefore, if DirName is already in the cache,
@@ -99,15 +100,23 @@ void FileManager::addAncestorsAsVirtualDirs(StringRef Path) {
   // Add the virtual directory to the cache.
   auto UDE = llvm::make_unique<DirectoryEntry>();
   UDE->Name = NamedDirEnt.first();
-  NamedDirEnt.second = UDE.get();
+  NamedDirEnt.second = *UDE.get();
   VirtualDirectoryEntries.push_back(std::move(UDE));
 
   // Recursively add the other ancestors.
   addAncestorsAsVirtualDirs(DirName);
 }
 
-const DirectoryEntry *FileManager::getDirectory(StringRef DirName,
-                                                bool CacheFailure) {
+/// Converts a llvm::ErrorOr<T &> to an llvm::ErrorOr<T *> by promoting
+/// the address of the inner reference to a pointer or by propagating the error.
+template <typename T>
+static llvm::ErrorOr<T *> promoteInnerReference(llvm::ErrorOr<T &> value) {
+  if (value) return &*value;
+  return value.getError();
+}
+
+llvm::ErrorOr<const DirectoryEntry *>
+FileManager::getDirectory(StringRef DirName, bool CacheFailure) {
   // stat doesn't like trailing separators except for root directory.
   // At least, on Win32 MSVCRT, stat() cannot strip trailing '/'.
   // (though it can strip '\\')
@@ -130,9 +139,10 @@ const DirectoryEntry *FileManager::getDirectory(StringRef DirName,
 
   // See if there was already an entry in the map.  Note that the map
   // contains both virtual and real directories.
-  auto SeenDirInsertResult = SeenDirEntries.insert({DirName, nullptr});
+  auto SeenDirInsertResult =
+      SeenDirEntries.insert({DirName, std::errc::no_such_file_or_directory});
   if (!SeenDirInsertResult.second)
-    return SeenDirInsertResult.first->second;
+    return promoteInnerReference(SeenDirInsertResult.first->second);
 
   // We've not seen this before. Fill it in.
   ++NumDirCacheMisses;
@@ -145,11 +155,15 @@ const DirectoryEntry *FileManager::getDirectory(StringRef DirName,
 
   // Check to see if the directory exists.
   llvm::vfs::Status Status;
-  if (getStatValue(InterndDirName, Status, false, nullptr /*directory lookup*/)) {
+  auto statError = getStatValue(InterndDirName, Status, false,
+                                nullptr /*directory lookup*/);
+  if (statError) {
     // There's no real directory at the given path.
-    if (!CacheFailure)
+    if (CacheFailure)
+      NamedDirEnt.second = statError;
+    else
       SeenDirEntries.erase(DirName);
-    return nullptr;
+    return statError;
   }
 
   // It exists.  See if we have already opened a directory with the
@@ -158,7 +172,7 @@ const DirectoryEntry *FileManager::getDirectory(StringRef DirName,
   // Windows).
   DirectoryEntry &UDE = UniqueRealDirs[Status.getUniqueID()];
 
-  NamedDirEnt.second = &UDE;
+  NamedDirEnt.second = UDE;
   if (UDE.getName().empty()) {
     // We don't have this directory yet, add it.  We use the string
     // key from the SeenDirEntries map as the string.
@@ -168,14 +182,15 @@ const DirectoryEntry *FileManager::getDirectory(StringRef DirName,
   return &UDE;
 }
 
-const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
-                                      bool CacheFailure) {
+llvm::ErrorOr<const FileEntry *>
+FileManager::getFile(StringRef Filename, bool openFile, bool CacheFailure) {
   ++NumFileLookups;
 
   // See if there is already an entry in the map.
-  auto SeenFileInsertResult = SeenFileEntries.insert({Filename, nullptr});
+  auto SeenFileInsertResult =
+      SeenFileEntries.insert({Filename, std::errc::no_such_file_or_directory});
   if (!SeenFileInsertResult.second)
-    return SeenFileInsertResult.first->second;
+    return promoteInnerReference(SeenFileInsertResult.first->second);
 
   // We've not seen this before. Fill it in.
   ++NumFileCacheMisses;
@@ -191,14 +206,16 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
   // subdirectory.  This will let us avoid having to waste time on known-to-fail
   // searches when we go to find sys/bar.h, because all the search directories
   // without a 'sys' subdir will get a cached failure result.
-  const DirectoryEntry *DirInfo = getDirectoryFromFile(*this, Filename,
-                                                       CacheFailure);
-  if (DirInfo == nullptr) { // Directory doesn't exist, file can't exist.
-    if (!CacheFailure)
+  auto DirInfoOrErr = getDirectoryFromFile(*this, Filename, CacheFailure);
+  if (!DirInfoOrErr) { // Directory doesn't exist, file can't exist.
+    if (CacheFailure)
+      NamedFileEnt.second = DirInfoOrErr.getError();
+    else
       SeenFileEntries.erase(Filename);
 
-    return nullptr;
+    return DirInfoOrErr.getError();
   }
+  const DirectoryEntry *DirInfo = *DirInfoOrErr;
 
   // FIXME: Use the directory info to prune this, before doing the stat syscall.
   // FIXME: This will reduce the # syscalls.
@@ -206,12 +223,16 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
   // Check to see if the file exists.
   std::unique_ptr<llvm::vfs::File> F;
   llvm::vfs::Status Status;
-  if (getStatValue(InterndFileName, Status, true, openFile ? &F : nullptr)) {
+  auto statError = getStatValue(InterndFileName, Status, true,
+                                openFile ? &F : nullptr);
+  if (statError) {
     // There's no real file at the given path.
-    if (!CacheFailure)
+    if (CacheFailure)
+      NamedFileEnt.second = statError;
+    else
       SeenFileEntries.erase(Filename);
 
-    return nullptr;
+    return statError;
   }
 
   assert((openFile || !F) && "undesired open file");
@@ -220,14 +241,14 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
   // This occurs when one dir is symlinked to another, for example.
   FileEntry &UFE = UniqueRealFiles[Status.getUniqueID()];
 
-  NamedFileEnt.second = &UFE;
+  NamedFileEnt.second = UFE;
 
   // If the name returned by getStatValue is different than Filename, re-intern
   // the name.
   if (Status.getName() != Filename) {
     auto &NamedFileEnt =
-      *SeenFileEntries.insert({Status.getName(), &UFE}).first;
-    assert(NamedFileEnt.second == &UFE &&
+      *SeenFileEntries.insert({Status.getName(), UFE}).first;
+    assert(&*NamedFileEnt.second == &UFE &&
            "filename from getStatValue() refers to wrong file");
     InterndFileName = NamedFileEnt.first().data();
   }
@@ -280,9 +301,10 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
   ++NumFileLookups;
 
   // See if there is already an entry in the map for an existing file.
-  auto &NamedFileEnt = *SeenFileEntries.insert({Filename, nullptr}).first;
+  auto &NamedFileEnt = *SeenFileEntries.insert(
+      {Filename, std::errc::no_such_file_or_directory}).first;
   if (NamedFileEnt.second)
-    return NamedFileEnt.second;
+    return &*NamedFileEnt.second;
 
   // We've not seen this before, or the file is cached as non-existent.
   ++NumFileCacheMisses;
@@ -292,15 +314,14 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
   // 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);
+  auto 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
   llvm::vfs::Status Status;
   const char *InterndFileName = NamedFileEnt.first().data();
-  if (getStatValue(InterndFileName, Status, true, nullptr) == 0) {
+  if (!getStatValue(InterndFileName, Status, true, nullptr)) {
     UFE = &UniqueRealFiles[Status.getUniqueID()];
     Status = llvm::vfs::Status(
       Status.getName(), Status.getUniqueID(),
@@ -308,7 +329,7 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
       Status.getUser(), Status.getGroup(), Size,
       Status.getType(), Status.getPermissions());
 
-    NamedFileEnt.second = UFE;
+    NamedFileEnt.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
@@ -326,13 +347,13 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
   } else {
     VirtualFileEntries.push_back(llvm::make_unique<FileEntry>());
     UFE = VirtualFileEntries.back().get();
-    NamedFileEnt.second = UFE;
+    NamedFileEnt.second = *UFE;
   }
 
   UFE->Name    = InterndFileName;
   UFE->Size    = Size;
   UFE->ModTime = ModificationTime;
-  UFE->Dir     = DirInfo;
+  UFE->Dir     = *DirInfo;
   UFE->UID     = NextFileUID++;
   UFE->IsValid = true;
   UFE->File.reset();
@@ -423,32 +444,33 @@ FileManager::getBufferForFile(StringRef Filename, bool isVolatile) {
 /// if the path points to a virtual file or does not exist, or returns
 /// false if it's an existent real file.  If FileDescriptor is NULL,
 /// do directory look-up instead of file look-up.
-bool FileManager::getStatValue(StringRef Path, llvm::vfs::Status &Status,
-                               bool isFile,
-                               std::unique_ptr<llvm::vfs::File> *F) {
+std::error_code
+FileManager::getStatValue(StringRef Path, llvm::vfs::Status &Status,
+                          bool isFile, std::unique_ptr<llvm::vfs::File> *F) {
   // FIXME: FileSystemOpts shouldn't be passed in here, all paths should be
   // absolute!
   if (FileSystemOpts.WorkingDir.empty())
-    return bool(FileSystemStatCache::get(Path, Status, isFile, F,
-                                         StatCache.get(), *FS));
+    return FileSystemStatCache::get(Path, Status, isFile, F,
+                                    StatCache.get(), *FS);
 
   SmallString<128> FilePath(Path);
   FixupRelativePath(FilePath);
 
-  return bool(FileSystemStatCache::get(FilePath.c_str(), Status, isFile, F,
-                                       StatCache.get(), *FS));
+  return FileSystemStatCache::get(FilePath.c_str(), Status, isFile, F,
+                                  StatCache.get(), *FS);
 }
 
-bool FileManager::getNoncachedStatValue(StringRef Path,
-                                        llvm::vfs::Status &Result) {
+std::error_code 
+FileManager::getNoncachedStatValue(StringRef Path,
+                                   llvm::vfs::Status &Result) {
   SmallString<128> FilePath(Path);
   FixupRelativePath(FilePath);
 
   llvm::ErrorOr<llvm::vfs::Status> S = FS->status(FilePath.c_str());
   if (!S)
-    return true;
+    return S.getError();
   Result = *S;
-  return false;
+  return std::error_code();
 }
 
 void FileManager::invalidateCache(const FileEntry *Entry) {
@@ -471,11 +493,13 @@ void FileManager::GetUniqueIDMapping(
   UIDToFiles.resize(NextFileUID);
 
   // Map file entries
-  for (llvm::StringMap<FileEntry*, llvm::BumpPtrAllocator>::const_iterator
+  for (llvm::StringMap<llvm::ErrorOr<FileEntry &>,
+                       llvm::BumpPtrAllocator>::const_iterator
          FE = SeenFileEntries.begin(), FEEnd = SeenFileEntries.end();
        FE != FEEnd; ++FE)
-    if (FE->getValue())
-      UIDToFiles[FE->getValue()->getUID()] = FE->getValue();
+    if (auto Entry = FE->getValue()) {
+      UIDToFiles[Entry->getUID()] = &*Entry;
+    }
 
   // Map virtual file entries
   for (const auto &VFE : VirtualFileEntries)
index 19e2180..3518325 100644 (file)
@@ -112,9 +112,9 @@ TEST_F(FileManagerTest, NoVirtualDirectoryExistsBeforeAVirtualFileIsAdded) {
   // by what's in the real file system.
   manager.setStatCache(llvm::make_unique<FakeStatCache>());
 
-  EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir/foo"));
-  EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir"));
-  EXPECT_EQ(nullptr, manager.getDirectory("virtual"));
+  ASSERT_FALSE(manager.getDirectory("virtual/dir/foo"));
+  ASSERT_FALSE(manager.getDirectory("virtual/dir"));
+  ASSERT_FALSE(manager.getDirectory("virtual"));
 }
 
 // When a virtual file is added, all of its ancestors should be created.
@@ -123,15 +123,15 @@ TEST_F(FileManagerTest, getVirtualFileCreatesDirectoryEntriesForAncestors) {
   manager.setStatCache(llvm::make_unique<FakeStatCache>());
 
   manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
-  EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir/foo"));
+  ASSERT_FALSE(manager.getDirectory("virtual/dir/foo"));
 
-  const DirectoryEntry *dir = manager.getDirectory("virtual/dir");
-  ASSERT_TRUE(dir != nullptr);
-  EXPECT_EQ("virtual/dir", dir->getName());
+  auto dir = manager.getDirectory("virtual/dir");
+  ASSERT_TRUE(dir);
+  EXPECT_EQ("virtual/dir", (*dir)->getName());
 
   dir = manager.getDirectory("virtual");
-  ASSERT_TRUE(dir != nullptr);
-  EXPECT_EQ("virtual", dir->getName());
+  ASSERT_TRUE(dir);
+  EXPECT_EQ("virtual", (*dir)->getName());
 }
 
 // getFile() returns non-NULL if a real file exists at the given path.
@@ -150,18 +150,18 @@ TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingRealFile) {
 
   manager.setStatCache(std::move(statCache));
 
-  const FileEntry *file = manager.getFile("/tmp/test");
-  ASSERT_TRUE(file != nullptr);
-  ASSERT_TRUE(file->isValid());
-  EXPECT_EQ("/tmp/test", file->getName());
+  auto file = manager.getFile("/tmp/test");
+  ASSERT_TRUE(file);
+  ASSERT_TRUE((*file)->isValid());
+  EXPECT_EQ("/tmp/test", (*file)->getName());
 
-  const DirectoryEntry *dir = file->getDir();
+  const DirectoryEntry *dir = (*file)->getDir();
   ASSERT_TRUE(dir != nullptr);
   EXPECT_EQ("/tmp", dir->getName());
 
 #ifdef _WIN32
   file = manager.getFile(FileName);
-  ASSERT_TRUE(file != NULL);
+  ASSERT_TRUE(file);
 
   dir = file->getDir();
   ASSERT_TRUE(dir != NULL);
@@ -175,12 +175,12 @@ TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingVirtualFile) {
   manager.setStatCache(llvm::make_unique<FakeStatCache>());
 
   manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
-  const FileEntry *file = manager.getFile("virtual/dir/bar.h");
-  ASSERT_TRUE(file != nullptr);
-  ASSERT_TRUE(file->isValid());
-  EXPECT_EQ("virtual/dir/bar.h", file->getName());
+  auto file = manager.getFile("virtual/dir/bar.h");
+  ASSERT_TRUE(file);
+  ASSERT_TRUE((*file)->isValid());
+  EXPECT_EQ("virtual/dir/bar.h", (*file)->getName());
 
-  const DirectoryEntry *dir = file->getDir();
+  const DirectoryEntry *dir = (*file)->getDir();
   ASSERT_TRUE(dir != nullptr);
   EXPECT_EQ("virtual/dir", dir->getName());
 }
@@ -196,13 +196,13 @@ TEST_F(FileManagerTest, getFileReturnsDifferentFileEntriesForDifferentFiles) {
   statCache->InjectFile("bar.cpp", 43);
   manager.setStatCache(std::move(statCache));
 
-  const FileEntry *fileFoo = manager.getFile("foo.cpp");
-  const FileEntry *fileBar = manager.getFile("bar.cpp");
-  ASSERT_TRUE(fileFoo != nullptr);
-  ASSERT_TRUE(fileFoo->isValid());
-  ASSERT_TRUE(fileBar != nullptr);
-  ASSERT_TRUE(fileBar->isValid());
-  EXPECT_NE(fileFoo, fileBar);
+  auto fileFoo = manager.getFile("foo.cpp");
+  auto fileBar = manager.getFile("bar.cpp");
+  ASSERT_TRUE(fileFoo);
+  ASSERT_TRUE((*fileFoo)->isValid());
+  ASSERT_TRUE(fileBar);
+  ASSERT_TRUE((*fileBar)->isValid());
+  EXPECT_NE(*fileFoo, *fileBar);
 }
 
 // getFile() returns NULL if neither a real file nor a virtual file
@@ -217,8 +217,8 @@ TEST_F(FileManagerTest, getFileReturnsNULLForNonexistentFile) {
   // Create a virtual bar.cpp file.
   manager.getVirtualFile("bar.cpp", 200, 0);
 
-  const FileEntry *file = manager.getFile("xyz.txt");
-  EXPECT_EQ(nullptr, file);
+  auto file = manager.getFile("xyz.txt");
+  ASSERT_FALSE(file);
 }
 
 // The following tests apply to Unix-like system only.
@@ -234,7 +234,11 @@ TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedRealFiles) {
   statCache->InjectFile("abc/bar.cpp", 42);
   manager.setStatCache(std::move(statCache));
 
-  EXPECT_EQ(manager.getFile("abc/foo.cpp"), manager.getFile("abc/bar.cpp"));
+  auto f1 = manager.getFile("abc/foo.cpp");
+  auto f2 = manager.getFile("abc/bar.cpp");
+
+  EXPECT_EQ(f1 ? *f1 : nullptr,
+            f2 ? *f2 : nullptr);
 }
 
 // getFile() returns the same FileEntry for virtual files that have
@@ -250,7 +254,11 @@ TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedVirtualFiles) {
   ASSERT_TRUE(manager.getVirtualFile("abc/foo.cpp", 100, 0)->isValid());
   ASSERT_TRUE(manager.getVirtualFile("abc/bar.cpp", 200, 0)->isValid());
 
-  EXPECT_EQ(manager.getFile("abc/foo.cpp"), manager.getFile("abc/bar.cpp"));
+  auto f1 = manager.getFile("abc/foo.cpp");
+  auto f2 = manager.getFile("abc/bar.cpp");
+
+  EXPECT_EQ(f1 ? *f1 : nullptr,
+            f2 ? *f2 : nullptr);
 }
 
 // getFile() Should return the same entry as getVirtualFile if the file actually
@@ -273,15 +281,15 @@ TEST_F(FileManagerTest, getVirtualFileWithDifferentName) {
   EXPECT_EQ(123, file1->getSize());
 
   // Lookup the virtual file with a different name:
-  const FileEntry *file2 = manager.getFile("c:/tmp/test", 100, 1);
-  ASSERT_TRUE(file2 != nullptr);
-  ASSERT_TRUE(file2->isValid());
+  auto file2 = manager.getFile("c:/tmp/test", 100, 1);
+  ASSERT_TRUE(file2);
+  ASSERT_TRUE((*file2)->isValid());
   // Check that it's the same UFE:
-  EXPECT_EQ(file1, file2);
-  EXPECT_EQ(43U, file2->getUniqueID().getFile());
+  EXPECT_EQ(file1, *file2);
+  EXPECT_EQ(43U, (*file2)->getUniqueID().getFile());
   // Check that the contents of the UFE are not overwritten by the entry in the
   // filesystem:
-  EXPECT_EQ(123, file2->getSize());
+  EXPECT_EQ(123, (*file2)->getSize());
 }
 
 #endif  // !_WIN32