hintonda updated this revision to Diff 121214.
hintonda added a comment.

Addressed @clayborg's comments.

- Added FileSpec ctor that takes an explicit RegularExpression object when 
using FileSpec as a regex pattern.

- Removed the RegularExpression member variable, added a flag, and create the 
regex on demand in operator==().

- Added type to PathSyntax enum, and reordered member variables from, largest 
to smallest (evens thought the last three are now the same size) to make sure 
sizeof(FileSpec) doesn't grow with this change.

- Removed implementation of special methods that had default behavior, and 
declared the default ctor '= default', which should reduce the maintenance 
burden.

Next steps:

- Create SBRegularExpression and add ctor to SBFileSpec that takes it as 
parameter.  That will allow this use case without any other changes:

  regex = lldb.SBRegularExpression(".*/some/path/.*") 
target.BreakpointCreateBySourceRegex(name, lldb.SBFileSpec(regex))

- Then wire up SBFileSpec to do the right thing.  SBRegularExpression can 
validate the expression on construction to give immediate feedback.


https://reviews.llvm.org/D39436

Files:
  include/lldb/Utility/FileSpec.h
  source/Commands/CommandObjectBreakpoint.cpp
  source/Utility/FileSpec.cpp

Index: source/Utility/FileSpec.cpp
===================================================================
--- source/Utility/FileSpec.cpp
+++ source/Utility/FileSpec.cpp
@@ -164,8 +164,6 @@
   }
 }
 
-FileSpec::FileSpec() : m_syntax(GetNativeSyntax()) {}
-
 //------------------------------------------------------------------
 // Default constructor that can take an optional full path to a
 // file on disk.
@@ -180,12 +178,12 @@
     : FileSpec{path, resolve_path,
                Triple.isOSWindows() ? ePathSyntaxWindows : ePathSyntaxPosix} {}
 
-//------------------------------------------------------------------
-// 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) {}
+FileSpec::FileSpec(RegularExpression regex) {
+  if(regex.IsValid()) {
+    m_filename.SetString(regex.GetText());
+    m_is_regex = true;
+  }
+}
 
 //------------------------------------------------------------------
 // Copy constructor
@@ -196,24 +194,6 @@
 }
 
 //------------------------------------------------------------------
