[VFS] Let the user decide if they want path normalization.

[bkramer]

Summary

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.

[klimek]

include/clang/Basic/VirtualFileSystem.h

276 ↗

(On Diff #37120)

I'd use an in class initializer.

282 ↗

(On Diff #37120)

Document in which cases false can happen (or return a status).

[bkramer]

Addressed review comments.

[klimek]

lg

include/clang/Basic/VirtualFileSystem.h

282 ↗

(On Diff #37124)

Here, too!

cfe/trunk/include/clang/Basic/VirtualFileSystem.h

 namespace detail {
 class InMemoryDirectory;
 } // end namespace detail
 
 /// An in-memory file system.
 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>>
   openFileForRead(const Twine &Path) override;
   directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
   llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
     return WorkingDirectory;
   }

cfe/trunk/lib/Basic/VirtualFileSystem.cpp

 //===- VirtualFileSystem.cpp - Virtual File System Layer --------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 // This file implements the VirtualFileSystem interface.
 //===----------------------------------------------------------------------===//
 
 #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"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
std::string toString(unsigned Indent) const override {
     return Result;
   }
   static bool classof(const InMemoryNode *N) {
     return N->getKind() == IME_Directory;
   }
 };
 }
 
-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() {}
 
 std::string InMemoryFileSystem::toString() const {
   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);
 
   // Fix up relative paths. This just prepends the current working directory.
   std::error_code EC = makeAbsolute(Path);
   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);
     ++I;
     if (!Node) {
       if (I == E) {
         // End of the path, create a new file.
         // FIXME: expose the status details in the interface.
         Status Stat(P.str(), getNextVirtualUniqueID(),
                     llvm::sys::TimeValue(ModificationTime, 0), 0, 0,
                     Buffer->getBufferSize(),
                     llvm::sys::fs::file_type::regular_file,
                     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.
       // FIXME: expose the status details in the interface.
       Status Stat(
           StringRef(Path.str().begin(), Name.end() - Path.str().begin()),
           getNextVirtualUniqueID(), llvm::sys::TimeValue(ModificationTime, 0),
           0, 0, Buffer->getBufferSize(),
           llvm::sys::fs::file_type::directory_file, llvm::sys::fs::all_all);
       Dir = cast<detail::InMemoryDirectory>(Dir->addChild(
           Name, llvm::make_unique<detail::InMemoryDirectory>(std::move(Stat))));
       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(
                      Buffer->getBuffer(), Buffer->getBufferIdentifier()));
 }
 
 static ErrorOr<detail::InMemoryNode *>
 lookupInMemoryNode(const InMemoryFileSystem &FS, detail::InMemoryDirectory *Dir,
                    const Twine &P) {
   SmallString<128> Path;
   P.toVector(Path);
 
   // Fix up relative paths. This just prepends the current working directory.
   std::error_code EC = FS.makeAbsolute(Path);
   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;
     if (!Node)
       return errc::no_such_file_or_directory;
 
     // Return the file if it's at the end of the path.
     if (auto File = dyn_cast<detail::InMemoryFile>(Node)) {

cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp

// Make sure we get the top-most entry
     ASSERT_NE(E, I);
     EXPECT_EQ(sys::fs::owner_write, I->getPermissions());
   }
 }
 
 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) {
   auto Stat = FS.status("/a");
   ASSERT_EQ(Stat.getError(),errc::no_such_file_or_directory) << FS.toString();
   Stat = FS.status("/");
   ASSERT_EQ(Stat.getError(), errc::no_such_file_or_directory) << FS.toString();
 }
 
 TEST_F(InMemoryFileSystemTest, WindowsPath) {
   FS.addFile("c:/windows/system128/foo.cpp", 0, MemoryBuffer::getMemBuffer(""));
   auto Stat = FS.status("c:");
 #if !defined(_WIN32)
   ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();
 #endif
   Stat = FS.status("c:/windows/system128/foo.cpp");
   ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();
   FS.addFile("d:/windows/foo.cpp", 0, MemoryBuffer::getMemBuffer(""));
   Stat = FS.status("d:/windows/foo.cpp");
   ASSERT_FALSE(Stat.getError()) << Stat.getError() << FS.toString();
 }
 
 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());
 }
 
 TEST_F(InMemoryFileSystemTest, OverlayFileNoOwn) {
   auto Buf = MemoryBuffer::getMemBuffer("a");
   FS.addFileNoOwn("/a", 0, Buf.get());
   auto Stat = FS.status("/a");
   ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();
   ASSERT_EQ("/a", Stat->getName());
 }
 
 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) {
   FS.addFile("/a", 0, MemoryBuffer::getMemBuffer(""));
   FS.addFile("/b/c", 0, MemoryBuffer::getMemBuffer(""));
 
   std::error_code EC;
   vfs::directory_iterator I = FS.dir_begin("/", EC);