usaxena95 created this revision. Herald added subscribers: cfe-commits, mgrang.
Some files are only Statted by Clang and not read. Clang mostly uses them for checking the existence of some files and in rare cases uses the value of the Status to proceed further (for example while loading module files, it checks the sizes of some files). This change adds support to represent files that are supposed to be only statted by Clang. Repository: rC Clang https://reviews.llvm.org/D52620 Files: include/clang/Basic/VirtualFileSystem.h lib/Basic/VirtualFileSystem.cpp unittests/Basic/VirtualFileSystemTest.cpp
Index: unittests/Basic/VirtualFileSystemTest.cpp =================================================================== --- unittests/Basic/VirtualFileSystemTest.cpp +++ unittests/Basic/VirtualFileSystemTest.cpp @@ -553,8 +553,8 @@ for (DirIter E; !EC && I != E; I.increment(EC)) InputToCheck.push_back(I->path()); - llvm::sort(InputToCheck); - llvm::sort(Expected); + llvm::sort(InputToCheck.begin(), InputToCheck.end()); + llvm::sort(Expected.begin(), Expected.end()); EXPECT_EQ(InputToCheck.size(), Expected.size()); unsigned LastElt = std::min(InputToCheck.size(), Expected.size()); @@ -1063,15 +1063,50 @@ TEST_F(InMemoryFileSystemTest, RecursiveIterationWithHardLink) { std::error_code EC; FS.addFile("/a/b", 0, MemoryBuffer::getMemBuffer("content string")); + FS.addStatOnlyFile("/a/d", 0, 10); EXPECT_TRUE(FS.addHardLink("/c/d", "/a/b")); auto I = vfs::recursive_directory_iterator(FS, "/", EC); ASSERT_FALSE(EC); std::vector<std::string> Nodes; for (auto E = vfs::recursive_directory_iterator(); !EC && I != E; I.increment(EC)) { Nodes.push_back(getPosixPath(I->path())); } - EXPECT_THAT(Nodes, testing::UnorderedElementsAre("/a", "/a/b", "/c", "/c/d")); + EXPECT_THAT(Nodes, testing::UnorderedElementsAre("/a", "/a/d", "/a/b", + "/c", "/c/d")); +} + +TEST_F(InMemoryFileSystemTest, AddStatOnlyFileWithCorrectSize) { + FS.addStatOnlyFile("/a/b", 0, 10); + auto Stat = FS.status("/a/b"); + EXPECT_EQ(Stat.get().getSize(), 10); +} + +TEST_F(InMemoryFileSystemTest, AddMultipleFilesUnderStatOnlyFile) { + FS.addStatOnlyFile("/a/b", 0, 10); + EXPECT_FALSE(FS.addStatOnlyFile("/a/b", 0, 11)); + EXPECT_TRUE(FS.addStatOnlyFile("/a/b", 0, 10)); + EXPECT_FALSE(FS.addFile("/a/b", 0, MemoryBuffer::getMemBuffer("content"))); +} + +TEST_F(InMemoryFileSystemTest, DeathWhenBufferRequestedForStatOnlyFile) { + FS.addStatOnlyFile("/a/b", 0, 10); + EXPECT_DEATH(FS.getBufferForFile("/a/b"), + "Cannot get buffer for StatOnlyFile"); +} + +TEST_F(InMemoryFileSystemTest, AddHardLinksToStatOnlyFile) { + FS.addStatOnlyFile("/target", 0, 10); + FS.addHardLink("/link1", "/target"); + FS.addHardLink("/link2", "/link1"); + EXPECT_THAT("/link1", IsHardLinkTo(&FS, "/target")); + EXPECT_THAT("/link2", IsHardLinkTo(&FS, "/target")); +} + +TEST_F(InMemoryFileSystemTest, AddFileUnderHardLinkToStatOnlyFile) { + FS.addStatOnlyFile("/target", 0, 10); + FS.addHardLink("/link", "/target"); + EXPECT_FALSE(FS.addFile("/link", 0, MemoryBuffer::getMemBuffer("content"))); } // NOTE: in the tests below, we use '//root/' as our root directory, since it is Index: lib/Basic/VirtualFileSystem.cpp =================================================================== --- lib/Basic/VirtualFileSystem.cpp +++ lib/Basic/VirtualFileSystem.cpp @@ -492,20 +492,26 @@ class InMemoryFile : public InMemoryNode { Status Stat; std::unique_ptr<llvm::MemoryBuffer> Buffer; + bool StatOnlyFile; public: - InMemoryFile(Status Stat, std::unique_ptr<llvm::MemoryBuffer> Buffer) + InMemoryFile(Status Stat, std::unique_ptr<llvm::MemoryBuffer> Buffer, + bool StatOnlyFile) : InMemoryNode(Stat.getName(), IME_File), Stat(std::move(Stat)), - Buffer(std::move(Buffer)) {} + Buffer(std::move(Buffer)), StatOnlyFile(StatOnlyFile) {} /// Return the \p Status for this node. \p RequestedName should be the name /// through which the caller referred to this node. It will override /// \p Status::Name in the return value, to mimic the behavior of \p RealFile. Status getStatus(StringRef RequestedName) const { return Status::copyWithNewName(Stat, RequestedName); } - llvm::MemoryBuffer *getBuffer() const { return Buffer.get(); } + llvm::MemoryBuffer *getBuffer() const { + assert(!StatOnlyFile && "Cannot get buffer for StatOnlyFile"); + return Buffer.get(); + } + bool IsStatOnlyFile() const { return StatOnlyFile; } std::string toString(unsigned Indent) const override { return (std::string(Indent, ' ') + Stat.getName() + "\n").str(); } @@ -640,7 +646,8 @@ Optional<uint32_t> Group, Optional<llvm::sys::fs::file_type> Type, Optional<llvm::sys::fs::perms> Perms, - const detail::InMemoryFile *HardLinkTarget) { + const detail::InMemoryFile *HardLinkTarget, + Optional<size_t> StatOnlyFileSize) { SmallString<128> Path; P.toVector(Path); @@ -661,7 +668,11 @@ const auto ResolvedGroup = Group.getValueOr(0); const auto ResolvedType = Type.getValueOr(sys::fs::file_type::regular_file); const auto ResolvedPerms = Perms.getValueOr(sys::fs::all_all); + assert(!(StatOnlyFileSize.hasValue() && Buffer) && + "StatOnlyFile cannot have a buffer"); assert(!(HardLinkTarget && Buffer) && "HardLink cannot have a buffer"); + const auto ResolvedFileSize = + StatOnlyFileSize.getValueOr(Buffer ? Buffer->getBufferSize() : 0); // Any intermediate directories we create should be accessible by // the owner, even if Perms says otherwise for the final path. const auto NewDirectoryPerms = ResolvedPerms | sys::fs::owner_all; @@ -679,13 +690,15 @@ // Create a new file or directory. Status Stat(P.str(), getNextVirtualUniqueID(), llvm::sys::toTimePoint(ModificationTime), ResolvedUser, - ResolvedGroup, Buffer->getBufferSize(), ResolvedType, + ResolvedGroup, ResolvedFileSize, ResolvedType, ResolvedPerms); if (ResolvedType == sys::fs::file_type::directory_file) { Child.reset(new detail::InMemoryDirectory(std::move(Stat))); } else { - Child.reset( - new detail::InMemoryFile(std::move(Stat), std::move(Buffer))); + // Create a new file. + Child.reset(new detail::InMemoryFile(std::move(Stat), + std::move(Buffer), + StatOnlyFileSize.hasValue())); } } Dir->addChild(Name, std::move(Child)); @@ -715,16 +728,33 @@ return false; // Return false only if the new file is different from the existing one. + const detail::InMemoryFile* File; if (auto Link = dyn_cast<detail::InMemoryHardLink>(Node)) { - return Link->getResolvedFile().getBuffer()->getBuffer() == - Buffer->getBuffer(); + File = &Link->getResolvedFile(); + } else { + File = cast<detail::InMemoryFile>(Node); + } + if (File->IsStatOnlyFile() != StatOnlyFileSize.hasValue()) { + return false; } - return cast<detail::InMemoryFile>(Node)->getBuffer()->getBuffer() == - Buffer->getBuffer(); + if (StatOnlyFileSize.hasValue()) { + return File->getStatus(Path).getSize() == StatOnlyFileSize.getValue(); + } + return File->getBuffer()->getBuffer() == Buffer->getBuffer(); } } } +bool InMemoryFileSystem::addStatOnlyFile( + const Twine &Path, time_t ModificationTime, size_t Size, + Optional<uint32_t> User, Optional<uint32_t> Group, + Optional<llvm::sys::fs::file_type> Type, + Optional<llvm::sys::fs::perms> Perms) { + return addFile(Path, ModificationTime, + /*Buffer=*/nullptr, User, Group, Type, Perms, + /*HardLinkTarget=*/nullptr, Size); +} + bool InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime, std::unique_ptr<llvm::MemoryBuffer> Buffer, Optional<uint32_t> User, @@ -2064,7 +2094,8 @@ } void YAMLVFSWriter::write(llvm::raw_ostream &OS) { - llvm::sort(Mappings, [](const YAMLVFSEntry &LHS, const YAMLVFSEntry &RHS) { + llvm::sort(Mappings.begin(), Mappings.end(), + [](const YAMLVFSEntry &LHS, const YAMLVFSEntry &RHS) { return LHS.VPath < RHS.VPath; }); Index: include/clang/Basic/VirtualFileSystem.h =================================================================== --- include/clang/Basic/VirtualFileSystem.h +++ include/clang/Basic/VirtualFileSystem.h @@ -391,7 +391,8 @@ Optional<uint32_t> User, Optional<uint32_t> Group, Optional<llvm::sys::fs::file_type> Type, Optional<llvm::sys::fs::perms> Perms, - const detail::InMemoryFile *HardLinkTarget); + const detail::InMemoryFile *HardLinkTarget, + Optional<size_t> StatOnlyFileSize=None); public: explicit InMemoryFileSystem(bool UseNormalizedPaths = true); @@ -409,6 +410,16 @@ Optional<llvm::sys::fs::file_type> Type = None, Optional<llvm::sys::fs::perms> Perms = None); + // Add a file with the given size but with no contents. The buffer of this + // file must never be requested. + /// \return true if the file successfully added, false if the file already + /// exists in the file system with different size or is not a StatOnlyFile. + bool addStatOnlyFile(const Twine &Path, time_t ModificationTime, + size_t Size, Optional<uint32_t> User = None, + Optional<uint32_t> Group = None, + Optional<llvm::sys::fs::file_type> Type = None, + Optional<llvm::sys::fs::perms> Perms = None); + /// Add a hard link to a file. /// Here hard links are not intended to be fully equivalent to the classical /// filesystem. Both the hard link and the file share the same buffer and
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits