Moving FileManager::removeDotPaths to llvm::sys::path::remove_dots

[aizatsky]

Summary

Corresponding clang change:

http://reviews.llvm.org/D14394

[aizatsky]

• bug fix

[aizatsky]

• nit

[aizatsky]

• fixes

[silvas]

lib/Support/Path.cpp

664 ↗

(On Diff #39427)

Why remove this function?

691 ↗

(On Diff #39427)

Can you implement the std::string version in terms of the SmallVectorImpl version? In the "small" case the version currently in clang does no heap allocation. It would be a shame to unnecessarily start doing heap allocation for this.

[aizatsky]

review

[aizatsky]

PTAL

[kcc]

LGTM

[silvas]

LGTM. It's a bit weird returning a SmallString, but inlining should take care of it.

llvm/trunk/include/llvm/Support/Path.h

 bool is_relative(const Twine &path);
 
 /// @brief Remove redundant leading "./" pieces and consecutive separators.
 ///
 /// @param path Input path.
 /// @result The cleaned-up \a path.
 StringRef remove_leading_dotslash(StringRef path);
 
+/// @brief In-place remove any './' and optionally '../' components from a path.
+///
+/// @param path processed path
+/// @param remove_dot_dot specify if '../' should be removed
+/// @result True if path was changed
+bool remove_dots(SmallVectorImpl<char> &path, bool remove_dot_dot = false);
+
 } // end namespace path
 } // end namespace sys
 } // end namespace llvm
 
 #endif

llvm/trunk/lib/Support/Path.cpp

StringRef remove_leading_dotslash(StringRef Path) {
   while (Path.size() > 2 && Path[0] == '.' && is_separator(Path[1])) {
     Path = Path.substr(2);
     while (Path.size() > 0 && is_separator(Path[0]))
       Path = Path.substr(1);
   }
   return Path;
 }
 
+static SmallString<256> remove_dots(StringRef path, bool remove_dot_dot) {
+  SmallVector<StringRef, 16> components;
+
+  // Skip the root path, then look for traversal in the components.
+  StringRef rel = path::relative_path(path);
+  for (StringRef C : llvm::make_range(path::begin(rel), path::end(rel))) {
+    if (C == ".")
+      continue;
+    if (remove_dot_dot) {
+      if (C == "..") {
+        if (!components.empty())
+          components.pop_back();
+        continue;
+      }
+    }
+    components.push_back(C);
+  }
+
+  SmallString<256> buffer = path::root_path(path);
+  for (StringRef C : components)
+    path::append(buffer, C);
+  return buffer;
+}
+
+bool remove_dots(SmallVectorImpl<char> &path, bool remove_dot_dot) {
+  StringRef p(path.data(), path.size());
+
+  SmallString<256> result = remove_dots(p, remove_dot_dot);
+  if (result == path)
+    return false;
+
+  path.swap(result);
+  return true;
+}
+
 } // end namespace path
 
 namespace fs {
 
 std::error_code getUniqueID(const Twine Path, UniqueID &Result) {
   file_status Status;
   std::error_code EC = status(Path, Status);
   if (EC)

llvm/trunk/unittests/Support/Path.cpp

TEST(Support, RemoveLeadingDotSlash) {
   StringRef Path1("././/foolz/wat");
   StringRef Path2("./////");
 
   Path1 = path::remove_leading_dotslash(Path1);
   EXPECT_EQ(Path1, "foolz/wat");
   Path2 = path::remove_leading_dotslash(Path2);
   EXPECT_EQ(Path2, "");
 }
+
+static std::string remove_dots(StringRef path,
+    bool remove_dot_dot) {
+  SmallString<256> buffer(path);
+  path::remove_dots(buffer, remove_dot_dot);
+  return buffer.str();
+}
+
+TEST(Support, RemoveDots) {
+  EXPECT_EQ("foolz/wat", remove_dots("././/foolz/wat", false));
+  EXPECT_EQ("", remove_dots("./////", false));
+
+  EXPECT_EQ("a/../b/c", remove_dots("./a/../b/c", false));
+  EXPECT_EQ("b/c", remove_dots("./a/../b/c", true));
+  EXPECT_EQ("c", remove_dots("././c", true));
+
+  SmallString<64> Path1("././c");
+  EXPECT_TRUE(path::remove_dots(Path1, true));
+  EXPECT_EQ("c", Path1);
+}
 } // anonymous namespace