Skip to content
Permalink
Browse files

[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.

An indirect impact of this change is that a -Wnonportable-include-path
warning is now emitted in test PCH/case-insensitive-include.c.  This is
because the real path of the included file (with the wrong case) was not
available previously, whereas it is now.

Reviewers: malaperle, ilya-biryukov, bkramer

Reviewed By: ilya-biryukov

Subscribers: eric_niebler, malaperle, omtcyfz, hokein, bkramer, ilya-biryukov, ioeric, cfe-commits

Differential Revision: https://reviews.llvm.org/D48903

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@339063 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information...
Simon Marchi
Simon Marchi committed Aug 6, 2018
1 parent 28b93f3 commit 89aa7f45a1f728144935289d4ce69d8522999de0
@@ -315,9 +315,11 @@ const FileEntry *FileManager::getFile(StringRef Filename, bool openFile,
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;
}

@@ -474,12 +474,28 @@ 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;

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,21 @@ class InMemoryFile : public InMemoryNode {
}
};

/// 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 +734,7 @@ lookupInMemoryNode(const InMemoryFileSystem &FS, detail::InMemoryDirectory *Dir,
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 +747,8 @@ 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<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 +760,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);
} 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 +802,8 @@ directory_iterator InMemoryFileSystem::dir_begin(const Twine &Dir,
}

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>());
@@ -13,7 +13,7 @@
// RUN: touch -r %t-dir/case-insensitive-include.h %t.copy
// RUN: mv %t.copy %t-dir/case-insensitive-include.h

// RUN: %clang_cc1 -fsyntax-only %s -include-pch %t.pch -I %t-dir -verify
// RUN: %clang_cc1 -Wno-nonportable-include-path -fsyntax-only %s -include-pch %t.pch -I %t-dir -verify

// expected-no-diagnostics

@@ -13,6 +13,7 @@
#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 <map>
@@ -151,6 +152,13 @@ class DummyFileSystem : public vfs::FileSystem {
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) {
@@ -782,7 +790,9 @@ TEST_F(InMemoryFileSystemTest, DirectoryIteration) {

I = FS.dir_begin("/b", EC);
ASSERT_FALSE(EC);
ASSERT_EQ("/b/c", I->getName());
// 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()));
I.increment(EC);
ASSERT_FALSE(EC);
ASSERT_EQ(vfs::directory_iterator(), I);
@@ -794,23 +804,19 @@ TEST_F(InMemoryFileSystemTest, WorkingDirectory) {

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(
NormalizedFS.getCurrentWorkingDirectory().get()));
ASSERT_EQ("/b/c",
getPosixPath(NormalizedFS.getCurrentWorkingDirectory().get()));
NormalizedFS.setCurrentWorkingDirectory("..");
ASSERT_EQ("/b", ReplaceBackslashes(
NormalizedFS.getCurrentWorkingDirectory().get()));
ASSERT_EQ("/b",
getPosixPath(NormalizedFS.getCurrentWorkingDirectory().get()));
}

#if !defined(_WIN32)
@@ -919,6 +925,39 @@ 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 {
@@ -113,7 +113,7 @@ TEST(ToolChainTest, VFSGCCInstallationRelativeDir) {
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"

0 comments on commit 89aa7f4

Please sign in to comment.
You can’t perform that action at this time.