From e9870c0c9145a2b261054348c7979333b8ed6655 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya Date: Fri, 30 Nov 2018 17:10:11 +0000 Subject: [PATCH] [clang] Fill RealPathName for virtual files. Summary: Absolute path information for virtual files were missing even if we have already stat'd the files. This patch puts that information for virtual files that can succesffully be stat'd. Reviewers: ilya-biryukov Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D55054 llvm-svn: 348006 --- clang/include/clang/Basic/FileManager.h | 7 +++++++ clang/lib/Basic/FileManager.cpp | 24 +++++++++++++--------- clang/unittests/Basic/FileManagerTest.cpp | 25 ++++++++++++++++------- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h index 66236c602f7a6..8e021f41c13da 100644 --- a/clang/include/clang/Basic/FileManager.h +++ b/clang/include/clang/Basic/FileManager.h @@ -104,6 +104,10 @@ class FileEntry { void closeFile() const { File.reset(); // rely on destructor to close File } + + // Only for use in tests to see if deferred opens are happening, rather than + // relying on RealPathName being empty. + bool isOpenForTests() const { return File != nullptr; } }; struct FileData; @@ -173,6 +177,9 @@ class FileManager : public RefCountedBase { /// or a directory) as virtual directories. void addAncestorsAsVirtualDirs(StringRef Path); + /// Fills the RealPathName in file entry. + void fillRealPathName(FileEntry *UFE, llvm::StringRef FileName); + public: FileManager(const FileSystemOptions &FileSystemOpts, IntrusiveRefCntPtr FS = nullptr); diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index 21cb92d51c266..455d25c100a47 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -293,16 +293,8 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile, // If we opened the file for the first time, record the resulting info. // Do this even if the cache entry was valid, maybe we didn't previously open. if (F && !UFE.File) { - if (auto PathName = F->getName()) { - llvm::SmallString<128> AbsPath(*PathName); - // This is not the same as `VFS::getRealPath()`, which resolves symlinks - // but can be very expensive on real file systems. - // FIXME: the semantic of RealPathName is unclear, and the name might be - // misleading. We need to clean up the interface here. - makeAbsolutePath(AbsPath); - llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true); - UFE.RealPathName = AbsPath.str(); - } + if (auto PathName = F->getName()) + fillRealPathName(&UFE, *PathName); UFE.File = std::move(F); assert(!UFE.DeferredOpen && "we just opened it!"); } @@ -395,6 +387,7 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size, UFE->UniqueID = Data.UniqueID; UFE->IsNamedPipe = Data.IsNamedPipe; UFE->InPCH = Data.InPCH; + fillRealPathName(UFE, Data.Name); } if (!UFE) { @@ -438,6 +431,17 @@ bool FileManager::makeAbsolutePath(SmallVectorImpl &Path) const { return Changed; } +void FileManager::fillRealPathName(FileEntry *UFE, llvm::StringRef FileName) { + llvm::SmallString<128> AbsPath(FileName); + // This is not the same as `VFS::getRealPath()`, which resolves symlinks + // but can be very expensive on real file systems. + // FIXME: the semantic of RealPathName is unclear, and the name might be + // misleading. We need to clean up the interface here. + makeAbsolutePath(AbsPath); + llvm::sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true); + UFE->RealPathName = AbsPath.str(); +} + llvm::ErrorOr> FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile, bool ShouldCloseOpenFile) { diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp index 7e1e37f3c4c03..78c99909b7615 100644 --- a/clang/unittests/Basic/FileManagerTest.cpp +++ b/clang/unittests/Basic/FileManagerTest.cpp @@ -235,22 +235,18 @@ TEST_F(FileManagerTest, getFileDefersOpen) { ASSERT_TRUE(file != nullptr); ASSERT_TRUE(file->isValid()); // "real path name" reveals whether the file was actually opened. - EXPECT_EQ("", file->tryGetRealPathName()); + EXPECT_FALSE(file->isOpenForTests()); file = manager.getFile("/tmp/test", /*OpenFile=*/true); ASSERT_TRUE(file != nullptr); ASSERT_TRUE(file->isValid()); -#ifdef _WIN32 - EXPECT_EQ("/tmp\\test", file->tryGetRealPathName()); -#else - EXPECT_EQ("/tmp/test", file->tryGetRealPathName()); -#endif + EXPECT_TRUE(file->isOpenForTests()); // However we should never try to open a file previously opened as virtual. ASSERT_TRUE(manager.getVirtualFile("/tmp/testv", 5, 0)); ASSERT_TRUE(manager.getFile("/tmp/testv", /*OpenFile=*/false)); file = manager.getFile("/tmp/testv", /*OpenFile=*/true); - EXPECT_EQ("", file->tryGetRealPathName()); + EXPECT_FALSE(file->isOpenForTests()); } // The following tests apply to Unix-like system only. @@ -353,4 +349,19 @@ TEST_F(FileManagerTest, makeAbsoluteUsesVFS) { EXPECT_EQ(Path, ExpectedResult); } +// getVirtualFile should always fill the real path. +TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) { + // Inject fake files into the file system. + auto statCache = llvm::make_unique(); + statCache->InjectDirectory("/tmp", 42); + statCache->InjectFile("/tmp/test", 43); + manager.addStatCache(std::move(statCache)); + + // Check for real path. + const FileEntry *file = manager.getVirtualFile("/tmp/test", 123, 1); + ASSERT_TRUE(file != nullptr); + ASSERT_TRUE(file->isValid()); + EXPECT_EQ(file->tryGetRealPathName(), "/tmp/test"); +} + } // anonymous namespace