Remove custom path manipulation functions from FileSpec

[labath]

Summary

now that llvm supports host-agnostic path manipulation functions (and
most of their kinks have been ironed out), we can remove our copies of
the path parsing functions in favour of the llvm ones.

This should be NFC except for the slight difference in handling of the
"" path, which is now normalized to "/" (this only applies to the
literal "" path; "//net" and friends still get to keep the two
slashes).

[clayborg]

source/Utility/FileSpec.cpp

277–281 ↗

(On Diff #146110)

Does this work ok in LLVM if the slashes have already been switched on line 275? We would specify a syntax of windows, but the slashes would be wrong?

[labath]

source/Utility/FileSpec.cpp

277–281 ↗

(On Diff #146110)

That's not a problem. in the windows path style, the llvm functions treat both kinds of slashes as directory separators.

lldb/trunk/source/Utility/FileSpec.cpp

 
 void Denormalize(llvm::SmallVectorImpl<char> &path,
                  FileSpec::PathSyntax syntax) {
   if (PathSyntaxIsPosix(syntax))
     return;
 
   std::replace(path.begin(), path.end(), '/', '\\');
 }
-
-size_t FilenamePos(llvm::StringRef str, FileSpec::PathSyntax syntax) {
-  if (str.size() == 2 && IsPathSeparator(str[0], syntax) && str[0] == str[1])
-    return 0;
-
-  if (str.size() > 0 && IsPathSeparator(str.back(), syntax))
-    return str.size() - 1;
-
-  size_t pos = str.find_last_of(GetPathSeparators(syntax), str.size() - 1);
-
-  if (!PathSyntaxIsPosix(syntax) && pos == llvm::StringRef::npos)
-    pos = str.find_last_of(':', str.size() - 2);
-
-  if (pos == llvm::StringRef::npos ||
-      (pos == 1 && IsPathSeparator(str[0], syntax)))
-    return 0;
-
-  return pos + 1;
-}
-
-size_t RootDirStart(llvm::StringRef str, FileSpec::PathSyntax syntax) {
-  // case "c:/"
-  if (!PathSyntaxIsPosix(syntax) &&
-      (str.size() > 2 && str[1] == ':' && IsPathSeparator(str[2], syntax)))
-    return 2;
-
-  // case "//"
-  if (str.size() == 2 && IsPathSeparator(str[0], syntax) && str[0] == str[1])
-    return llvm::StringRef::npos;
-
-  // case "//net"
-  if (str.size() > 3 && IsPathSeparator(str[0], syntax) && str[0] == str[1] &&
-      !IsPathSeparator(str[2], syntax))
-    return str.find_first_of(GetPathSeparators(syntax), 2);
-
-  // case "/"
-  if (str.size() > 0 && IsPathSeparator(str[0], syntax))
-    return 0;
-
-  return llvm::StringRef::npos;
-}
-
-size_t ParentPathEnd(llvm::StringRef path, FileSpec::PathSyntax syntax) {
-  size_t end_pos = FilenamePos(path, syntax);
-
-  bool filename_was_sep =
-      path.size() > 0 && IsPathSeparator(path[end_pos], syntax);
-
-  // Skip separators except for root dir.
-  size_t root_dir_pos = RootDirStart(path.substr(0, end_pos), syntax);
-
-  while (end_pos > 0 && (end_pos - 1) != root_dir_pos &&
-         IsPathSeparator(path[end_pos - 1], syntax))
-    --end_pos;
-
-  if (end_pos == 1 && root_dir_pos == 0 && filename_was_sep)
-    return llvm::StringRef::npos;
-
-  return end_pos;
-}
-
 } // end anonymous namespace
 
 void FileSpec::Resolve(llvm::SmallVectorImpl<char> &path) {
   if (path.empty())
     return;
 
   llvm::SmallString<32> Source(path.begin(), path.end());
   StandardTildeExpressionResolver Resolver;

 
 //------------------------------------------------------------------
 // Update the contents of this object with a new path. The path will be split
 // up into a directory and filename and stored as uniqued string values for
 // quick comparison and efficient memory usage.
 //------------------------------------------------------------------
 void FileSpec::SetFile(llvm::StringRef pathname, bool resolve,
                        PathSyntax syntax) {
-  // CLEANUP: Use StringRef for string handling.  This function is kind of a
-  // mess and the unclear semantics of RootDirStart and ParentPathEnd make it
-  // very difficult to understand this function.  There's no reason this
-  // function should be particularly complicated or difficult to understand.
   m_filename.Clear();
   m_directory.Clear();
   m_is_resolved = false;
   m_syntax = (syntax == ePathSyntaxHostNative) ? GetNativeSyntax() : syntax;
 
   if (pathname.empty())
     return;
 
   llvm::SmallString<64> resolved(pathname);
 
   if (resolve) {
     FileSpec::Resolve(resolved);
     m_is_resolved = true;
   }
 
   // Normalize the path by removing ".", ".." and other redundant components.
   if (needsNormalization(resolved))
     llvm::sys::path::remove_dots(resolved, true, LLVMPathSyntax(m_syntax));
 
   // Normalize back slashes to forward slashes
   if (m_syntax == FileSpec::ePathSyntaxWindows)
     std::replace(resolved.begin(), resolved.end(), '\\', '/');
 
-  llvm::StringRef resolve_path_ref(resolved.c_str());
-  size_t dir_end = ParentPathEnd(resolve_path_ref, m_syntax);
-  if (dir_end == 0) {
-    m_filename.SetString(resolve_path_ref);
-    return;
-  }
-
-  m_directory.SetString(resolve_path_ref.substr(0, dir_end));
-
-  size_t filename_begin = dir_end;
-  size_t root_dir_start = RootDirStart(resolve_path_ref, m_syntax);
-  while (filename_begin != llvm::StringRef::npos &&
-         filename_begin < resolve_path_ref.size() &&
-         filename_begin != root_dir_start &&
-         IsPathSeparator(resolve_path_ref[filename_begin], m_syntax))
-    ++filename_begin;
-  m_filename.SetString((filename_begin == llvm::StringRef::npos ||
-                        filename_begin >= resolve_path_ref.size())
-                           ? "."
-                           : resolve_path_ref.substr(filename_begin));
+  auto style = LLVMPathSyntax(syntax);
+  m_filename.SetString(llvm::sys::path::filename(resolved, style));
+  llvm::StringRef dir = llvm::sys::path::parent_path(resolved, style);
+  if (!dir.empty())
+    m_directory.SetString(dir);
 }
 
 void FileSpec::SetFile(llvm::StringRef path, bool resolve,
                        const llvm::Triple &Triple) {
   return SetFile(path, resolve,
                  Triple.isOSWindows() ? ePathSyntaxWindows : ePathSyntaxPosix);
 }
 

lldb/trunk/unittests/Utility/FileSpecTest.cpp

std::pair<const char *, const char *> posix_tests[] = {
       {"/foo/./bar", "/foo/bar"},
       {"/foo/..", "/"},
       {"/foo/.", "/foo"},
       {"/foo//bar", "/foo/bar"},
       {"/foo//bar/baz", "/foo/bar/baz"},
       {"/foo//bar/./baz", "/foo/bar/baz"},
       {"/./foo", "/foo"},
       {"/", "/"},
-      // TODO: fix llvm::sys::path::remove_dots() to return "//" below.
-      //{"//", "//"},
+      {"//", "/"},
       {"//net", "//net"},
       {"/..", "/"},
       {"/.", "/"},
       {"..", ".."},
       {".", ""},
       {"../..", "../.."},
       {"foo/..", ""},
       {"foo/../bar", "bar"},
       {"../foo/..", ".."},
       {"./foo", "foo"},
       {"././foo", "foo"},
       {"../foo", "../foo"},
       {"../../foo", "../../foo"},
   };
   for (auto test : posix_tests) {
+    SCOPED_TRACE(llvm::Twine("test.first = ") + test.first);
     EXPECT_EQ(test.second,
               FileSpec(test.first, false, FileSpec::ePathSyntaxPosix)
                   .GetPath());
   }
 
   std::pair<const char *, const char *> windows_tests[] = {
       {R"(c:\bar\..\bar)", R"(c:\bar)"},
       {R"(c:\bar\.\bar)", R"(c:\bar\bar)"},
       {R"(c:\bar\..)", R"(c:\)"},
       {R"(c:\bar\.)", R"(c:\bar)"},
       {R"(c:\.\bar)", R"(c:\bar)"},
       {R"(\)", R"(\)"},
-      //      {R"(\\)", R"(\\)"},
-      //      {R"(\\net)", R"(\\net)"},
+      {R"(\\)", R"(\)"},
+      {R"(\\net)", R"(\\net)"},
       {R"(c:\..)", R"(c:\)"},
       {R"(c:\.)", R"(c:\)"},
       // TODO: fix llvm::sys::path::remove_dots() to return "\" below.
       {R"(\..)", R"(\..)"},
       //      {R"(c:..)", R"(c:..)"},
       {R"(..)", R"(..)"},
       {R"(.)", R"()"},
       // TODO: fix llvm::sys::path::remove_dots() to return "c:\" below.