jankratochvil updated this revision to Diff 215794.
jankratochvil retitled this revision from "Fix `TestDataFormatterStdList` 
regression + D66174 follow-up/cleanup" to "D66174 `RegularExpression` 
follow-up/cleanup".
jankratochvil edited the summary of this revision.

Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66392/new/

https://reviews.llvm.org/D66392

Files:
  lldb/include/lldb/Interpreter/OptionValueRegex.h
  lldb/include/lldb/Utility/RegularExpression.h
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Core/AddressResolverName.cpp
  lldb/source/Interpreter/CommandObjectRegexCommand.cpp
  lldb/source/Interpreter/OptionValueRegex.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Target/ThreadPlanStepInRange.cpp
  lldb/source/Utility/RegularExpression.cpp
  llvm/include/llvm/Support/Regex.h

Index: llvm/include/llvm/Support/Regex.h
===================================================================
--- llvm/include/llvm/Support/Regex.h
+++ llvm/include/llvm/Support/Regex.h
@@ -58,6 +58,9 @@
     /// matching, if any.
     bool isValid(std::string &Error) const;
 
+    // Return whether constructor has been called with no parameters.
+    bool empty() const { return preg == nullptr; }
+
     /// getNumMatches - In a valid regex, return the number of parenthesized
     /// matches it contains.  The number filled in by match will include this
     /// many entries plus one for the whole regex (as element 0).
Index: lldb/source/Utility/RegularExpression.cpp
===================================================================
--- lldb/source/Utility/RegularExpression.cpp
+++ lldb/source/Utility/RegularExpression.cpp
@@ -12,26 +12,43 @@
 
 using namespace lldb_private;
 
-RegularExpression::RegularExpression(llvm::StringRef str) { Compile(str); }
+RegularExpression::RegularExpression(llvm::StringRef str) {
+  m_regex_text = str;
+}
 
+// m_regex must not be copied as it contains pointers to RHS m_regex_text.
 RegularExpression::RegularExpression(const RegularExpression &rhs)
