From 4d23f45a11325dfc03a9904cfc1e354c54f06081 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Thu, 26 Jul 2018 23:21:51 +0000 Subject: [PATCH] Revert r338057 "[VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name" This broke clang/test/PCH/case-insensitive-include.c on Windows. llvm-svn: 338084 --- clang/lib/Basic/FileManager.cpp | 8 ++- clang/lib/Basic/VirtualFileSystem.cpp | 65 ++++++------------------- clang/unittests/Basic/VirtualFileSystemTest.cpp | 59 ++++------------------ clang/unittests/Driver/ToolChainTest.cpp | 2 +- 4 files changed, 28 insertions(+), 106 deletions(-) diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index 0a79800..7e2d01c 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -315,11 +315,9 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile, UFE.InPCH = Data.InPCH; UFE.File = std::move(F); UFE.IsValid = true; - - SmallString<128> RealPathName; - if (!FS->getRealPath(InterndFileName, RealPathName)) - UFE.RealPathName = RealPathName.str(); - + if (UFE.File) + if (auto RealPathName = UFE.File->getName()) + UFE.RealPathName = *RealPathName; return &UFE; } diff --git a/clang/lib/Basic/VirtualFileSystem.cpp b/clang/lib/Basic/VirtualFileSystem.cpp index a7b8b02..bcfcbdb 100644 --- a/clang/lib/Basic/VirtualFileSystem.cpp +++ b/clang/lib/Basic/VirtualFileSystem.cpp @@ -474,28 +474,12 @@ class InMemoryNode { Status Stat; InMemoryNodeKind Kind; -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) {} virtual ~InMemoryNode() = default; - /// 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()); - } + const Status &getStatus() const { return Stat; } InMemoryNodeKind getKind() const { return Kind; } virtual std::string toString(unsigned Indent) const = 0; }; @@ -520,21 +504,14 @@ public: } }; -/// Adapt a InMemoryFile for VFS' File interface. The goal is to make -/// \p InMemoryFileAdaptor mimic as much as possible the behavior of -/// \p RealFile. +/// Adapt a InMemoryFile for VFS' File interface. 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, std::string RequestedName) - : Node(Node), RequestedName(std::move(RequestedName)) {} + explicit InMemoryFileAdaptor(InMemoryFile &Node) : Node(Node) {} - llvm::ErrorOr status() override { - return Node.getStatus(RequestedName); - } + llvm::ErrorOr status() override { return Node.getStatus(); } llvm::ErrorOr> getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator, @@ -734,7 +711,7 @@ lookupInMemoryNode(const InMemoryFileSystem &FS, detail::InMemoryDirectory *Dir, llvm::ErrorOr InMemoryFileSystem::status(const Twine &Path) { auto Node = lookupInMemoryNode(*this, Root.get(), Path); if (Node) - return (*Node)->getStatus(Path.str()); + return (*Node)->getStatus(); return Node.getError(); } @@ -747,8 +724,7 @@ InMemoryFileSystem::openFileForRead(const Twine &Path) { // 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(*Node)) - return std::unique_ptr( - new detail::InMemoryFileAdaptor(*F, Path.str())); + return std::unique_ptr(new detail::InMemoryFileAdaptor(*F)); // FIXME: errc::not_a_file? return make_error_code(llvm::errc::invalid_argument); @@ -760,33 +736,21 @@ 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); - } 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, - std::string RequestedDirName) - : I(Dir.begin()), E(Dir.end()), - RequestedDirName(std::move(RequestedDirName)) { - setCurrentEntry(); + explicit InMemoryDirIterator(detail::InMemoryDirectory &Dir) + : I(Dir.begin()), E(Dir.end()) { + if (I != E) + CurrentEntry = I->second->getStatus(); } std::error_code increment() override { ++I; - setCurrentEntry(); + // When we're at the end, make CurrentEntry invalid and DirIterImpl will do + // the rest. + CurrentEntry = I != E ? I->second->getStatus() : Status(); return {}; } }; @@ -802,8 +766,7 @@ directory_iterator InMemoryFileSystem::dir_begin(const Twine &Dir, } if (auto *DirNode = dyn_cast(*Node)) - return directory_iterator( - std::make_shared(*DirNode, Dir.str())); + return directory_iterator(std::make_shared(*DirNode)); EC = make_error_code(llvm::errc::not_a_directory); return directory_iterator(std::make_shared()); diff --git a/clang/unittests/Basic/VirtualFileSystemTest.cpp b/clang/unittests/Basic/VirtualFileSystemTest.cpp index a9b39b5..c795be0 100644 --- a/clang/unittests/Basic/VirtualFileSystemTest.cpp +++ b/clang/unittests/Basic/VirtualFileSystemTest.cpp @@ -13,7 +13,6 @@ #include "llvm/Support/Errc.h" #include "llvm/Support/Host.h" #include "llvm/Support/MemoryBuffer.h" -#include "llvm/Support/Path.h" #include "llvm/Support/SourceMgr.h" #include "gtest/gtest.h" #include @@ -152,13 +151,6 @@ public: addEntry(Path, S); } }; - -/// Replace back-slashes by front-slashes. -std::string getPosixPath(std::string S) { - SmallString<128> Result; - llvm::sys::path::native(S, Result, llvm::sys::path::Style::posix); - return Result.str(); -}; } // end anonymous namespace TEST(VirtualFileSystemTest, StatusQueries) { @@ -790,9 +782,7 @@ TEST_F(InMemoryFileSystemTest, DirectoryIteration) { I = FS.dir_begin("/b", EC); ASSERT_FALSE(EC); - // When on Windows, we end up with "/b\\c" as the name. Convert to Posix - // path for the sake of the comparison. - ASSERT_EQ("/b/c", getPosixPath(I->getName())); + ASSERT_EQ("/b/c", I->getName()); I.increment(EC); ASSERT_FALSE(EC); ASSERT_EQ(vfs::directory_iterator(), I); @@ -804,19 +794,23 @@ TEST_F(InMemoryFileSystemTest, WorkingDirectory) { auto Stat = FS.status("/b/c"); ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString(); - ASSERT_EQ("/b/c", Stat->getName()); + ASSERT_EQ("c", Stat->getName()); ASSERT_EQ("/b", *FS.getCurrentWorkingDirectory()); Stat = FS.status("c"); ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString(); + auto ReplaceBackslashes = [](std::string S) { + std::replace(S.begin(), S.end(), '\\', '/'); + return S; + }; NormalizedFS.setCurrentWorkingDirectory("/b/c"); NormalizedFS.setCurrentWorkingDirectory("."); - ASSERT_EQ("/b/c", - getPosixPath(NormalizedFS.getCurrentWorkingDirectory().get())); + ASSERT_EQ("/b/c", ReplaceBackslashes( + NormalizedFS.getCurrentWorkingDirectory().get())); NormalizedFS.setCurrentWorkingDirectory(".."); - ASSERT_EQ("/b", - getPosixPath(NormalizedFS.getCurrentWorkingDirectory().get())); + ASSERT_EQ("/b", ReplaceBackslashes( + NormalizedFS.getCurrentWorkingDirectory().get())); } #if !defined(_WIN32) @@ -925,39 +919,6 @@ TEST_F(InMemoryFileSystemTest, AddDirectoryThenAddChild) { 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); - // When on Windows, we end up with "../b\\c" as the name. Convert to Posix - // path for the sake of the comparison. - ASSERT_EQ("../b/c", getPosixPath(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 { diff --git a/clang/unittests/Driver/ToolChainTest.cpp b/clang/unittests/Driver/ToolChainTest.cpp index 0d4c545..d4198ea 100644 --- a/clang/unittests/Driver/ToolChainTest.cpp +++ b/clang/unittests/Driver/ToolChainTest.cpp @@ -113,7 +113,7 @@ TEST(ToolChainTest, VFSGCCInstallationRelativeDir) { std::replace(S.begin(), S.end(), '\\', '/'); #endif EXPECT_EQ("Found candidate GCC installation: " - "/home/test/bin/../lib/gcc/arm-linux-gnueabi/4.6.1\n" + "/home/test/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" -- 2.7.4