r250060 - [VFS] Let the user decide if they want path normalization.
Benjamin Kramer via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 12 09:16:39 PDT 2015
Author: d0k
Date: Mon Oct 12 11:16:39 2015
New Revision: 250060
URL: http://llvm.org/viewvc/llvm-project?rev=250060&view=rev
Log:
[VFS] Let the user decide if they want path normalization.
This is a more principled version of what I did earlier. Path
normalization is generally a good thing, but may break users in strange
environments, e. g. using lots of symlinks. Let the user choose and
default it to on.
This also changes adding a duplicated file into returning an error if
the file contents are different instead of an assertion failure.
Differential Revision: http://reviews.llvm.org/D13658
Modified:
cfe/trunk/include/clang/Basic/VirtualFileSystem.h
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=250060&r1=250059&r2=250060&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Mon Oct 12 11:16:39 2015
@@ -273,17 +273,24 @@ class InMemoryDirectory;
class InMemoryFileSystem : public FileSystem {
std::unique_ptr<detail::InMemoryDirectory> Root;
std::string WorkingDirectory;
+ bool UseNormalizedPaths = true;
public:
- InMemoryFileSystem();
+ explicit InMemoryFileSystem(bool UseNormalizedPaths = true);
~InMemoryFileSystem() override;
/// Add a buffer to the VFS with a path. The VFS owns the buffer.
- void addFile(const Twine &Path, time_t ModificationTime,
+ /// \return true if the file was successfully added, false if the file already
+ /// exists in the file system with different contents.
+ bool addFile(const Twine &Path, time_t ModificationTime,
std::unique_ptr<llvm::MemoryBuffer> Buffer);
/// Add a buffer to the VFS with a path. The VFS does not own the buffer.
- void addFileNoOwn(const Twine &Path, time_t ModificationTime,
+ /// \return true if the file was successfully added, false if the file already
+ /// exists in the file system with different contents.
+ bool addFileNoOwn(const Twine &Path, time_t ModificationTime,
llvm::MemoryBuffer *Buffer);
std::string toString() const;
+ /// Return true if this file system normalizes . and .. in paths.
+ bool useNormalizedPaths() const { return UseNormalizedPaths; }
llvm::ErrorOr<Status> status(const Twine &Path) override;
llvm::ErrorOr<std::unique_ptr<File>>
Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=250060&r1=250059&r2=250060&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Mon Oct 12 11:16:39 2015
@@ -10,6 +10,7 @@
//===----------------------------------------------------------------------===//
#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Basic/FileManager.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
@@ -474,11 +475,12 @@ public:
};
}
-InMemoryFileSystem::InMemoryFileSystem()
+InMemoryFileSystem::InMemoryFileSystem(bool UseNormalizedPaths)
: Root(new detail::InMemoryDirectory(
Status("", getNextVirtualUniqueID(), llvm::sys::TimeValue::MinTime(),
0, 0, 0, llvm::sys::fs::file_type::directory_file,
- llvm::sys::fs::perms::all_all))) {}
+ llvm::sys::fs::perms::all_all))),
+ UseNormalizedPaths(UseNormalizedPaths) {}
InMemoryFileSystem::~InMemoryFileSystem() {}
@@ -486,7 +488,7 @@ std::string InMemoryFileSystem::toString
return Root->toString(/*Indent=*/0);
}
-void InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime,
+bool InMemoryFileSystem::addFile(const Twine &P, time_t ModificationTime,
std::unique_ptr<llvm::MemoryBuffer> Buffer) {
SmallString<128> Path;
P.toVector(Path);
@@ -496,14 +498,14 @@ void InMemoryFileSystem::addFile(const T
assert(!EC);
(void)EC;
- detail::InMemoryDirectory *Dir = Root.get();
- auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);
- if (*I == ".")
- ++I;
+ if (useNormalizedPaths())
+ FileManager::removeDotPaths(Path, /*RemoveDotDot=*/true);
- if (I == E)
- return;
+ if (Path.empty())
+ return false;
+ detail::InMemoryDirectory *Dir = Root.get();
+ auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);
while (true) {
StringRef Name = *I;
detail::InMemoryNode *Node = Dir->getChild(Name);
@@ -519,7 +521,7 @@ void InMemoryFileSystem::addFile(const T
llvm::sys::fs::all_all);
Dir->addChild(Name, llvm::make_unique<detail::InMemoryFile>(
std::move(Stat), std::move(Buffer)));
- return;
+ return true;
}
// Create a new directory. Use the path up to here.
@@ -534,12 +536,24 @@ void InMemoryFileSystem::addFile(const T
continue;
}
- if (auto *NewDir = dyn_cast<detail::InMemoryDirectory>(Node))
+ if (auto *NewDir = dyn_cast<detail::InMemoryDirectory>(Node)) {
Dir = NewDir;
+ } else {
+ assert(isa<detail::InMemoryFile>(Node) &&
+ "Must be either file or directory!");
+
+ // Trying to insert a directory in place of a file.
+ if (I != E)
+ return false;
+
+ // Return false only if the new file is different from the existing one.
+ return cast<detail::InMemoryFile>(Node)->getBuffer()->getBuffer() ==
+ Buffer->getBuffer();
+ }
}
}
-void InMemoryFileSystem::addFileNoOwn(const Twine &P, time_t ModificationTime,
+bool InMemoryFileSystem::addFileNoOwn(const Twine &P, time_t ModificationTime,
llvm::MemoryBuffer *Buffer) {
return addFile(P, ModificationTime,
llvm::MemoryBuffer::getMemBuffer(
@@ -557,13 +571,13 @@ lookupInMemoryNode(const InMemoryFileSys
assert(!EC);
(void)EC;
- auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);
- if (*I == ".")
- ++I;
+ if (FS.useNormalizedPaths())
+ FileManager::removeDotPaths(Path, /*RemoveDotDot=*/true);
- if (I == E)
+ if (Path.empty())
return Dir;
+ auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);
while (true) {
detail::InMemoryNode *Node = Dir->getChild(*I);
++I;
Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=250060&r1=250059&r2=250060&view=diff
==============================================================================
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Mon Oct 12 11:16:39 2015
@@ -525,6 +525,11 @@ TEST(VirtualFileSystemTest, HiddenInIter
class InMemoryFileSystemTest : public ::testing::Test {
protected:
clang::vfs::InMemoryFileSystem FS;
+ clang::vfs::InMemoryFileSystem NormalizedFS;
+
+ InMemoryFileSystemTest()
+ : FS(/*UseNormalizedPaths=*/false),
+ NormalizedFS(/*UseNormalizedPaths=*/true) {}
};
TEST_F(InMemoryFileSystemTest, IsEmpty) {
@@ -549,8 +554,13 @@ TEST_F(InMemoryFileSystemTest, WindowsPa
TEST_F(InMemoryFileSystemTest, OverlayFile) {
FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a"));
+ NormalizedFS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a"));
auto Stat = FS.status("/");
ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();
+ Stat = FS.status("/.");
+ ASSERT_FALSE(Stat);
+ Stat = NormalizedFS.status("/.");
+ ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();
Stat = FS.status("/a");
ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();
ASSERT_EQ("/a", Stat->getName());
@@ -566,17 +576,36 @@ TEST_F(InMemoryFileSystemTest, OverlayFi
TEST_F(InMemoryFileSystemTest, OpenFileForRead) {
FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a"));
- FS.addFile("./c", 0, MemoryBuffer::getMemBuffer("c"));
+ FS.addFile("././c", 0, MemoryBuffer::getMemBuffer("c"));
+ FS.addFile("./d/../d", 0, MemoryBuffer::getMemBuffer("d"));
+ NormalizedFS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a"));
+ NormalizedFS.addFile("././c", 0, MemoryBuffer::getMemBuffer("c"));
+ NormalizedFS.addFile("./d/../d", 0, MemoryBuffer::getMemBuffer("d"));
auto File = FS.openFileForRead("/a");
ASSERT_EQ("a", (*(*File)->getBuffer("ignored"))->getBuffer());
File = FS.openFileForRead("/a"); // Open again.
ASSERT_EQ("a", (*(*File)->getBuffer("ignored"))->getBuffer());
+ File = NormalizedFS.openFileForRead("/././a"); // Open again.
+ ASSERT_EQ("a", (*(*File)->getBuffer("ignored"))->getBuffer());
File = FS.openFileForRead("/");
ASSERT_EQ(File.getError(), errc::invalid_argument) << FS.toString();
File = FS.openFileForRead("/b");
ASSERT_EQ(File.getError(), errc::no_such_file_or_directory) << FS.toString();
- File = FS.openFileForRead("c");
+ File = FS.openFileForRead("./c");
+ ASSERT_FALSE(File);
+ File = FS.openFileForRead("e/../d");
+ ASSERT_FALSE(File);
+ File = NormalizedFS.openFileForRead("./c");
ASSERT_EQ("c", (*(*File)->getBuffer("ignored"))->getBuffer());
+ File = NormalizedFS.openFileForRead("e/../d");
+ ASSERT_EQ("d", (*(*File)->getBuffer("ignored"))->getBuffer());
+}
+
+TEST_F(InMemoryFileSystemTest, DuplicatedFile) {
+ ASSERT_TRUE(FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a")));
+ ASSERT_FALSE(FS.addFile("/a/b", 0, MemoryBuffer::getMemBuffer("a")));
+ ASSERT_TRUE(FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("a")));
+ ASSERT_FALSE(FS.addFile("/a", 0, MemoryBuffer::getMemBuffer("b")));
}
TEST_F(InMemoryFileSystemTest, DirectoryIteration) {
More information about the cfe-commits
mailing list