FileSpec: Fix PrependPathComponent("/")

[labath]

Summary

PrependPathComponent was unconditionally inserting path separators between the
path components. This is not correct if the prepended path is "/", which caused
problems down the line. Fix the function to use the same algorithm as
AppendPathComponent and add a test. This fixes one part of llvm.org/pr31611.

lldb/trunk/source/Host/common/FileSpec.cpp

if (m_directory) {
     }
     ConstString result(penultimate_slash_ptr + 1,
                        last_slash_ptr - penultimate_slash_ptr);
     return result;
   }
   return ConstString();
 }
 
+static std::string
+join_path_components(FileSpec::PathSyntax syntax,
+                     const std::vector<llvm::StringRef> components) {
+  std::string result;
+  for (size_t i = 0; i < components.size(); ++i) {
+    if (components[i].empty())
+      continue;
+    result += components[i];
+    if (i != components.size() - 1 &&
+        !IsPathSeparator(components[i].back(), syntax))
+      result += GetPreferredPathSeparator(syntax);
+  }
+
+  return result;
+}
+
 void FileSpec::PrependPathComponent(llvm::StringRef component) {
   if (component.empty())
     return;
 
   const bool resolve = false;
   if (m_filename.IsEmpty() && m_directory.IsEmpty()) {
     SetFile(component, resolve);
     return;
   }
 
-  char sep = GetPreferredPathSeparator(m_syntax);
-  std::string result;
-  if (m_filename.IsEmpty())
-    result = llvm::join_items(sep, component, m_directory.GetStringRef());
-  else if (m_directory.IsEmpty())
-    result = llvm::join_items(sep, component, m_filename.GetStringRef());
-  else
-    result = llvm::join_items(sep, component, m_directory.GetStringRef(),
-                              m_filename.GetStringRef());
-
+  std::string result =
+      join_path_components(m_syntax, {component, m_directory.GetStringRef(),
+                                      m_filename.GetStringRef()});
   SetFile(result, resolve);
 }
 
 void FileSpec::PrependPathComponent(const FileSpec &new_path) {
   return PrependPathComponent(new_path.GetPath(false));
 }
 
 void FileSpec::AppendPathComponent(llvm::StringRef component) {
   if (component.empty())
     return;
 
-  std::string result;
-  if (!m_directory.IsEmpty()) {
-    result += m_directory.GetStringRef();
-    if (!IsPathSeparator(m_directory.GetStringRef().back(), m_syntax))
-      result += GetPreferredPathSeparator(m_syntax);
-  }
-
-  if (!m_filename.IsEmpty()) {
-    result += m_filename.GetStringRef();
-    if (!IsPathSeparator(m_filename.GetStringRef().back(), m_syntax))
-      result += GetPreferredPathSeparator(m_syntax);
-  }
-
   component = component.drop_while(
       [this](char c) { return IsPathSeparator(c, m_syntax); });
 
-  result += component;
+  std::string result =
+      join_path_components(m_syntax, {m_directory.GetStringRef(),
+                                      m_filename.GetStringRef(), component});
 
   SetFile(result, false, m_syntax);
 }
 
 void FileSpec::AppendPathComponent(const FileSpec &new_path) {
   return AppendPathComponent(new_path.GetPath(false));
 }
 

lldb/trunk/unittests/Host/FileSpecTest.cpp

 TEST(FileSpecTest, CopyByAppendingPathComponent) {
   FileSpec fs = FileSpec("/foo", false, FileSpec::ePathSyntaxPosix)
                     .CopyByAppendingPathComponent("bar");
   EXPECT_STREQ("/foo/bar", fs.GetCString());
   EXPECT_STREQ("/foo", fs.GetDirectory().GetCString());
   EXPECT_STREQ("bar", fs.GetFilename().GetCString());
 }
 
+TEST(FileSpecTest, PrependPathComponent) {
+  FileSpec fs_posix("foo", false, FileSpec::ePathSyntaxPosix);
+  fs_posix.PrependPathComponent("/bar");
+  EXPECT_STREQ("/bar/foo", fs_posix.GetCString());
+
+  FileSpec fs_posix_2("foo/bar", false, FileSpec::ePathSyntaxPosix);
+  fs_posix_2.PrependPathComponent("/baz");
+  EXPECT_STREQ("/baz/foo/bar", fs_posix_2.GetCString());
+
+  FileSpec fs_windows("baz", false, FileSpec::ePathSyntaxWindows);
+  fs_windows.PrependPathComponent("F:\\bar");
+  EXPECT_STREQ("F:\\bar\\baz", fs_windows.GetCString());
+
+  FileSpec fs_posix_root("bar", false, FileSpec::ePathSyntaxPosix);
+  fs_posix_root.PrependPathComponent("/");
+  EXPECT_STREQ("/bar", fs_posix_root.GetCString());
+
+  FileSpec fs_windows_root("bar", false, FileSpec::ePathSyntaxWindows);
+  fs_windows_root.PrependPathComponent("F:\\");
+  EXPECT_STREQ("F:\\bar", fs_windows_root.GetCString());
+}
+
 static void Compare(const FileSpec &one, const FileSpec &two, bool full_match,
                     bool remove_backup_dots, bool result) {
   EXPECT_EQ(result, FileSpec::Equal(one, two, full_match, remove_backup_dots))
       << "File one: " << one.GetCString() << "\nFile two: " << two.GetCString()
       << "\nFull match: " << full_match
       << "\nRemove backup dots: " << remove_backup_dots;
 }
 
TEST(FileSpecTest, FormatFileSpec) {
   EXPECT_EQ("foo\\bar.txt", llvm::formatv("{0}", F).str());
   EXPECT_EQ("foo\\", llvm::formatv("{0:D}", F).str());
   EXPECT_EQ("bar.txt", llvm::formatv("{0:F}", F).str());
 
   F = FileSpec("foo", false, win);
   EXPECT_EQ("foo", llvm::formatv("{0}", F).str());
   EXPECT_EQ("foo", llvm::formatv("{0:F}", F).str());
   EXPECT_EQ("(empty)", llvm::formatv("{0:D}", F).str());
-}
+}