Make FileSpec use StringRef.

[zturner]

Summary

This patch depends on D24880 going in for the join_items function, but at least you can comment on it now.

[clayborg]

Watch the buildbots for failures, but this looks fine. We aren't changing how the strings for filename and directory are stored, just using StringRef to deliver the arguments.

[amccarth]

source/Host/common/FileSpec.cpp

1317 ↗

(On Diff #72555)

Function is misspelled. Should be GetPreferredPathSeparator (1 'f', 2 'r's), throughout this file.

Fortunately, it's file-static, so it's easy to fix in this patch.

lldb/trunk/include/lldb/Host/FileSpec.h

public:
   ///
   /// @param[in] resolve_path
   ///     If \b true, then we resolve the path, removing stray ../.. and so
   ///     forth,
   ///     if \b false we trust the path is in canonical form already.
   ///
   /// @see FileSpec::SetFile (const char *path, bool resolve)
   //------------------------------------------------------------------
-  // TODO: Convert this constructor to use a StringRef.
-  explicit FileSpec(const char *path, bool resolve_path,
+  explicit FileSpec(llvm::StringRef path, bool resolve_path,
                     PathSyntax syntax = ePathSyntaxHostNative);
 
-  explicit FileSpec(const char *path, bool resolve_path, ArchSpec arch);
-
-  explicit FileSpec(const std::string &path, bool resolve_path,
-                    PathSyntax syntax = ePathSyntaxHostNative);
-
-  explicit FileSpec(const std::string &path, bool resolve_path, ArchSpec arch);
+  explicit FileSpec(llvm::StringRef path, bool resolve_path, ArchSpec arch);
 
   //------------------------------------------------------------------
   /// Copy constructor
   ///
   /// Makes a copy of the uniqued directory and filename strings from
   /// \a rhs.
   ///
   /// @param[in] rhs
public:
   ///
   /// @param[in] path
   ///     A full, partial, or relative path to a file.
   ///
   /// @param[in] resolve_path
   ///     If \b true, then we will try to resolve links the path using
   ///     the static FileSpec::Resolve.
   //------------------------------------------------------------------
-  void SetFile(const char *path, bool resolve_path,
-               PathSyntax syntax = ePathSyntaxHostNative);
-
-  void SetFile(const char *path, bool resolve_path, ArchSpec arch);
-
-  void SetFile(const std::string &path, bool resolve_path,
+  void SetFile(llvm::StringRef path, bool resolve_path,
                PathSyntax syntax = ePathSyntaxHostNative);
 
-  void SetFile(const std::string &path, bool resolve_path, ArchSpec arch);
+  void SetFile(llvm::StringRef path, bool resolve_path, ArchSpec arch);
 
   bool IsResolved() const { return m_is_resolved; }
 
   //------------------------------------------------------------------
   /// Set if the file path has been resolved or not.
   ///
   /// If you know a file path is already resolved and avoided passing
   /// a \b true parameter for any functions that take a "bool
public:
   /// argument with the resolved path.
   ///
   /// @param[in] path
   ///     Input path to be resolved, in the form of a llvm::SmallString or
   ///     similar.
   //------------------------------------------------------------------
   static void Resolve(llvm::SmallVectorImpl<char> &path);
 
-  FileSpec CopyByAppendingPathComponent(const char *new_path) const;
-
+  FileSpec CopyByAppendingPathComponent(llvm::StringRef component) const;
   FileSpec CopyByRemovingLastPathComponent() const;
 
-  void PrependPathComponent(const char *new_path);
-
-  void PrependPathComponent(const std::string &new_path);
-
+  void PrependPathComponent(llvm::StringRef component);
   void PrependPathComponent(const FileSpec &new_path);
 
-  void AppendPathComponent(const char *new_path);
-
-  void AppendPathComponent(const std::string &new_path);
-
+  void AppendPathComponent(llvm::StringRef component);
   void AppendPathComponent(const FileSpec &new_path);
 
   void RemoveLastPathComponent();
 
   ConstString GetLastPathComponent() const;
 
   //------------------------------------------------------------------
   /// Resolves the user name at the beginning of \a src_path, and writes the
   /// output
   /// to \a dst_path.  Note, \a src_path can contain other path components after
   /// the
   /// user name, they will be copied over, and if the path doesn't start with
   /// "~" it
   /// will also be copied over to \a dst_path.
   ///
   /// @param[in] src_path
   ///     Input path to be resolved.
   ///
   /// @param[in] dst_path
   ///     Buffer to store the resolved path.
   //------------------------------------------------------------------
   static void ResolveUsername(llvm::SmallVectorImpl<char> &path);
 
-  static size_t ResolvePartialUsername(const char *partial_name,
+  static size_t ResolvePartialUsername(llvm::StringRef partial_name,
                                        StringList &matches);
 
   enum EnumerateDirectoryResult {
     eEnumerateDirectoryResultNext,  // Enumerate next entry in the current
                                     // directory
     eEnumerateDirectoryResultEnter, // Recurse into the current entry if it is a
                                     // directory or symlink, or next if not
     eEnumerateDirectoryResultExit,  // Exit from the current directory at the
                                     // current level.
     eEnumerateDirectoryResultQuit   // Stop directory enumerations at any level
   };
 
   typedef EnumerateDirectoryResult (*EnumerateDirectoryCallbackType)(
       void *baton, FileType file_type, const FileSpec &spec);
 
   static EnumerateDirectoryResult
-  EnumerateDirectory(const char *dir_path, bool find_directories,
+  EnumerateDirectory(llvm::StringRef dir_path, bool find_directories,
                      bool find_files, bool find_other,
                      EnumerateDirectoryCallbackType callback,
                      void *callback_baton);
 
   typedef std::function<EnumerateDirectoryResult(FileType file_type,
                                                  const FileSpec &spec)>
       DirectoryCallback;
 
   static EnumerateDirectoryResult
-  ForEachItemInDirectory(const char *dir_path,
+  ForEachItemInDirectory(llvm::StringRef dir_path,
                          DirectoryCallback const &callback);
 
 protected:
   //------------------------------------------------------------------
   // Member variables
   //------------------------------------------------------------------
   ConstString m_directory;    ///< The uniqued directory path
   ConstString m_filename;     ///< The uniqued filename path

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

 #include "lldb/Core/Stream.h"
 #include "lldb/Core/StreamString.h"
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Utility/CleanUp.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 namespace {
 
 bool PathSyntaxIsPosix(FileSpec::PathSyntax syntax) {
   return (syntax == FileSpec::ePathSyntaxPosix ||
           (syntax == FileSpec::ePathSyntaxHostNative &&
            FileSystem::GetNativePathSyntax() == FileSpec::ePathSyntaxPosix));
 }
 
 const char *GetPathSeparators(FileSpec::PathSyntax syntax) {
   return PathSyntaxIsPosix(syntax) ? "/" : "\\/";
 }
 
-char GetPrefferedPathSeparator(FileSpec::PathSyntax syntax) {
+char GetPreferredPathSeparator(FileSpec::PathSyntax syntax) {
   return GetPathSeparators(syntax)[0];
 }
 
 bool IsPathSeparator(char value, FileSpec::PathSyntax syntax) {
   return value == '/' || (!PathSyntaxIsPosix(syntax) && value == '\\');
 }
 
 void Normalize(llvm::SmallVectorImpl<char> &path, FileSpec::PathSyntax syntax) {
if (user_entry != nullptr) {
     }
   } else {
     // Unable to resolve username (user doesn't exist?)
     path.clear();
   }
 #endif
 }
 
-size_t FileSpec::ResolvePartialUsername(const char *partial_name,
+size_t FileSpec::ResolvePartialUsername(llvm::StringRef partial_name,
                                         StringList &matches) {
 #ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER
   size_t extant_entries = matches.GetSize();
 
   setpwent();
   struct passwd *user_entry;
-  const char *name_start = partial_name + 1;
+  partial_name = partial_name.drop_front();
   std::set<std::string> name_list;
 
   while ((user_entry = getpwent()) != NULL) {
-    if (strstr(user_entry->pw_name, name_start) == user_entry->pw_name) {
+    if (llvm::StringRef(user_entry->pw_name).startswith(partial_name)) {
       std::string tmp_buf("~");
       tmp_buf.append(user_entry->pw_name);
       tmp_buf.push_back('/');
       name_list.insert(tmp_buf);
     }
   }
-  std::set<std::string>::iterator pos, end = name_list.end();
-  for (pos = name_list.begin(); pos != end; pos++) {
-    matches.AppendString((*pos).c_str());
+
+  for (auto &name : name_list) {
+    matches.AppendString(name);
   }
   return matches.GetSize() - extant_entries;
 #else
   // Resolving home directories is not supported, just copy the path...
   return 0;
 #endif // #ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER
 }
 

 FileSpec::FileSpec()
     : m_directory(), m_filename(), m_syntax(FileSystem::GetNativePathSyntax()) {
 }
 
 //------------------------------------------------------------------
 // Default constructor that can take an optional full path to a
 // file on disk.
 //------------------------------------------------------------------
-FileSpec::FileSpec(const char *pathname, bool resolve_path, PathSyntax syntax)
+FileSpec::FileSpec(llvm::StringRef path, bool resolve_path, PathSyntax syntax)
     : m_directory(), m_filename(), m_is_resolved(false), m_syntax(syntax) {
-  if (pathname && pathname[0])
-    SetFile(pathname, resolve_path, syntax);
+  SetFile(path, resolve_path, syntax);
 }
 
-FileSpec::FileSpec(const char *pathname, bool resolve_path, ArchSpec arch)
-    : FileSpec{pathname, resolve_path, arch.GetTriple().isOSWindows()
-                                           ? ePathSyntaxWindows
-                                           : ePathSyntaxPosix} {}
+FileSpec::FileSpec(llvm::StringRef path, bool resolve_path, ArchSpec arch)
+    : FileSpec{path, resolve_path, arch.GetTriple().isOSWindows()
 
-FileSpec::FileSpec(const std::string &path, bool resolve_path,
-                   PathSyntax syntax)
-    : FileSpec{path.c_str(), resolve_path, syntax} {}
-
-FileSpec::FileSpec(const std::string &path, bool resolve_path, ArchSpec arch)
-    : FileSpec{path.c_str(), resolve_path, arch} {}
-
 //------------------------------------------------------------------
 // Copy constructor
 //------------------------------------------------------------------
 FileSpec::FileSpec(const FileSpec &rhs)
     : m_directory(rhs.m_directory), m_filename(rhs.m_filename),
       m_is_resolved(rhs.m_is_resolved), m_syntax(rhs.m_syntax) {}
 
 //------------------------------------------------------------------
const FileSpec &FileSpec::operator=(const FileSpec &rhs) {
   return *this;
 }
 
 //------------------------------------------------------------------
 // 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(const char *pathname, bool resolve, PathSyntax syntax) {
+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)
                  ? FileSystem::GetNativePathSyntax()
                  : syntax;
 
-  if (pathname == NULL || pathname[0] == '\0')
+  if (pathname.empty())
     return;
 
   llvm::SmallString<64> resolved(pathname);
 
   if (resolve) {
     FileSpec::Resolve(resolved);
     m_is_resolved = true;
   }
while (filename_begin != llvm::StringRef::npos &&
          IsPathSeparator(resolve_path_ref[filename_begin], syntax))
     ++filename_begin;
   m_filename.SetString((filename_begin == llvm::StringRef::npos ||
                         filename_begin >= resolve_path_ref.size())
                            ? "."
                            : resolve_path_ref.substr(filename_begin));
 }
 
-void FileSpec::SetFile(const char *pathname, bool resolve, ArchSpec arch) {
-  return SetFile(pathname, resolve, arch.GetTriple().isOSWindows()
-                                        ? ePathSyntaxWindows
-                                        : ePathSyntaxPosix);
+void FileSpec::SetFile(llvm::StringRef path, bool resolve, ArchSpec arch) {
+  return SetFile(path, resolve, arch.GetTriple().isOSWindows()
 }
 
-void FileSpec::SetFile(const std::string &pathname, bool resolve,
-                       PathSyntax syntax) {
-  return SetFile(pathname.c_str(), resolve, syntax);
-}
-
-void FileSpec::SetFile(const std::string &pathname, bool resolve,
-                       ArchSpec arch) {
-  return SetFile(pathname.c_str(), resolve, arch);
-}
-
 //----------------------------------------------------------------------
 // Convert to pointer operator. This allows code to check any FileSpec
 // objects to see if they contain anything valid using code such as:
 //
 //  if (file_spec)
 //  {}
 //----------------------------------------------------------------------
 FileSpec::operator bool() const { return m_filename || m_directory; }

 // Dump the object to the supplied stream. If the object contains
 // a valid directory name, it will be displayed followed by a
 // directory delimiter, and the filename.
 //------------------------------------------------------------------
 void FileSpec::Dump(Stream *s) const {
   if (s) {
     std::string path{GetPath(true)};
     s->PutCString(path.c_str());
-    char path_separator = GetPrefferedPathSeparator(m_syntax);
+    char path_separator = GetPreferredPathSeparator(m_syntax);
     if (!m_filename && !path.empty() && path.back() != path_separator)
       s->PutChar(path_separator);
   }
 }
 
 //------------------------------------------------------------------
 // Returns true if the file exists.
 //------------------------------------------------------------------
 bool FileSpec::Exists() const {
   struct stat file_stats;
   return GetFileStats(this, &file_stats);
 }
 
 bool FileSpec::Readable() const {
   const uint32_t permissions = GetPermissions();
   if (permissions & eFilePermissionsEveryoneR)
     return true;
   return false;
 }
 
 bool FileSpec::ResolveExecutableLocation() {
+  // CLEANUP: Use StringRef for string handling.
   if (!m_directory) {
     const char *file_cstr = m_filename.GetCString();
     if (file_cstr) {
       const std::string file_str(file_cstr);
       llvm::ErrorOr<std::string> error_or_path =
           llvm::sys::findProgramByName(file_str);
       if (!error_or_path)
         return false;

 }
 
 void FileSpec::GetPath(llvm::SmallVectorImpl<char> &path,
                        bool denormalize) const {
   path.append(m_directory.GetStringRef().begin(),
               m_directory.GetStringRef().end());
   if (m_directory && m_filename &&
       !IsPathSeparator(m_directory.GetStringRef().back(), m_syntax))
-    path.insert(path.end(), GetPrefferedPathSeparator(m_syntax));
+    path.insert(path.end(), GetPreferredPathSeparator(m_syntax));
   path.append(m_filename.GetStringRef().begin(),
               m_filename.GetStringRef().end());
   Normalize(path, m_syntax);
   if (denormalize && !path.empty())
     Denormalize(path, m_syntax);
 }
 
 ConstString FileSpec::GetFileNameExtension() const {
if (file_stream) {
       while (getline(file_stream, line))
         lines.push_back(line);
     }
   }
   return lines.size();
 }
 
 FileSpec::EnumerateDirectoryResult
-FileSpec::ForEachItemInDirectory(const char *dir_path,
+FileSpec::ForEachItemInDirectory(llvm::StringRef dir_path,
                                  DirectoryCallback const &callback) {
-  if (dir_path && dir_path[0]) {
+  if (dir_path.empty())
+    return eEnumerateDirectoryResultNext;
+
 #ifdef _WIN32
     std::string szDir(dir_path);
     szDir += "\\*";
 
     std::wstring wszDir;
     if (!llvm::ConvertUTF8toWide(szDir, wszDir)) {
       return eEnumerateDirectoryResultNext;
     }
do {
         file_type = eFileTypeRegular;
       }
 
       std::string fileName;
       if (!llvm::convertWideToUTF8(ffd.cFileName, fileName)) {
         continue;
       }
 
-      std::vector<char> child_path(PATH_MAX);
-      const int child_path_len =
-          ::snprintf(child_path.data(), child_path.size(), "%s\\%s", dir_path,
-                     fileName.c_str());
-      if (child_path_len < (int)(child_path.size() - 1)) {
-        // Don't resolve the file type or path
-        FileSpec child_path_spec(child_path.data(), false);
+      std::string child_path = llvm::join_items("\\", dir_path, fileName);
 
-        EnumerateDirectoryResult result = callback(file_type, child_path_spec);
 
-        switch (result) {
-        case eEnumerateDirectoryResultNext:
-          // Enumerate next entry in the current directory. We just
-          // exit this switch and will continue enumerating the
-          // current directory as we currently are...
-          break;
 
-        case eEnumerateDirectoryResultEnter: // Recurse into the current entry
-                                             // if it is a directory or symlink,
-                                             // or next if not
-          if (FileSpec::ForEachItemInDirectory(child_path.data(), callback) ==
-              eEnumerateDirectoryResultQuit) {
-            // The subdirectory returned Quit, which means to
-            // stop all directory enumerations at all levels.
-            return eEnumerateDirectoryResultQuit;
-          }
-          break;
 
-        case eEnumerateDirectoryResultExit: // Exit from the current directory
-                                            // at the current level.
-          // Exit from this directory level and tell parent to
-          // keep enumerating.
-          return eEnumerateDirectoryResultNext;
 
-        case eEnumerateDirectoryResultQuit: // Stop directory enumerations at
-                                            // any level
-          return eEnumerateDirectoryResultQuit;
-        }
-      }
     } while (FindNextFileW(hFind, &ffd) != 0);
 
     FindClose(hFind);
 #else
-    lldb_utility::CleanUp<DIR *, int> dir_path_dir(opendir(dir_path), NULL,
-                                                   closedir);
-    if (dir_path_dir.is_valid()) {
-      char dir_path_last_char = dir_path[strlen(dir_path) - 1];
+  std::string dir_string(dir_path);
+  lldb_utility::CleanUp<DIR *, int> dir_path_dir(opendir(dir_string.c_str()),
+                                                 NULL, closedir);
+    char dir_path_last_char = dir_path.back();
 
-      long path_max = fpathconf(dirfd(dir_path_dir.get()), _PC_NAME_MAX);
 #if defined(__APPLE_) && defined(__DARWIN_MAXPATHLEN)
       if (path_max < __DARWIN_MAXPATHLEN)
         path_max = __DARWIN_MAXPATHLEN;
 #endif
       struct dirent *buf, *dp;
       buf = (struct dirent *)malloc(offsetof(struct dirent, d_name) + path_max +
                                     1);
 
#endif
           break;
 #if !defined(__OpenBSD__)
         case DT_WHT:
           file_type = eFileTypeOther;
           break;
 #endif
         }
 
-        char child_path[PATH_MAX];
-
+        std::string child_path;
         // Don't make paths with "/foo//bar", that just confuses everybody.
-        int child_path_len;
         if (dir_path_last_char == '/')
-          child_path_len = ::snprintf(child_path, sizeof(child_path), "%s%s",
-                                      dir_path, dp->d_name);
+          child_path = llvm::join_items("", dir_path, dp->d_name);
         else
-          child_path_len = ::snprintf(child_path, sizeof(child_path), "%s/%s",
-                                      dir_path, dp->d_name);
+          child_path = llvm::join_items('/', dir_path, dp->d_name);
 
-        if (child_path_len < (int)(sizeof(child_path) - 1)) {
           // Don't resolve the file type or path
           FileSpec child_path_spec(child_path, false);
 
           EnumerateDirectoryResult result =
               callback(file_type, child_path_spec);
 
           switch (result) {
           case eEnumerateDirectoryResultNext:
#endif
             return eEnumerateDirectoryResultNext;
 
           case eEnumerateDirectoryResultQuit: // Stop directory enumerations at
                                               // any level
             if (buf)
               free(buf);
             return eEnumerateDirectoryResultQuit;
           }
-        }
-      }
       if (buf) {
         free(buf);
       }
     }
 #endif
-  }
   // By default when exiting a directory, we tell the parent enumeration
   // to continue enumerating.
   return eEnumerateDirectoryResultNext;
 }
 
 FileSpec::EnumerateDirectoryResult
-FileSpec::EnumerateDirectory(const char *dir_path, bool find_directories,
+FileSpec::EnumerateDirectory(llvm::StringRef dir_path, bool find_directories,
                              bool find_files, bool find_other,
                              EnumerateDirectoryCallbackType callback,
                              void *callback_baton) {
   return ForEachItemInDirectory(
       dir_path,
       [&find_directories, &find_files, &find_other, &callback,
        &callback_baton](FileType file_type, const FileSpec &file_spec) {
         switch (file_type) {
return ForEachItemInDirectory(
           if (find_other)
             return callback(callback_baton, file_type, file_spec);
           break;
         }
         return eEnumerateDirectoryResultNext;
       });
 }
 
-FileSpec FileSpec::CopyByAppendingPathComponent(const char *new_path) const {
+FileSpec
+FileSpec::CopyByAppendingPathComponent(llvm::StringRef component) const {
   FileSpec ret = *this;
-  ret.AppendPathComponent(new_path);
+  ret.AppendPathComponent(component);
   return ret;
 }
 
 FileSpec FileSpec::CopyByRemovingLastPathComponent() const {
+  // CLEANUP: Use StringRef for string handling.
   const bool resolve = false;
   if (m_filename.IsEmpty() && m_directory.IsEmpty())
     return FileSpec("", resolve);
   if (m_directory.IsEmpty())
     return FileSpec("", resolve);
   if (m_filename.IsEmpty()) {
     const char *dir_cstr = m_directory.GetCString();
     const char *last_slash_ptr = ::strrchr(dir_cstr, '/');
 
     // check for obvious cases before doing the full thing
     if (!last_slash_ptr)
       return FileSpec("", resolve);
     if (last_slash_ptr == dir_cstr)
       return FileSpec("/", resolve);
 
     size_t last_slash_pos = last_slash_ptr - dir_cstr + 1;
     ConstString new_path(dir_cstr, last_slash_pos);
     return FileSpec(new_path.GetCString(), resolve);
   } else
     return FileSpec(m_directory.GetCString(), resolve);
 }
 
 ConstString FileSpec::GetLastPathComponent() const {
+  // CLEANUP: Use StringRef for string handling.
   if (m_filename)
     return m_filename;
   if (m_directory) {
     const char *dir_cstr = m_directory.GetCString();
     const char *last_slash_ptr = ::strrchr(dir_cstr, '/');
     if (last_slash_ptr == NULL)
       return m_directory;
     if (last_slash_ptr == dir_cstr) {
if (m_directory) {
     }
     ConstString result(penultimate_slash_ptr + 1,
                        last_slash_ptr - penultimate_slash_ptr);
     return result;
   }
   return ConstString();
 }
 
-void FileSpec::PrependPathComponent(const char *new_path) {
-  if (!new_path)
+void FileSpec::PrependPathComponent(llvm::StringRef component) {
+  if (component.empty())
     return;
+
   const bool resolve = false;
   if (m_filename.IsEmpty() && m_directory.IsEmpty()) {
-    SetFile(new_path, resolve);
+    SetFile(component, resolve);
     return;
   }
-  StreamString stream;
+
+  char sep = GetPreferredPathSeparator(m_syntax);
+  std::string result;
   if (m_filename.IsEmpty())
-    stream.Printf("%s/%s", new_path, m_directory.GetCString());
+    result = llvm::join_items(sep, component, m_directory.GetStringRef());
   else if (m_directory.IsEmpty())
-    stream.Printf("%s/%s", new_path, m_filename.GetCString());
+    result = llvm::join_items(sep, component, m_filename.GetStringRef());
   else
-    stream.Printf("%s/%s/%s", new_path, m_directory.GetCString(),
-                  m_filename.GetCString());
-  SetFile(stream.GetData(), resolve);
-}
+    result = llvm::join_items(sep, component, m_directory.GetStringRef(),
+                              m_filename.GetStringRef());
 
-void FileSpec::PrependPathComponent(const std::string &new_path) {
-  return PrependPathComponent(new_path.c_str());
+  SetFile(result, resolve);
 }
 
 void FileSpec::PrependPathComponent(const FileSpec &new_path) {
   return PrependPathComponent(new_path.GetPath(false));
 }
 
-void FileSpec::AppendPathComponent(const char *new_path) {
-  if (!new_path)
+void FileSpec::AppendPathComponent(llvm::StringRef component) {
+  if (component.empty())
     return;
 
-  StreamString stream;
+  std::string result;
   if (!m_directory.IsEmpty()) {
-    stream.PutCString(m_directory.GetCString());
+    result += m_directory.GetStringRef();
     if (!IsPathSeparator(m_directory.GetStringRef().back(), m_syntax))
-      stream.PutChar(GetPrefferedPathSeparator(m_syntax));
+      result += GetPreferredPathSeparator(m_syntax);
   }
 
   if (!m_filename.IsEmpty()) {
-    stream.PutCString(m_filename.GetCString());
+    result += m_filename.GetStringRef();
     if (!IsPathSeparator(m_filename.GetStringRef().back(), m_syntax))
-      stream.PutChar(GetPrefferedPathSeparator(m_syntax));
+      result += GetPreferredPathSeparator(m_syntax);
   }
 
-  while (IsPathSeparator(new_path[0], m_syntax))
-    new_path++;
-
-  stream.PutCString(new_path);
+  component = component.drop_while(
+      [this](char c) { return IsPathSeparator(c, m_syntax); });
 
-  const bool resolve = false;
-  SetFile(stream.GetData(), resolve, m_syntax);
-}
+  result += component;
 
-void FileSpec::AppendPathComponent(const std::string &new_path) {
-  return AppendPathComponent(new_path.c_str());
+  SetFile(result, false, m_syntax);
 }
 
 void FileSpec::AppendPathComponent(const FileSpec &new_path) {
   return AppendPathComponent(new_path.GetPath(false));
 }
 
 void FileSpec::RemoveLastPathComponent() {
+  // CLEANUP: Use StringRef for string handling.
+
   const bool resolve = false;
   if (m_filename.IsEmpty() && m_directory.IsEmpty()) {
     SetFile("", resolve);
     return;
   }
   if (m_directory.IsEmpty()) {
     SetFile("", resolve);
     return;