ppluzhnikov created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
ppluzhnikov updated this revision to Diff 432324.
ppluzhnikov added a comment.
ppluzhnikov updated this revision to Diff 432461.
ppluzhnikov updated this revision to Diff 432551.
ppluzhnikov updated this revision to Diff 432577.
ppluzhnikov updated this revision to Diff 432592.
ppluzhnikov updated this revision to Diff 432612.
ppluzhnikov updated this revision to Diff 432623.
ppluzhnikov published this revision for review.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Fix failed tests.


ppluzhnikov added a comment.

Rebase to ToT.


ppluzhnikov added a comment.

Fix InMemoryFileSystemTest bug.


ppluzhnikov added a comment.

Fix ProxyFileSystemTest.Basic on Win/x64


ppluzhnikov added a comment.

One more ProxyFileSystemTest fixe.


ppluzhnikov added a comment.

Fix compile failure.


ppluzhnikov added a comment.

Fix formatting.


The function promises to canonicalize the path, but neglected to do so
for the root component.

For example, calling remove_dots("/tmp/foo.c", Style::windows_backslash)
resulted in "/tmp\foo.c". Now it produces "\tmp\foo.c".

Also fix FIXME in the corresponding test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126412

Files:
  clang/unittests/Basic/FileManagerTest.cpp
  clang/unittests/Frontend/PCHPreambleTest.cpp
  llvm/lib/Support/Path.cpp
  llvm/unittests/Support/Path.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===================================================================
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -199,7 +199,7 @@
 };
 
 /// Replace back-slashes by front-slashes.
-std::string getPosixPath(std::string S) {
+std::string getPosixPath(const Twine &S) {
   SmallString<128> Result;
   llvm::sys::path::native(S, Result, llvm::sys::path::Style::posix);
   return std::string(Result.str());
@@ -923,11 +923,11 @@
 
   auto PWD = PFS.getCurrentWorkingDirectory();
   ASSERT_FALSE(PWD.getError());
-  ASSERT_EQ("/", *PWD);
+  ASSERT_EQ("/", getPosixPath(*PWD));
 
   SmallString<16> Path;
   ASSERT_FALSE(PFS.getRealPath("a", Path));
-  ASSERT_EQ("/a", Path);
+  ASSERT_EQ("/a", getPosixPath(Path));
 
   bool Local = true;
   ASSERT_FALSE(PFS.isLocal("/a", Local));
@@ -1343,7 +1343,8 @@
   EXPECT_NE(FS.status("/a")->getUniqueID(), FS.status("/e")->getUniqueID());
 
   // Recreating the "same" FS yields the same UniqueIDs.
-  vfs::InMemoryFileSystem FS2;
+  // Note: FS2 should match FS with respect to path normalization.
+  vfs::InMemoryFileSystem FS2(/*UseNormalizedPath=*/false);
   ASSERT_TRUE(FS2.addFile("/a/b", 0, MemoryBuffer::getMemBuffer("text")));
   EXPECT_EQ(FS.status("/a/b")->getUniqueID(),
             FS2.status("/a/b")->getUniqueID());
Index: llvm/unittests/Support/Path.cpp
===================================================================
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1544,16 +1544,14 @@
   EXPECT_EQ("C:\\a\\c", remove_dots("C:\\foo\\bar//..\\..\\a\\c", true,
                                     path::Style::windows));
 
-  // FIXME: These leading forward slashes are emergent behavior. VFS depends on
-  // this behavior now.
-  EXPECT_EQ("C:/bar",
+  EXPECT_EQ("C:\\bar",
             remove_dots("C:/foo/../bar", true, path::Style::windows));
-  EXPECT_EQ("C:/foo\\bar",
+  EXPECT_EQ("C:\\foo\\bar",
             remove_dots("C:/foo/bar", true, path::Style::windows));
-  EXPECT_EQ("C:/foo\\bar",
+  EXPECT_EQ("C:\\foo\\bar",
             remove_dots("C:/foo\\bar", true, path::Style::windows));
-  EXPECT_EQ("/", remove_dots("/", true, path::Style::windows));
-  EXPECT_EQ("C:/", remove_dots("C:/", true, path::Style::windows));
+  EXPECT_EQ("\\", remove_dots("/", true, path::Style::windows));
+  EXPECT_EQ("C:\\", remove_dots("C:/", true, path::Style::windows));
 
   // Some clients of remove_dots expect it to remove trailing slashes. Again,
   // this is emergent behavior that VFS relies on, and not inherently part of
@@ -1564,7 +1562,8 @@
             remove_dots("/foo/bar/", true, path::Style::posix));
 
   // A double separator is rewritten.
-  EXPECT_EQ("C:/foo\\bar", remove_dots("C:/foo//bar", true, path::Style::windows));
+  EXPECT_EQ("C:\\foo\\bar",
+            remove_dots("C:/foo//bar", true, path::Style::windows));
 
   SmallString<64> Path1(".\\.\\c");
   EXPECT_TRUE(path::remove_dots(Path1, true, path::Style::windows));
Index: llvm/lib/Support/Path.cpp
===================================================================
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -760,11 +760,15 @@
     }
   }
 
+  SmallString<256> buffer = root;
+  // "root" could be "/", which may need to be translated into "\".
+  make_preferred(buffer, style);
+  needs_change |= root != buffer;
+
   // Avoid rewriting the path unless we have to.
   if (!needs_change)
     return false;
 
-  SmallString<256> buffer = root;
   if (!components.empty()) {
     buffer += components[0];
     for (StringRef C : makeArrayRef(components).drop_front()) {
Index: clang/unittests/Frontend/PCHPreambleTest.cpp
===================================================================
--- clang/unittests/Frontend/PCHPreambleTest.cpp
+++ clang/unittests/Frontend/PCHPreambleTest.cpp
@@ -24,6 +24,13 @@
 
 namespace {
 
+std::string Canonicalize(const Twine &Path) {
+  SmallVector<char, 128> PathVec;
+  Path.toVector(PathVec);
+  llvm::sys::path::remove_dots(PathVec, true);
+  return std::string(PathVec.begin(), PathVec.end());
+}
+
 class ReadCountingInMemoryFileSystem : public vfs::InMemoryFileSystem
 {
   std::map<std::string, unsigned> ReadCounts;
@@ -31,16 +38,13 @@
 public:
   ErrorOr<std::unique_ptr<vfs::File>> openFileForRead(const Twine &Path) override
   {
-    SmallVector<char, 128> PathVec;
-    Path.toVector(PathVec);
-    llvm::sys::path::remove_dots(PathVec, true);
-    ++ReadCounts[std::string(PathVec.begin(), PathVec.end())];
+    ++ReadCounts[Canonicalize(Path)];
     return InMemoryFileSystem::openFileForRead(Path);
   }
 
   unsigned GetReadCount(const Twine &Path) const
   {
-    auto it = ReadCounts.find(Path.str());
+    auto it = ReadCounts.find(Canonicalize(Path));
     return it == ReadCounts.end() ? 0 : it->second;
   }
 };
Index: clang/unittests/Basic/FileManagerTest.cpp
===================================================================
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -411,7 +411,7 @@
 #endif  // !_WIN32
 
 static StringRef getSystemRoot() {
-  return is_style_windows(llvm::sys::path::Style::native) ? "C:/" : "/";
+  return is_style_windows(llvm::sys::path::Style::native) ? "C:\\" : "/";
 }
 
 TEST_F(FileManagerTest, makeAbsoluteUsesVFS) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to