From f9d90bc5f690de43cbfd8bd15f6f3d3e01471615 Mon Sep 17 00:00:00 2001 From: Jan Kratochvil Date: Tue, 20 Aug 2019 09:24:20 +0000 Subject: [PATCH] [lldb] D66174 `RegularExpression` cleanup I find as a good cleanup to drop the Compile method. As I do not find TIMTOWTDI as an advantage and there is already constructor parameter to compile the regex. Differential Revision: https://reviews.llvm.org/D66392 llvm-svn: 369352 --- .../lldb/Interpreter/OptionValueRegex.h | 2 +- lldb/include/lldb/Utility/RegularExpression.h | 29 +++++++---------- .../Breakpoint/BreakpointResolverName.cpp | 3 +- lldb/source/Commands/CommandCompletions.cpp | 2 +- lldb/source/Commands/CommandObjectFrame.cpp | 2 +- lldb/source/Commands/CommandObjectType.cpp | 32 ++++++++++--------- lldb/source/Core/AddressResolverName.cpp | 3 +- .../Interpreter/CommandObjectRegexCommand.cpp | 5 +-- lldb/source/Interpreter/OptionValueRegex.cpp | 3 +- .../AppleObjCRuntime/AppleObjCRuntimeV2.cpp | 6 ++-- .../RenderScriptRuntime.cpp | 17 +++------- .../gdb-remote/GDBRemoteCommunication.cpp | 5 +-- lldb/source/Target/ThreadPlanStepInRange.cpp | 6 ++-- lldb/source/Utility/RegularExpression.cpp | 17 ++++------ 14 files changed, 61 insertions(+), 71 deletions(-) diff --git a/lldb/include/lldb/Interpreter/OptionValueRegex.h b/lldb/include/lldb/Interpreter/OptionValueRegex.h index e38bee140a00..8c10dacb0313 100644 --- a/lldb/include/lldb/Interpreter/OptionValueRegex.h +++ b/lldb/include/lldb/Interpreter/OptionValueRegex.h @@ -50,7 +50,7 @@ public: 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(); } diff --git a/lldb/include/lldb/Utility/RegularExpression.h b/lldb/include/lldb/Utility/RegularExpression.h index 2bcc53c9bcda..84dac38ada01 100644 --- a/lldb/include/lldb/Utility/RegularExpression.h +++ b/lldb/include/lldb/Utility/RegularExpression.h @@ -23,7 +23,18 @@ public: /// contains no compiled regular expression. RegularExpression() = default; + /// Constructor for 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. + /// + /// \param[in] string + /// An llvm::StringRef that represents the regular expression to compile. + // String is not referenced anymore after the object is constructed. explicit RegularExpression(llvm::StringRef string); + ~RegularExpression() = default; RegularExpression(const RegularExpression &rhs); @@ -32,22 +43,6 @@ public: 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); - /// Executes a regular expression. /// /// Execute a regular expression match using the compiled regular expression @@ -65,7 +60,7 @@ public: /// matches, or nullptr if no parenthesized matching is needed. /// /// \return \b true if \a string matches the compiled regular expression, \b - /// false otherwise. + /// false otherwise incl. the case regular exression failed to compile. bool Execute(llvm::StringRef string, llvm::SmallVectorImpl *matches = nullptr) const; diff --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp b/lldb/source/Breakpoint/BreakpointResolverName.cpp index b18c1c46be1d..e63661d1c01a 100644 --- a/lldb/source/Breakpoint/BreakpointResolverName.cpp +++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp @@ -31,7 +31,8 @@ BreakpointResolverName::BreakpointResolverName( 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) diff --git a/lldb/source/Commands/CommandCompletions.cpp b/lldb/source/Commands/CommandCompletions.cpp index 9a969d289be8..1cff39c37345 100644 --- a/lldb/source/Commands/CommandCompletions.cpp +++ b/lldb/source/Commands/CommandCompletions.cpp @@ -454,7 +454,7 @@ CommandCompletions::SymbolCompleter::SymbolCompleter( 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() { diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index eb54e354a631..1087c6111690 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -534,7 +534,7 @@ protected: 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, diff --git a/lldb/source/Commands/CommandObjectType.cpp b/lldb/source/Commands/CommandObjectType.cpp index cbe0e37824cd..0fe82b88c6cb 100644 --- a/lldb/source/Commands/CommandObjectType.cpp +++ b/lldb/source/Commands/CommandObjectType.cpp @@ -691,8 +691,8 @@ protected: 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); @@ -1043,9 +1043,9 @@ protected: std::unique_ptr 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()); @@ -1056,8 +1056,9 @@ protected: 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); @@ -1629,8 +1630,8 @@ bool CommandObjectTypeSummaryAdd::AddSummary(ConstString type_name, } 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?)"); @@ -2115,9 +2116,9 @@ protected: std::unique_ptr 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); @@ -2369,8 +2370,8 @@ bool CommandObjectTypeSynthAdd::AddSynth(ConstString type_name, } 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?)"); @@ -2497,8 +2498,9 @@ private: } 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?)"); diff --git a/lldb/source/Core/AddressResolverName.cpp b/lldb/source/Core/AddressResolverName.cpp index 894b7612bb2a..089f0da44005 100644 --- a/lldb/source/Core/AddressResolverName.cpp +++ b/lldb/source/Core/AddressResolverName.cpp @@ -36,7 +36,8 @@ AddressResolverName::AddressResolverName(const char *func_name, : 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) diff --git a/lldb/source/Interpreter/CommandObjectRegexCommand.cpp b/lldb/source/Interpreter/CommandObjectRegexCommand.cpp index 16e6feb957c1..94e52a36fb35 100644 --- a/lldb/source/Interpreter/CommandObjectRegexCommand.cpp +++ b/lldb/source/Interpreter/CommandObjectRegexCommand.cpp @@ -73,8 +73,9 @@ bool CommandObjectRegexCommand::AddRegexCommand(const char *re_cstr, 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; } diff --git a/lldb/source/Interpreter/OptionValueRegex.cpp b/lldb/source/Interpreter/OptionValueRegex.cpp index eaf05928fc71..cf806fb550f9 100644 --- a/lldb/source/Interpreter/OptionValueRegex.cpp +++ b/lldb/source/Interpreter/OptionValueRegex.cpp @@ -46,7 +46,8 @@ Status OptionValueRegex::SetValueFromString(llvm::StringRef value, 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()) { diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp index cfec2de1fb95..1ff3fc42dd0a 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp @@ -582,9 +582,9 @@ protected: 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); diff --git a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp index 2e0c8d52dddf..38bd79d68cf9 100644 --- a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp @@ -442,21 +442,12 @@ bool ParseCoordinate(llvm::StringRef coord_s, RSCoordinate &coord) { // elements fails the contents of &coord are undefined and `false` is // returned, `true` otherwise - RegularExpression regex; llvm::SmallVector 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) + if (!RegularExpression("^([0-9]+),([0-9]+),([0-9]+)$") + .Execute(coord_s, &matches) && + !RegularExpression("^([0-9]+),([0-9]+)$").Execute(coord_s, &matches) && + !RegularExpression("^([0-9]+)$").Execute(coord_s, &matches)) return false; auto get_index = [&](size_t idx, uint32_t &i) -> bool { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index b4472037b9bf..bb283b7397a6 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -332,11 +332,12 @@ GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote &packet, 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 = diff --git a/lldb/source/Target/ThreadPlanStepInRange.cpp b/lldb/source/Target/ThreadPlanStepInRange.cpp index a0549c81edcb..71045cc7a990 100644 --- a/lldb/source/Target/ThreadPlanStepInRange.cpp +++ b/lldb/source/Target/ThreadPlanStepInRange.cpp @@ -313,10 +313,10 @@ bool ThreadPlanStepInRange::ShouldStop(Event *event_ptr) { void ThreadPlanStepInRange::SetAvoidRegexp(const char *name) { auto name_ref = llvm::StringRef::withNullAsEmpty(name); - if (!m_avoid_regexp_up) + if (m_avoid_regexp_up) + *m_avoid_regexp_up = RegularExpression(name_ref); + else m_avoid_regexp_up.reset(new RegularExpression(name_ref)); - - m_avoid_regexp_up->Compile(name_ref); } void ThreadPlanStepInRange::SetDefaultFlagValue(uint32_t new_value) { diff --git a/lldb/source/Utility/RegularExpression.cpp b/lldb/source/Utility/RegularExpression.cpp index 356d78a041f0..432b45ee4c46 100644 --- a/lldb/source/Utility/RegularExpression.cpp +++ b/lldb/source/Utility/RegularExpression.cpp @@ -12,22 +12,19 @@ using namespace lldb_private; -RegularExpression::RegularExpression(llvm::StringRef str) { Compile(str); } +RegularExpression::RegularExpression(llvm::StringRef str) + : m_regex_text(str), + // m_regex does not reference str anymore after it is constructed. + m_regex(llvm::Regex(str)) {} RegularExpression::RegularExpression(const RegularExpression &rhs) - : RegularExpression() { - Compile(rhs.GetText()); -} - -bool RegularExpression::Compile(llvm::StringRef str) { - m_regex_text = str; - m_regex = llvm::Regex(str); - return IsValid(); -} + : RegularExpression(rhs.GetText()) {} bool RegularExpression::Execute( llvm::StringRef str, llvm::SmallVectorImpl *matches) const { + if (!IsValid()) + return false; return m_regex.match(str, matches); }