-// Virtual destructor in case anyone inherits from this class.
-//------------------------------------------------------------------
-FileSpec::~FileSpec() {}
-
-//------------------------------------------------------------------
-// Assignment operator.
-//------------------------------------------------------------------
-const FileSpec &FileSpec::operator=(const FileSpec &rhs) {
-  if (this != &rhs) {
-    m_directory = rhs.m_directory;
-    m_filename = rhs.m_filename;
-    m_is_resolved = rhs.m_is_resolved;
-    m_syntax = rhs.m_syntax;
-  }
-  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.
@@ -301,6 +281,12 @@
 // Equal to operator
 //------------------------------------------------------------------
 bool FileSpec::operator==(const FileSpec &rhs) const {
+  if(m_is_regex) {
+    RegularExpression regex(m_filename.GetStringRef());
+    if (regex.IsValid())
+      return regex.Execute(rhs.GetPath());
+  }
+
   if (!FileEquals(rhs))
     return false;
   if (DirectoryEquals(rhs))
@@ -411,6 +397,9 @@
 
 bool FileSpec::Equal(const FileSpec &a, const FileSpec &b, bool full,
                      bool remove_backups) {
+  if (a.m_is_regex)
+    return a == b;
+
   static ConstString g_dot_string(".");
   static ConstString g_dot_dot_string("..");
 
Index: source/Commands/CommandObjectBreakpoint.cpp
===================================================================
--- source/Commands/CommandObjectBreakpoint.cpp
+++ source/Commands/CommandObjectBreakpoint.cpp
@@ -256,6 +256,8 @@
   { LLDB_OPT_NOT_10,               false, "shlib",                  's', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eModuleCompletion,     eArgTypeShlibName,           "Set the breakpoint only in this shared library.  Can repeat this option "
   "multiple times to specify multiple shared libraries." },
   { LLDB_OPT_SET_ALL,              false, "hardware",               'H', OptionParser::eNoArgument,       nullptr, nullptr, 0,                                         eArgTypeNone,                "Require the breakpoint to use hardware breakpoints." },
+  { LLDB_OPT_FILE,                 false, "source-file-regex",      'z', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeRegularExpression,   "Only files matching pattern." },
+  { LLDB_OPT_FILE,                 false, "module-regex",           'Z', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeRegularExpression,   "Only modules matching pattern." },
   { LLDB_OPT_FILE,                 false, "file",                   'f', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeFilename,            "Specifies the source file in which to set this breakpoint.  Note, by default "
   "lldb only looks for files that are #included if they use the standard include "
   "file extensions.  To set breakpoints on .c/.cpp/.m/.mm files that are "
@@ -560,6 +562,14 @@
         m_source_regex_func_names.insert(option_arg);
         break;
 
+      case 'z':
+        m_filenames.AppendIfUnique(FileSpec(RegularExpression(option_arg)));
+        break;
+
+      case 'Z':
+        m_modules.AppendIfUnique(FileSpec(RegularExpression(option_arg)));
+        break;
+
       default:
         error.SetErrorStringWithFormat("unrecognized option '%c'",
                                        short_option);
Index: include/lldb/Utility/FileSpec.h
===================================================================
--- include/lldb/Utility/FileSpec.h
+++ include/lldb/Utility/FileSpec.h
@@ -18,6 +18,7 @@
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/RegularExpression.h"
 
 #include "llvm/ADT/StringRef.h" // for StringRef
 #include "llvm/Support/FileSystem.h"
@@ -61,14 +62,12 @@
 //----------------------------------------------------------------------
 class FileSpec {
 public:
-  enum PathSyntax {
+  enum PathSyntax : unsigned char {
     ePathSyntaxPosix,
     ePathSyntaxWindows,
     ePathSyntaxHostNative
   };
 
-  FileSpec();
-
   //------------------------------------------------------------------
   /// Constructor with path.
   ///
@@ -81,27 +80,18 @@
   ///
   /// @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.
+  ///     forth, if \b false we trust the path is in canonical form already.
   ///
   /// @see FileSpec::SetFile (const char *path, bool resolve)
   //------------------------------------------------------------------
-  explicit FileSpec(llvm::StringRef path, bool resolve_path,
-                    PathSyntax syntax = ePathSyntaxHostNative);
+  FileSpec(llvm::StringRef path, bool resolve_path,
+           PathSyntax syntax = ePathSyntaxHostNative);
 
-  explicit FileSpec(llvm::StringRef path, bool resolve_path,
-                    const llvm::Triple &Triple);
+  FileSpec(llvm::StringRef path, bool resolve_path, const llvm::Triple &Triple);
 
-  //------------------------------------------------------------------
-  /// Copy constructor
-  ///
-  /// Makes a copy of the uniqued directory and filename strings from
-  /// \a rhs.
-  ///
-  /// @param[in] rhs
-  ///     A const FileSpec object reference to copy.
-  //------------------------------------------------------------------
-  FileSpec(const FileSpec &rhs);
+  explicit FileSpec(RegularExpression regex);
+
+  FileSpec() = default;
 
   //------------------------------------------------------------------
   /// Copy constructor
@@ -114,30 +104,11 @@
   //------------------------------------------------------------------
   FileSpec(const FileSpec *rhs);
 
-  //------------------------------------------------------------------
-  /// Destructor.
-  //------------------------------------------------------------------
-  ~FileSpec();
-
   bool DirectoryEquals(const FileSpec &other) const;
 
   bool FileEquals(const FileSpec &other) const;
 
   //------------------------------------------------------------------
-  /// Assignment operator.
-  ///
-  /// Makes a copy of the uniqued directory and filename strings from
-  /// \a rhs.
-  ///
-  /// @param[in] rhs
-  ///     A const FileSpec object reference to assign to this object.
-  ///
-  /// @return
-  ///     A const reference to this object.
-  //------------------------------------------------------------------
-  const FileSpec &operator=(const FileSpec &rhs);
-
-  //------------------------------------------------------------------
   /// Equal to operator
   ///
   /// Tests if this object is equal to \a rhs.
@@ -580,9 +551,10 @@
   //------------------------------------------------------------------
   ConstString m_directory;            ///< The uniqued directory path
   ConstString m_filename;             ///< The uniqued filename path
+  PathSyntax m_syntax = ePathSyntaxHostNative; ///< The syntax that this path
+                                               ///< uses (e.g. Windows / Posix)
   mutable bool m_is_resolved = false; ///< True if this path has been resolved.
-  PathSyntax
-      m_syntax; ///< The syntax that this path uses (e.g. Windows / Posix)
+  bool m_is_regex = false;            ///< Filename is a regular expression.
 };
 
 //----------------------------------------------------------------------
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to