Hello Simon, This commit added broken tests to one of our builders: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/18386/steps/test/logs/stdio . . . Failing Tests (4): Clang :: PCH/case-insensitive-include.c Clang-Unit :: Basic/./BasicTests.exe/InMemoryFileSystemTest.DirectoryIteration Clang-Unit :: Basic/./BasicTests.exe/InMemoryFileSystemTest.StatusName LLVM :: MC/ELF/debug-prefix-map.s
Please have a look? The builder was already red and did not send notifications. Thanks Galina On Wed, Jul 11, 2018 at 7:08 AM, Simon Marchi via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: simark > Date: Wed Jul 11 07:08:17 2018 > New Revision: 336807 > > URL: http://llvm.org/viewvc/llvm-project?rev=336807&view=rev > Log: > [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the > requested name > > Summary: > InMemoryFileSystem::status behaves differently than > RealFileSystem::status. The Name contained in the Status returned by > RealFileSystem::status will be the path as requested by the caller, > whereas InMemoryFileSystem::status returns the normalized path. > > For example, when requested the status for "../src/first.h", > RealFileSystem returns a Status with "../src/first.h" as the Name. > InMemoryFileSystem returns "/absolute/path/to/src/first.h". > > The reason for this change is that I want to make a unit test in the > clangd testsuite (where we use an InMemoryFileSystem) to reproduce a > bug I get with the clangd program (where a RealFileSystem is used). > This difference in behavior "hides" the bug in the unit test version. > > In general, I guess it's good if InMemoryFileSystem works as much as > possible like RealFileSystem. > > Doing so made the FileEntry::RealPathName value (assigned in > FileManager::getFile) wrong when using the InMemoryFileSystem. That's > because it assumes that vfs::File::getName will always return the real > path. I changed to to use FileSystem::getRealPath instead. > > Subscribers: ilya-biryukov, ioeric, cfe-commits > > Differential Revision: https://reviews.llvm.org/D48903 > > Modified: > cfe/trunk/lib/Basic/FileManager.cpp > cfe/trunk/lib/Basic/VirtualFileSystem.cpp > cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp > cfe/trunk/unittests/Driver/ToolChainTest.cpp > > Modified: cfe/trunk/lib/Basic/FileManager.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/ > FileManager.cpp?rev=336807&r1=336806&r2=336807&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/Basic/FileManager.cpp (original) > +++ cfe/trunk/lib/Basic/FileManager.cpp Wed Jul 11 07:08:17 2018 > @@ -315,9 +315,11 @@ const FileEntry *FileManager::getFile(St > UFE.InPCH = Data.InPCH; > UFE.File = std::move(F); > UFE.IsValid = true; > - if (UFE.File) > - if (auto RealPathName = UFE.File->getName()) > - UFE.RealPathName = *RealPathName; > + > + SmallString<128> RealPathName; > + if (!FS->getRealPath(InterndFileName, RealPathName)) > + UFE.RealPathName = RealPathName.str(); > + > return &UFE; > } > > > Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/ > VirtualFileSystem.cpp?rev=336807&r1=336806&r2=336807&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original) > +++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Wed Jul 11 07:08:17 2018 > @@ -471,15 +471,33 @@ enum InMemoryNodeKind { IME_File, IME_Di > /// The in memory file system is a tree of Nodes. Every node can either > be a > /// file or a directory. > class InMemoryNode { > - Status Stat; > InMemoryNodeKind Kind; > + Status Stat; > + > +protected: > + /// Return Stat. This should only be used for internal/debugging use. > When > + /// clients wants the Status of this node, they should use > + /// \p getStatus(StringRef). > + const Status& getStatus() const { > + return Stat; > + } > > public: > InMemoryNode(Status Stat, InMemoryNodeKind Kind) > - : Stat(std::move(Stat)), Kind(Kind) {} > + : Kind(Kind), Stat(std::move(Stat)) {} > virtual ~InMemoryNode() = default; > > - const Status &getStatus() const { return Stat; } > + /// 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); > + } > + > + /// Get the filename of this node (the name without the directory part). > + StringRef getFileName() const { > + return llvm::sys::path::filename(Stat.getName()); > + } > InMemoryNodeKind getKind() const { return Kind; } > virtual std::string toString(unsigned Indent) const = 0; > }; > @@ -504,14 +522,22 @@ public: > } > }; > > -/// Adapt a InMemoryFile for VFS' File interface. > +/// Adapt a InMemoryFile for VFS' File interface. The goal is to make > +/// \p InMemoryFileAdaptor mimic as much as possible the behavior of > +/// \p RealFile. > class InMemoryFileAdaptor : public File { > InMemoryFile &Node; > > + /// The name to use when returning a Status for this file. > + std::string RequestedName; > + > public: > - explicit InMemoryFileAdaptor(InMemoryFile &Node) : Node(Node) {} > + explicit InMemoryFileAdaptor(InMemoryFile &Node, std::string > RequestedName) > + : Node(Node), RequestedName(std::move(RequestedName)) {} > > - llvm::ErrorOr<Status> status() override { return Node.getStatus(); } > + llvm::ErrorOr<Status> status() override { > + return Node.getStatus(RequestedName); > + } > > llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> > getBuffer(const Twine &Name, int64_t FileSize, bool > RequiresNullTerminator, > @@ -711,7 +737,7 @@ lookupInMemoryNode(const InMemoryFileSys > llvm::ErrorOr<Status> InMemoryFileSystem::status(const Twine &Path) { > auto Node = lookupInMemoryNode(*this, Root.get(), Path); > if (Node) > - return (*Node)->getStatus(); > + return (*Node)->getStatus(Path.str()); > return Node.getError(); > } > > @@ -724,7 +750,8 @@ InMemoryFileSystem::openFileForRead(cons > // When we have a file provide a heap-allocated wrapper for the memory > buffer > // to match the ownership semantics for File. > if (auto *F = dyn_cast<detail::InMemoryFile>(*Node)) > - return std::unique_ptr<File>(new detail::InMemoryFileAdaptor(*F)); > + return std::unique_ptr<File>( > + new detail::InMemoryFileAdaptor(*F, Path.str())); > > // FIXME: errc::not_a_file? > return make_error_code(llvm::errc::invalid_argument); > @@ -736,21 +763,33 @@ namespace { > class InMemoryDirIterator : public clang::vfs::detail::DirIterImpl { > detail::InMemoryDirectory::const_iterator I; > detail::InMemoryDirectory::const_iterator E; > + std::string RequestedDirName; > + > + void setCurrentEntry() { > + if (I != E) { > + SmallString<256> Path(RequestedDirName); > + llvm::sys::path::append(Path, I->second->getFileName()); > + CurrentEntry = I->second->getStatus(Path.str()); > + } else { > + // When we're at the end, make CurrentEntry invalid and DirIterImpl > will > + // do the rest. > + CurrentEntry = Status(); > + } > + } > > public: > InMemoryDirIterator() = default; > > - explicit InMemoryDirIterator(detail::InMemoryDirectory &Dir) > - : I(Dir.begin()), E(Dir.end()) { > - if (I != E) > - CurrentEntry = I->second->getStatus(); > + explicit InMemoryDirIterator(detail::InMemoryDirectory &Dir, > + std::string RequestedDirName) > + : I(Dir.begin()), E(Dir.end()), > + RequestedDirName(std::move(RequestedDirName)) { > + setCurrentEntry(); > } > > std::error_code increment() override { > ++I; > - // When we're at the end, make CurrentEntry invalid and DirIterImpl > will do > - // the rest. > - CurrentEntry = I != E ? I->second->getStatus() : Status(); > + setCurrentEntry(); > return {}; > } > }; > @@ -766,7 +805,8 @@ directory_iterator InMemoryFileSystem::d > } > > if (auto *DirNode = dyn_cast<detail::InMemoryDirectory>(*Node)) > - return directory_iterator(std::make_shared<InMemoryDirIterator>(* > DirNode)); > + return directory_iterator( > + std::make_shared<InMemoryDirIterator>(*DirNode, Dir.str())); > > EC = make_error_code(llvm::errc::not_a_directory); > return directory_iterator(std::make_shared<InMemoryDirIterator>()); > > Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ > Basic/VirtualFileSystemTest.cpp?rev=336807&r1=336806&r2=336807&view=diff > ============================================================ > ================== > --- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original) > +++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Wed Jul 11 > 07:08:17 2018 > @@ -794,7 +794,7 @@ TEST_F(InMemoryFileSystemTest, WorkingDi > > auto Stat = FS.status("/b/c"); > ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << > FS.toString(); > - ASSERT_EQ("c", Stat->getName()); > + ASSERT_EQ("/b/c", Stat->getName()); > ASSERT_EQ("/b", *FS.getCurrentWorkingDirectory()); > > Stat = FS.status("c"); > @@ -919,6 +919,37 @@ TEST_F(InMemoryFileSystemTest, AddDirect > ASSERT_TRUE(Stat->isRegularFile()); > } > > +// Test that the name returned by status() is in the same form as the > path that > +// was requested (to match the behavior of RealFileSystem). > +TEST_F(InMemoryFileSystemTest, StatusName) { > + NormalizedFS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"), > + /*User=*/None, > + /*Group=*/None, sys::fs::file_type::regular_file); > + NormalizedFS.setCurrentWorkingDirectory("/a/b"); > + > + // Access using InMemoryFileSystem::status. > + auto Stat = NormalizedFS.status("../b/c"); > + ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" > + << NormalizedFS.toString(); > + ASSERT_TRUE(Stat->isRegularFile()); > + ASSERT_EQ("../b/c", Stat->getName()); > + > + // Access using InMemoryFileAdaptor::status. > + auto File = NormalizedFS.openFileForRead("../b/c"); > + ASSERT_FALSE(File.getError()) << File.getError() << "\n" > + << NormalizedFS.toString(); > + Stat = (*File)->status(); > + ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" > + << NormalizedFS.toString(); > + ASSERT_TRUE(Stat->isRegularFile()); > + ASSERT_EQ("../b/c", Stat->getName()); > + > + // Access using a directory iterator. > + std::error_code EC; > + clang::vfs::directory_iterator It = NormalizedFS.dir_begin("../b", EC); > + ASSERT_EQ("../b/c", It->getName()); > +} > + > // NOTE: in the tests below, we use '//root/' as our root directory, > since it is > // a legal *absolute* path on Windows as well as *nix. > class VFSFromYAMLTest : public ::testing::Test { > > Modified: cfe/trunk/unittests/Driver/ToolChainTest.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ > Driver/ToolChainTest.cpp?rev=336807&r1=336806&r2=336807&view=diff > ============================================================ > ================== > --- cfe/trunk/unittests/Driver/ToolChainTest.cpp (original) > +++ cfe/trunk/unittests/Driver/ToolChainTest.cpp Wed Jul 11 07:08:17 2018 > @@ -113,7 +113,7 @@ TEST(ToolChainTest, VFSGCCInstallationRe > std::replace(S.begin(), S.end(), '\\', '/'); > #endif > EXPECT_EQ("Found candidate GCC installation: " > - "/home/test/lib/gcc/arm-linux-gnueabi/4.6.1\n" > + "/home/test/bin/../lib/gcc/arm-linux-gnueabi/4.6.1\n" > "Selected GCC installation: " > "/home/test/bin/../lib/gcc/arm-linux-gnueabi/4.6.1\n" > "Candidate multilib: .;@m32\n" > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
