Re: [Lldb-commits] [PATCH] D24936: Make FileSpec use StringRef.

2016-09-27 Thread Zachary Turner via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL282537: Update FileSpec's interface to use StringRefs. 
(authored by zturner).

Changed prior to commit:
  https://reviews.llvm.org/D24936?vs=72555=72705#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24936

Files:
  lldb/trunk/include/lldb/Host/FileSpec.h
  lldb/trunk/source/Host/common/FileSpec.cpp

Index: lldb/trunk/include/lldb/Host/FileSpec.h
===
--- lldb/trunk/include/lldb/Host/FileSpec.h
+++ lldb/trunk/include/lldb/Host/FileSpec.h
@@ -80,16 +80,10 @@
   ///
   /// @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 , bool resolve_path,
-PathSyntax syntax = ePathSyntaxHostNative);
-
-  explicit FileSpec(const std::string , bool resolve_path, ArchSpec arch);
+  explicit FileSpec(llvm::StringRef path, bool resolve_path, ArchSpec arch);
 
   //--
   /// Copy constructor
@@ -657,15 +651,10 @@
   /// 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 , bool resolve_path,
+  void SetFile(llvm::StringRef path, bool resolve_path,
PathSyntax syntax = ePathSyntaxHostNative);
 
-  void SetFile(const std::string , bool resolve_path, ArchSpec arch);
+  void SetFile(llvm::StringRef path, bool resolve_path, ArchSpec arch);
 
   bool IsResolved() const { return m_is_resolved; }
 
@@ -709,20 +698,13 @@
   //--
   static void Resolve(llvm::SmallVectorImpl );
 
-  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 _path);
-
+  void PrependPathComponent(llvm::StringRef component);
   void PrependPathComponent(const FileSpec _path);
 
-  void AppendPathComponent(const char *new_path);
-
-  void AppendPathComponent(const std::string _path);
-
+  void AppendPathComponent(llvm::StringRef component);
   void AppendPathComponent(const FileSpec _path);
 
   void RemoveLastPathComponent();
@@ -746,7 +728,7 @@
   //--
   static void ResolveUsername(llvm::SmallVectorImpl );
 
-  static size_t ResolvePartialUsername(const char *partial_name,
+  static size_t ResolvePartialUsername(llvm::StringRef partial_name,
StringList );
 
   enum EnumerateDirectoryResult {
@@ -763,7 +745,7 @@
   void *baton, FileType file_type, const FileSpec );
 
   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);
@@ -773,7 +755,7 @@
   DirectoryCallback;
 
   static EnumerateDirectoryResult
-  ForEachItemInDirectory(const char *dir_path,
+  ForEachItemInDirectory(llvm::StringRef dir_path,
  DirectoryCallback const );
 
 protected:
Index: lldb/trunk/source/Host/common/FileSpec.cpp
===
--- lldb/trunk/source/Host/common/FileSpec.cpp
+++ lldb/trunk/source/Host/common/FileSpec.cpp
@@ -37,6 +37,7 @@
 #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"
@@ -58,7 +59,7 @@
   return PathSyntaxIsPosix(syntax) ? "/" : "\\/";
 }
 
-char GetPrefferedPathSeparator(FileSpec::PathSyntax syntax) {
+char GetPreferredPathSeparator(FileSpec::PathSyntax syntax) {
   return GetPathSeparators(syntax)[0];
 }
 
@@ -228,27 +229,27 @@
 #endif
 }
 
-size_t FileSpec::ResolvePartialUsername(const char *partial_name,
+size_t FileSpec::ResolvePartialUsername(llvm::StringRef partial_name,

Re: [Lldb-commits] [PATCH] D24936: Make FileSpec use StringRef.

2016-09-26 Thread Greg Clayton via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

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.


https://reviews.llvm.org/D24936



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D24936: Make FileSpec use StringRef.

2016-09-26 Thread Zachary Turner via lldb-commits
zturner created this revision.
zturner added a reviewer: clayborg.
zturner added a subscriber: lldb-commits.
zturner added a dependency: D24880: Add StringExtras join_items function.

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



https://reviews.llvm.org/D24936

Files:
  include/lldb/Host/FileSpec.h
  source/Host/common/FileSpec.cpp

Index: source/Host/common/FileSpec.cpp
===
--- source/Host/common/FileSpec.cpp
+++ source/Host/common/FileSpec.cpp
@@ -37,6 +37,7 @@
 #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"
@@ -228,27 +229,27 @@
 #endif
 }
 
-size_t FileSpec::ResolvePartialUsername(const char *partial_name,
+size_t FileSpec::ResolvePartialUsername(llvm::StringRef partial_name,
 StringList ) {
 #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 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::iterator pos, end = name_list.end();
-  for (pos = name_list.begin(); pos != end; pos++) {
-matches.AppendString((*pos).c_str());
+
+  for (auto  : name_list) {
+matches.AppendString(name);
   }
   return matches.GetSize() - extant_entries;
 #else
@@ -284,23 +285,15 @@
 // 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(const std::string , bool resolve_path,
-   PathSyntax syntax)
-: FileSpec{path.c_str(), resolve_path, syntax} {}
-
-FileSpec::FileSpec(const std::string , bool resolve_path, ArchSpec arch)
-: FileSpec{path.c_str(), resolve_path, arch} {}
+FileSpec::FileSpec(llvm::StringRef path, bool resolve_path, ArchSpec arch)
+: FileSpec{path, resolve_path, arch.GetTriple().isOSWindows()
+   ? ePathSyntaxWindows
+   : ePathSyntaxPosix} {}
 
 //--
 // Copy constructor
@@ -340,7 +333,12 @@
 // 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;
@@ -348,7 +346,7 @@
  ? FileSystem::GetNativePathSyntax()
  : syntax;
 
-  if (pathname == NULL || pathname[0] == '\0')
+  if (pathname.empty())
 return;
 
   llvm::SmallString<64> resolved(pathname);
@@ -382,20 +380,10 @@
: 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(const std::string , bool resolve,
-   PathSyntax syntax) {
-  return SetFile(pathname.c_str(), resolve, syntax);
-}
-
-void FileSpec::SetFile(const std::string , bool resolve,
-