simark updated this revision to Diff 157287.
simark added a comment.
Fix tests on Windows
Fix InMemoryFileSystem tests on Windows. The paths returned by the
InMemoryFileSystem directory iterators in the tests mix posix and windows
directory separators. THis is because we do queries with posix-style
separators ("/a/b") but filnames are appended using native-style separators
(backslash on Windows). So we end up with "/a/b\c".
I fixed the test by re-using the ReplaceBackslashes function defined in another
test. I'm not sure this is the best fix, but the only alternative I see would
be to completely rewrite tests to use posix-style paths on non-Windows and
Windows-style paths on Windows. That would lead to quite a bit of
duplication...
Repository:
rC Clang
https://reviews.llvm.org/D48903
Files:
lib/Basic/FileManager.cpp
lib/Basic/VirtualFileSystem.cpp
unittests/Basic/VirtualFileSystemTest.cpp
unittests/Driver/ToolChainTest.cpp
Index: unittests/Driver/ToolChainTest.cpp
===================================================================
--- unittests/Driver/ToolChainTest.cpp
+++ unittests/Driver/ToolChainTest.cpp
@@ -113,7 +113,7 @@
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"
Index: unittests/Basic/VirtualFileSystemTest.cpp
===================================================================
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -151,6 +151,11 @@
addEntry(Path, S);
}
};
+
+auto ReplaceBackslashes = [](std::string S) {
+ std::replace(S.begin(), S.end(), '\\', '/');
+ return S;
+};
} // end anonymous namespace
TEST(VirtualFileSystemTest, StatusQueries) {
@@ -782,7 +787,7 @@
I = FS.dir_begin("/b", EC);
ASSERT_FALSE(EC);
- ASSERT_EQ("/b/c", I->getName());
+ ASSERT_EQ("/b/c", ReplaceBackslashes(I->getName()));
I.increment(EC);
ASSERT_FALSE(EC);
ASSERT_EQ(vfs::directory_iterator(), I);
@@ -794,16 +799,12 @@
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");
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", ReplaceBackslashes(
@@ -919,6 +920,37 @@
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", ReplaceBackslashes(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 {
Index: lib/Basic/VirtualFileSystem.cpp
===================================================================
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -471,15 +471,31 @@
/// 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 +520,22 @@
}
};
-/// 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 +735,7 @@
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 +748,8 @@
// 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 +761,33 @@
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 +803,8 @@
}
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>());
Index: lib/Basic/FileManager.cpp
===================================================================
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -315,9 +315,11 @@
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
