Skip to content

Commit

Permalink
[clang] Fill RealPathName for virtual files.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kadircet committed Nov 30, 2018
1 parent 5399552 commit e9870c0
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 17 deletions.
7 changes: 7 additions & 0 deletions clang/include/clang/Basic/FileManager.h
Expand Up @@ -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;
Expand Down Expand Up @@ -173,6 +177,9 @@ class FileManager : public RefCountedBase<FileManager> {
/// 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<llvm::vfs::FileSystem> FS = nullptr);
Expand Down
24 changes: 14 additions & 10 deletions clang/lib/Basic/FileManager.cpp
Expand Up @@ -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!");
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -438,6 +431,17 @@ bool FileManager::makeAbsolutePath(SmallVectorImpl<char> &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<std::unique_ptr<llvm::MemoryBuffer>>
FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile,
bool ShouldCloseOpenFile) {
Expand Down
25 changes: 18 additions & 7 deletions clang/unittests/Basic/FileManagerTest.cpp
Expand Up @@ -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.
Expand Down Expand Up @@ -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<FakeStatCache>();
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

0 comments on commit e9870c0

Please sign in to comment.