-    : RegularExpression() {
-  Compile(rhs.GetText());
+    : RegularExpression(rhs.m_regex_text) {}
+
+// m_regex must not be moved as it contains pointers to RHS m_regex_text.
+RegularExpression::RegularExpression(RegularExpression &&rhs)
+    : RegularExpression(std::move(rhs.m_regex_text)) {}
+
+// m_regex must not be copied as it contains pointers to RHS m_regex_text.
+RegularExpression &RegularExpression::operator=(const RegularExpression &rhs) {
+  m_regex_text = rhs.m_regex_text;
+  m_regex = llvm::Regex();
+  return *this;
 }
 
-bool RegularExpression::Compile(llvm::StringRef str) {
-  m_regex_text = str;
-  m_regex = llvm::Regex(str);
-  return IsValid();
+// m_regex must not be moved as it contains pointers to RHS m_regex_text.
+RegularExpression &RegularExpression::operator=(RegularExpression &&rhs) {
+  m_regex_text = std::move(rhs.m_regex_text);
+  m_regex = llvm::Regex();
+  return *this;
 }
 
 bool RegularExpression::Execute(
     llvm::StringRef str,
     llvm::SmallVectorImpl<llvm::StringRef> *matches) const {
+  if (!IsValid())
+    return false;
   return m_regex.match(str, matches);
 }
 
 bool RegularExpression::IsValid() const {
+  if (m_regex.empty())
+    m_regex = llvm::Regex(m_regex_text);
   std::string discarded;
   return m_regex.isValid(discarded);
 }
@@ -39,6 +56,8 @@
 llvm::StringRef RegularExpression::GetText() const { return m_regex_text; }
 
 llvm::Error RegularExpression::GetError() const {
+  if (m_regex.empty())
+    m_regex = llvm::Regex(m_regex_text);
   std::string error;
   if (!m_regex.isValid(error))
     return llvm::make_error<llvm::StringError>(llvm::inconvertibleErrorCode(),
Index: lldb/source/Target/ThreadPlanStepInRange.cpp
===================================================================
--- lldb/source/Target/ThreadPlanStepInRange.cpp
+++ lldb/source/Target/ThreadPlanStepInRange.cpp
@@ -315,8 +315,8 @@
   auto name_ref = llvm::StringRef::withNullAsEmpty(name);
   if (!m_avoid_regexp_up)
     m_avoid_regexp_up.reset(new RegularExpression(name_ref));
-
-  m_avoid_regexp_up->Compile(name_ref);
+  else
+    *m_avoid_regexp_up = RegularExpression(name_ref);
 }
 
 void ThreadPlanStepInRange::SetDefaultFlagValue(uint32_t new_value) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -332,11 +332,12 @@
             std::string regex_str = "^";
             regex_str += echo_packet;
             regex_str += "$";
-            response_regex.Compile(regex_str);
+            response_regex = RegularExpression(regex_str);
           } else {
             echo_packet_len =
                 ::snprintf(echo_packet, sizeof(echo_packet), "qC");
-            response_regex.Compile(llvm::StringRef("^QC[0-9A-Fa-f]+$"));
+            response_regex =
+                RegularExpression(llvm::StringRef("^QC[0-9A-Fa-f]+$"));
           }
 
           PacketResult echo_packet_result =
Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -446,15 +446,21 @@
   llvm::SmallVector<llvm::StringRef, 4> matches;
 
   bool matched = false;
-  if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+),([0-9]+)$")) &&
-      regex.Execute(coord_s, &matches))
-    matched = true;
-  else if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+)$")) &&
-           regex.Execute(coord_s, &matches))
-    matched = true;
-  else if (regex.Compile(llvm::StringRef("^([0-9]+)$")) &&
-           regex.Execute(coord_s, &matches))
-    matched = true;
+  if (!matched) {
+    regex = RegularExpression(llvm::StringRef("^([0-9]+),([0-9]+),([0-9]+)$"));
+    if (regex.Execute(coord_s, &matches))
+      matched = true;
+  }
+  if (!matched) {
+    regex = RegularExpression(llvm::StringRef("^([0-9]+),([0-9]+)$"));
+    if (regex.Execute(coord_s, &matches))
+      matched = true;
+  }
+  if (!matched) {
+    regex = RegularExpression(llvm::StringRef("^([0-9]+)$"));
+    if (regex.Execute(coord_s, &matches))
+      matched = true;
+  }
 
   if (!matched)
     return false;
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -582,9 +582,9 @@
     case 0:
       break;
     case 1: {
-      regex_up.reset(new RegularExpression());
-      if (!regex_up->Compile(llvm::StringRef::withNullAsEmpty(
-              command.GetArgumentAtIndex(0)))) {
+      regex_up.reset(new RegularExpression(
+          llvm::StringRef::withNullAsEmpty(command.GetArgumentAtIndex(0))));
+      if (!regex_up->IsValid()) {
         result.AppendError(
             "invalid argument - please provide a valid regular expression");
         result.SetStatus(lldb::eReturnStatusFailed);
Index: lldb/source/Interpreter/OptionValueRegex.cpp
===================================================================
--- lldb/source/Interpreter/OptionValueRegex.cpp
+++ lldb/source/Interpreter/OptionValueRegex.cpp
@@ -46,7 +46,8 @@
 
   case eVarSetOperationReplace:
   case eVarSetOperationAssign:
-    if (m_regex.Compile(value)) {
+    m_regex = RegularExpression(value);
+    if (m_regex.IsValid()) {
       m_value_was_set = true;
       NotifyValueChanged();
     } else if (llvm::Error err = m_regex.GetError()) {
Index: lldb/source/Interpreter/CommandObjectRegexCommand.cpp
===================================================================
--- lldb/source/Interpreter/CommandObjectRegexCommand.cpp
+++ lldb/source/Interpreter/CommandObjectRegexCommand.cpp
@@ -73,8 +73,9 @@
                                                 const char *command_cstr) {
   m_entries.resize(m_entries.size() + 1);
   // Only add the regular expression if it compiles
-  if (m_entries.back().regex.Compile(
-          llvm::StringRef::withNullAsEmpty(re_cstr))) {
+  m_entries.back().regex =
+      RegularExpression(llvm::StringRef::withNullAsEmpty(re_cstr));
+  if (m_entries.back().regex.IsValid()) {
     m_entries.back().command.assign(command_cstr);
     return true;
   }
Index: lldb/source/Core/AddressResolverName.cpp
===================================================================
--- lldb/source/Core/AddressResolverName.cpp
+++ lldb/source/Core/AddressResolverName.cpp
@@ -36,7 +36,8 @@
     : AddressResolver(), m_func_name(func_name), m_class_name(nullptr),
       m_regex(), m_match_type(type) {
   if (m_match_type == AddressResolver::Regexp) {
-    if (!m_regex.Compile(m_func_name.GetStringRef())) {
+    m_regex = RegularExpression(m_func_name.GetStringRef());
+    if (!m_regex.IsValid()) {
       Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
 
       if (log)
Index: lldb/source/Commands/CommandObjectType.cpp
===================================================================
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -692,8 +692,8 @@
 
       ConstString typeCS(arg_entry.ref);
       if (m_command_options.m_regex) {
-        RegularExpressionSP typeRX(new RegularExpression());
-        if (!typeRX->Compile(arg_entry.ref)) {
+        RegularExpressionSP typeRX(new RegularExpression(arg_entry.ref));
+        if (!typeRX->IsValid()) {
           result.AppendError(
               "regex format error (maybe this is not really a regex?)");
           result.SetStatus(eReturnStatusFailed);
@@ -1044,9 +1044,9 @@
     std::unique_ptr<RegularExpression> formatter_regex;
 
     if (m_options.m_category_regex.OptionWasSet()) {
-      category_regex.reset(new RegularExpression());
-      if (!category_regex->Compile(
-              m_options.m_category_regex.GetCurrentValueAsRef())) {
+      category_regex.reset(new RegularExpression(
+          m_options.m_category_regex.GetCurrentValueAsRef()));
+      if (!category_regex->IsValid()) {
         result.AppendErrorWithFormat(
             "syntax error in category regular expression '%s'",
             m_options.m_category_regex.GetCurrentValueAsRef().str().c_str());
@@ -1057,8 +1057,9 @@
 
     if (argc == 1) {
       const char *arg = command.GetArgumentAtIndex(0);
-      formatter_regex.reset(new RegularExpression());
-      if (!formatter_regex->Compile(llvm::StringRef::withNullAsEmpty(arg))) {
+      formatter_regex.reset(
+          new RegularExpression(llvm::StringRef::withNullAsEmpty(arg)));
+      if (!formatter_regex->IsValid()) {
         result.AppendErrorWithFormat("syntax error in regular expression '%s'",
                                      arg);
         result.SetStatus(eReturnStatusFailed);
@@ -1630,8 +1631,8 @@
   }
 
   if (type == eRegexSummary) {
-    RegularExpressionSP typeRX(new RegularExpression());
-    if (!typeRX->Compile(type_name.GetStringRef())) {
+    RegularExpressionSP typeRX(new RegularExpression(type_name.GetStringRef()));
+    if (!typeRX->IsValid()) {
       if (error)
         error->SetErrorString(
             "regex format error (maybe this is not really a regex?)");
@@ -2116,9 +2117,9 @@
     std::unique_ptr<RegularExpression> regex;
 
     if (argc == 1) {
-      regex.reset(new RegularExpression());
       const char *arg = command.GetArgumentAtIndex(0);
-      if (!regex->Compile(llvm::StringRef::withNullAsEmpty(arg))) {
+      regex.reset(new RegularExpression(llvm::StringRef::withNullAsEmpty(arg)));
+      if (!regex->IsValid()) {
         result.AppendErrorWithFormat(
             "syntax error in category regular expression '%s'", arg);
         result.SetStatus(eReturnStatusFailed);
@@ -2370,8 +2371,8 @@
   }
 
   if (type == eRegexSynth) {
-    RegularExpressionSP typeRX(new RegularExpression());
-    if (!typeRX->Compile(type_name.GetStringRef())) {
+    RegularExpressionSP typeRX(new RegularExpression(type_name.GetStringRef()));
+    if (!typeRX->IsValid()) {
       if (error)
         error->SetErrorString(
             "regex format error (maybe this is not really a regex?)");
@@ -2498,8 +2499,8 @@
     }
 
     if (type == eRegexFilter) {
-      RegularExpressionSP typeRX(new RegularExpression());
-      if (!typeRX->Compile(type_name.GetStringRef())) {
+      RegularExpressionSP typeRX(new RegularExpression(type_name.GetStringRef()));
+      if (!typeRX->IsValid()) {
         if (error)
           error->SetErrorString(
               "regex format error (maybe this is not really a regex?)");
Index: lldb/source/Commands/CommandObjectFrame.cpp
===================================================================
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -534,7 +534,7 @@
             const size_t regex_start_index = regex_var_list.GetSize();
             llvm::StringRef name_str = entry.ref;
             RegularExpression regex(name_str);
-            if (regex.Compile(name_str)) {
+            if (regex.IsValid()) {
               size_t num_matches = 0;
               const size_t num_new_regex_vars =
                   variable_list->AppendVariablesIfUnique(regex, regex_var_list,
Index: lldb/source/Commands/CommandCompletions.cpp
===================================================================
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -448,7 +448,7 @@
     pos = regex_str.insert(pos, '\\');
     pos = find_if(pos + 2, regex_str.end(), regex_chars);
   }
-  m_regex.Compile(regex_str);
+  m_regex = RegularExpression(regex_str);
 }
 
 lldb::SearchDepth CommandCompletions::SymbolCompleter::GetDepth() {
Index: lldb/source/Breakpoint/BreakpointResolverName.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -31,7 +31,8 @@
       m_class_name(), m_regex(), m_match_type(type), m_language(language),
       m_skip_prologue(skip_prologue) {
   if (m_match_type == Breakpoint::Regexp) {
-    if (!m_regex.Compile(llvm::StringRef::withNullAsEmpty(name_cstr))) {
+    m_regex = RegularExpression(llvm::StringRef::withNullAsEmpty(name_cstr));
+    if (!m_regex.IsValid()) {
       Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
 
       if (log)
Index: lldb/include/lldb/Utility/RegularExpression.h
===================================================================
--- lldb/include/lldb/Utility/RegularExpression.h
+++ lldb/include/lldb/Utility/RegularExpression.h
@@ -15,7 +15,7 @@
 
 namespace lldb_private {
 
-class RegularExpression : public llvm::Regex {
+class RegularExpression {
 public:
   /// Default constructor.
   ///
@@ -27,31 +27,14 @@
   ~RegularExpression() = default;
 
   RegularExpression(const RegularExpression &rhs);
-  RegularExpression(RegularExpression &&rhs) = default;
-
-  RegularExpression &operator=(RegularExpression &&rhs) = default;
-  RegularExpression &operator=(const RegularExpression &rhs) = default;
-
-  /// Compile a regular expression.
-  ///
-  /// Compile a regular expression using the supplied regular expression text.
-  /// The compiled regular expression lives in this object so that it can be
-  /// readily used for regular expression matches. Execute() can be called
-  /// after the regular expression is compiled. Any previously compiled
-  /// regular expression contained in this object will be freed.
-  ///
-  /// \param[in] re
-  ///     A NULL terminated C string that represents the regular
-  ///     expression to compile.
-  ///
-  /// \return \b true if the regular expression compiles successfully, \b false
-  ///     otherwise.
-  bool Compile(llvm::StringRef string);
+  RegularExpression(RegularExpression &&rhs);
+  RegularExpression &operator=(const RegularExpression &rhs);
+  RegularExpression &operator=(RegularExpression &&rhs);
 
   /// Executes a regular expression.
   ///
   /// Execute a regular expression match using the compiled regular expression
-  /// that is already in this object against the match string \a s. If any
+  /// that is in this object against the match string \a s. If any
   /// parens are used for regular expression matches \a match_count should
   /// indicate the number of regmatch_t values that are present in \a
   /// match_ptr.
@@ -88,8 +71,16 @@
   bool IsValid() const;
 
   /// Return an error if the regular expression failed to compile.
+  /// The value is not valid if the object has been copy/move-ctored.
   llvm::Error GetError() const;
 
+  bool operator<(const RegularExpression &rhs) const {
+    return GetText() < rhs.GetText();
+  }
+  bool operator==(const RegularExpression &rhs) const {
+    return GetText() == rhs.GetText();
+  }
+
 private:
   /// A copy of the original regular expression text.
   std::string m_regex_text;
Index: lldb/include/lldb/Interpreter/OptionValueRegex.h
===================================================================
--- lldb/include/lldb/Interpreter/OptionValueRegex.h
+++ lldb/include/lldb/Interpreter/OptionValueRegex.h
@@ -50,7 +50,7 @@
 
   void SetCurrentValue(const char *value) {
     if (value && value[0])
-      m_regex.Compile(llvm::StringRef(value));
+      m_regex = RegularExpression(llvm::StringRef(value));
     else
       m_regex = RegularExpression();
   }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to