diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h index f38664ab2949..27bfb5fb3589 100644 --- a/clang/include/clang/Basic/FileManager.h +++ b/clang/include/clang/Basic/FileManager.h @@ -132,20 +132,24 @@ class FileManager : public RefCountedBase { SmallVector, 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 SeenDirEntries; + llvm::StringMap, 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 SeenFileEntries; + llvm::StringMap, llvm::BumpPtrAllocator> + SeenFileEntries; /// The canonical names of directories. llvm::DenseMap CanonicalDirNames; @@ -164,8 +168,9 @@ class FileManager : public RefCountedBase { // Caching. std::unique_ptr StatCache; - bool getStatValue(StringRef Path, llvm::vfs::Status &Status, bool isFile, - std::unique_ptr *F); + std::error_code getStatValue(StringRef Path, llvm::vfs::Status &Status, + bool isFile, + std::unique_ptr *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 + 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 + 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); diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index b6a7fde09f35..e582eac6626f 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -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 +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(); 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 to an llvm::ErrorOr by promoting +/// the address of the inner reference to a pointer or by propagating the error. +template +static llvm::ErrorOr promoteInnerReference(llvm::ErrorOr value) { + if (value) return &*value; + return value.getError(); +} + +llvm::ErrorOr +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 +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 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()); 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 *F) { +std::error_code +FileManager::getStatValue(StringRef Path, llvm::vfs::Status &Status, + bool isFile, std::unique_ptr *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 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::const_iterator + for (llvm::StringMap, + 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) diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp index 19e2180d3ff6..3518325083f0 100644 --- a/clang/unittests/Basic/FileManagerTest.cpp +++ b/clang/unittests/Basic/FileManagerTest.cpp @@ -112,9 +112,9 @@ TEST_F(FileManagerTest, NoVirtualDirectoryExistsBeforeAVirtualFileIsAdded) { // by what's in the real file system. manager.setStatCache(llvm::make_unique()); - 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()); 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()); 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