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

Why remove this function?

691

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.

include/llvm/Support/Path.h

 bool is_absolute(const Twine &path);
 
 /// @brief Is path relative?
 ///
 /// @param path Input path.
 /// @result True if the path is relative, false if it is not.
 bool is_relative(const Twine &path);
 
-/// @brief Remove redundant leading "./" pieces and consecutive separators.
+/// @brief Remove any './' and optionally '../' components from a path.
 ///
-/// @param path Input path.
-/// @result The cleaned-up \a path.
-StringRef remove_leading_dotslash(StringRef path);
+/// @param path Input path
+/// @param remove_dot_dot specify if '../' should be removed
+/// @result processed path
+std::string remove_dots(StringRef path, bool remove_dot_dot = false);
+
+/// @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

lib/Support/Path.cpp

#else
        rootName = true;
 #endif
 
   return rootDir && rootName;
 }
 
 bool is_relative(const Twine &path) { return !is_absolute(path); }
 
-StringRef remove_leading_dotslash(StringRef Path) {
-  // Remove leading "./" (or ".//" or "././" etc.)
-  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;
+std::string 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.str();
+
+bool remove_dots(SmallVectorImpl<char> &path, bool remove_dot_dot) {
+  StringRef p(path.data(), path.size());
+
+  std::string result = remove_dots(p, remove_dot_dot);
+  if (result == p)
+    return false;
+
+  path.erase(path.begin(), path.end());
+  path.append(result.begin(), result.end());
+  return true;
 }
 
 } // end namespace path
 
 namespace fs {
 
 std::error_code getUniqueID(const Twine Path, UniqueID &Result) {
   file_status Status;

unittests/Support/Path.cpp

#endif
 
   path::native(Path6);
   EXPECT_PATH_IS(Path6, "a\\", "a/");
 
 #undef EXPECT_PATH_IS
 }
 
 TEST(Support, RemoveLeadingDotSlash) {
-  StringRef Path1("././/foolz/wat");
-  StringRef Path2("./////");
+  EXPECT_EQ("foolz/wat", path::remove_dots("././/foolz/wat"));
+  EXPECT_EQ("", path::remove_dots("./////"));
 
-  Path1 = path::remove_leading_dotslash(Path1);
-  EXPECT_EQ(Path1, "foolz/wat");
-  Path2 = path::remove_leading_dotslash(Path2);
-  EXPECT_EQ(Path2, "");
+  EXPECT_EQ("a/../b/c", path::remove_dots("./a/../b/c"));
+  EXPECT_EQ("b/c", path::remove_dots("./a/../b/c", true));
+  EXPECT_EQ("c", path::remove_dots("././c", true));
+
+  SmallString<64> Path1("././c");
+  EXPECT_TRUE(path::remove_dots(Path1, true));
+  EXPECT_EQ("c", Path1);
 }
 } // anonymous namespace