Skip to content

Commit f9d90bc

Browse files
jankratochviljankratochvil
committed
[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
1 parent 12d83b4 commit f9d90bc

File tree

14 files changed

+61
-71
lines changed

14 files changed

+61
-71
lines changed

‎lldb/include/lldb/Interpreter/OptionValueRegex.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class OptionValueRegex : public OptionValue {
5050

5151
void SetCurrentValue(const char *value) {
5252
if (value && value[0])
53-
m_regex.Compile(llvm::StringRef(value));
53+
m_regex = RegularExpression(llvm::StringRef(value));
5454
else
5555
m_regex = RegularExpression();
5656
}

‎lldb/include/lldb/Utility/RegularExpression.h

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,18 @@ class RegularExpression {
2323
/// contains no compiled regular expression.
2424
RegularExpression() = default;
2525

26+
/// Constructor for a regular expression.
27+
///
28+
/// Compile a regular expression using the supplied regular expression text.
29+
/// The compiled regular expression lives in this object so that it can be
30+
/// readily used for regular expression matches. Execute() can be called
31+
/// after the regular expression is compiled.
32+
///
33+
/// \param[in] string
34+
/// An llvm::StringRef that represents the regular expression to compile.
35+
// String is not referenced anymore after the object is constructed.
2636
explicit RegularExpression(llvm::StringRef string);
37+
2738
~RegularExpression() = default;
2839

2940
RegularExpression(const RegularExpression &rhs);
@@ -32,22 +43,6 @@ class RegularExpression {
3243
RegularExpression &operator=(RegularExpression &&rhs) = default;
3344
RegularExpression &operator=(const RegularExpression &rhs) = default;
3445

35-
/// Compile a regular expression.
36-
///
37-
/// Compile a regular expression using the supplied regular expression text.
38-
/// The compiled regular expression lives in this object so that it can be
39-
/// readily used for regular expression matches. Execute() can be called
40-
/// after the regular expression is compiled. Any previously compiled
41-
/// regular expression contained in this object will be freed.
42-
///
43-
/// \param[in] re
44-
/// A NULL terminated C string that represents the regular
45-
/// expression to compile.
46-
///
47-
/// \return \b true if the regular expression compiles successfully, \b false
48-
/// otherwise.
49-
bool Compile(llvm::StringRef string);
50-
5146
/// Executes a regular expression.
5247
///
5348
/// Execute a regular expression match using the compiled regular expression
@@ -65,7 +60,7 @@ class RegularExpression {
6560
/// matches, or nullptr if no parenthesized matching is needed.
6661
///
6762
/// \return \b true if \a string matches the compiled regular expression, \b
68-
/// false otherwise.
63+
/// false otherwise incl. the case regular exression failed to compile.
6964
bool Execute(llvm::StringRef string,
7065
llvm::SmallVectorImpl<llvm::StringRef> *matches = nullptr) const;
7166

‎lldb/source/Breakpoint/BreakpointResolverName.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ BreakpointResolverName::BreakpointResolverName(
3131
m_class_name(), m_regex(), m_match_type(type), m_language(language),
3232
m_skip_prologue(skip_prologue) {
3333
if (m_match_type == Breakpoint::Regexp) {
34-
if (!m_regex.Compile(llvm::StringRef::withNullAsEmpty(name_cstr))) {
34+
m_regex = RegularExpression(llvm::StringRef::withNullAsEmpty(name_cstr));
35+
if (!m_regex.IsValid()) {
3536
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
3637

3738
if (log)

‎lldb/source/Commands/CommandCompletions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ CommandCompletions::SymbolCompleter::SymbolCompleter(
454454
pos = regex_str.insert(pos, '\\');
455455
pos = find_if(pos + 2, regex_str.end(), regex_chars);
456456
}
457-
m_regex.Compile(regex_str);
457+
m_regex = RegularExpression(regex_str);
458458
}
459459

460460
lldb::SearchDepth CommandCompletions::SymbolCompleter::GetDepth() {

‎lldb/source/Commands/CommandObjectFrame.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ class CommandObjectFrameVariable : public CommandObjectParsed {
534534
const size_t regex_start_index = regex_var_list.GetSize();
535535
llvm::StringRef name_str = entry.ref;
536536
RegularExpression regex(name_str);
537-
if (regex.Compile(name_str)) {
537+
if (regex.IsValid()) {
538538
size_t num_matches = 0;
539539
const size_t num_new_regex_vars =
540540
variable_list->AppendVariablesIfUnique(regex, regex_var_list,

‎lldb/source/Commands/CommandObjectType.cpp

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -691,8 +691,8 @@ pointers to floats. Nor will it change the default display for Afloat and Bfloa
691691

692692
ConstString typeCS(arg_entry.ref);
693693
if (m_command_options.m_regex) {
694-
RegularExpressionSP typeRX(new RegularExpression());
695-
if (!typeRX->Compile(arg_entry.ref)) {
694+
RegularExpressionSP typeRX(new RegularExpression(arg_entry.ref));
695+
if (!typeRX->IsValid()) {
696696
result.AppendError(
697697
"regex format error (maybe this is not really a regex?)");
698698
result.SetStatus(eReturnStatusFailed);
@@ -1043,9 +1043,9 @@ class CommandObjectTypeFormatterList : public CommandObjectParsed {
10431043
std::unique_ptr<RegularExpression> formatter_regex;
10441044

10451045
if (m_options.m_category_regex.OptionWasSet()) {
1046-
category_regex.reset(new RegularExpression());
1047-
if (!category_regex->Compile(
1048-
m_options.m_category_regex.GetCurrentValueAsRef())) {
1046+
category_regex.reset(new RegularExpression(
1047+
m_options.m_category_regex.GetCurrentValueAsRef()));
1048+
if (!category_regex->IsValid()) {
10491049
result.AppendErrorWithFormat(
10501050
"syntax error in category regular expression '%s'",
10511051
m_options.m_category_regex.GetCurrentValueAsRef().str().c_str());
@@ -1056,8 +1056,9 @@ class CommandObjectTypeFormatterList : public CommandObjectParsed {
10561056

10571057
if (argc == 1) {
10581058
const char *arg = command.GetArgumentAtIndex(0);
1059-
formatter_regex.reset(new RegularExpression());
1060-
if (!formatter_regex->Compile(llvm::StringRef::withNullAsEmpty(arg))) {
1059+
formatter_regex.reset(
1060+
new RegularExpression(llvm::StringRef::withNullAsEmpty(arg)));
1061+
if (!formatter_regex->IsValid()) {
10611062
result.AppendErrorWithFormat("syntax error in regular expression '%s'",
10621063
arg);
10631064
result.SetStatus(eReturnStatusFailed);
@@ -1629,8 +1630,8 @@ bool CommandObjectTypeSummaryAdd::AddSummary(ConstString type_name,
16291630
}
16301631

16311632
if (type == eRegexSummary) {
1632-
RegularExpressionSP typeRX(new RegularExpression());
1633-
if (!typeRX->Compile(type_name.GetStringRef())) {
1633+
RegularExpressionSP typeRX(new RegularExpression(type_name.GetStringRef()));
1634+
if (!typeRX->IsValid()) {
16341635
if (error)
16351636
error->SetErrorString(
16361637
"regex format error (maybe this is not really a regex?)");
@@ -2115,9 +2116,9 @@ class CommandObjectTypeCategoryList : public CommandObjectParsed {
21152116
std::unique_ptr<RegularExpression> regex;
21162117

21172118
if (argc == 1) {
2118-
regex.reset(new RegularExpression());
21192119
const char *arg = command.GetArgumentAtIndex(0);
2120-
if (!regex->Compile(llvm::StringRef::withNullAsEmpty(arg))) {
2120+
regex.reset(new RegularExpression(llvm::StringRef::withNullAsEmpty(arg)));
2121+
if (!regex->IsValid()) {
21212122
result.AppendErrorWithFormat(
21222123
"syntax error in category regular expression '%s'", arg);
21232124
result.SetStatus(eReturnStatusFailed);
@@ -2369,8 +2370,8 @@ bool CommandObjectTypeSynthAdd::AddSynth(ConstString type_name,
23692370
}
23702371

23712372
if (type == eRegexSynth) {
2372-
RegularExpressionSP typeRX(new RegularExpression());
2373-
if (!typeRX->Compile(type_name.GetStringRef())) {
2373+
RegularExpressionSP typeRX(new RegularExpression(type_name.GetStringRef()));
2374+
if (!typeRX->IsValid()) {
23742375
if (error)
23752376
error->SetErrorString(
23762377
"regex format error (maybe this is not really a regex?)");
@@ -2497,8 +2498,9 @@ class CommandObjectTypeFilterAdd : public CommandObjectParsed {
24972498
}
24982499

24992500
if (type == eRegexFilter) {
2500-
RegularExpressionSP typeRX(new RegularExpression());
2501-
if (!typeRX->Compile(type_name.GetStringRef())) {
2501+
RegularExpressionSP typeRX(
2502+
new RegularExpression(type_name.GetStringRef()));
2503+
if (!typeRX->IsValid()) {
25022504
if (error)
25032505
error->SetErrorString(
25042506
"regex format error (maybe this is not really a regex?)");

‎lldb/source/Core/AddressResolverName.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ AddressResolverName::AddressResolverName(const char *func_name,
3636
: AddressResolver(), m_func_name(func_name), m_class_name(nullptr),
3737
m_regex(), m_match_type(type) {
3838
if (m_match_type == AddressResolver::Regexp) {
39-
if (!m_regex.Compile(m_func_name.GetStringRef())) {
39+
m_regex = RegularExpression(m_func_name.GetStringRef());
40+
if (!m_regex.IsValid()) {
4041
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
4142

4243
if (log)

‎lldb/source/Interpreter/CommandObjectRegexCommand.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,9 @@ bool CommandObjectRegexCommand::AddRegexCommand(const char *re_cstr,
7373
const char *command_cstr) {
7474
m_entries.resize(m_entries.size() + 1);
7575
// Only add the regular expression if it compiles
76-
if (m_entries.back().regex.Compile(
77-
llvm::StringRef::withNullAsEmpty(re_cstr))) {
76+
m_entries.back().regex =
77+
RegularExpression(llvm::StringRef::withNullAsEmpty(re_cstr));
78+
if (m_entries.back().regex.IsValid()) {
7879
m_entries.back().command.assign(command_cstr);
7980
return true;
8081
}

‎lldb/source/Interpreter/OptionValueRegex.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ Status OptionValueRegex::SetValueFromString(llvm::StringRef value,
4646

4747
case eVarSetOperationReplace:
4848
case eVarSetOperationAssign:
49-
if (m_regex.Compile(value)) {
49+
m_regex = RegularExpression(value);
50+
if (m_regex.IsValid()) {
5051
m_value_was_set = true;
5152
NotifyValueChanged();
5253
} else if (llvm::Error err = m_regex.GetError()) {

‎lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -582,9 +582,9 @@ class CommandObjectObjC_ClassTable_Dump : public CommandObjectParsed {
582582
case 0:
583583
break;
584584
case 1: {
585-
regex_up.reset(new RegularExpression());
586-
if (!regex_up->Compile(llvm::StringRef::withNullAsEmpty(
587-
command.GetArgumentAtIndex(0)))) {
585+
regex_up.reset(new RegularExpression(
586+
llvm::StringRef::withNullAsEmpty(command.GetArgumentAtIndex(0))));
587+
if (!regex_up->IsValid()) {
588588
result.AppendError(
589589
"invalid argument - please provide a valid regular expression");
590590
result.SetStatus(lldb::eReturnStatusFailed);

‎lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -442,21 +442,12 @@ bool ParseCoordinate(llvm::StringRef coord_s, RSCoordinate &coord) {
442442
// elements fails the contents of &coord are undefined and `false` is
443443
// returned, `true` otherwise
444444

445-
RegularExpression regex;
446445
llvm::SmallVector<llvm::StringRef, 4> matches;
447446

448-
bool matched = false;
449-
if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+),([0-9]+)$")) &&
450-
regex.Execute(coord_s, &matches))
451-
matched = true;
452-
else if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+)$")) &&
453-
regex.Execute(coord_s, &matches))
454-
matched = true;
455-
else if (regex.Compile(llvm::StringRef("^([0-9]+)$")) &&
456-
regex.Execute(coord_s, &matches))
457-
matched = true;
458-
459-
if (!matched)
447+
if (!RegularExpression("^([0-9]+),([0-9]+),([0-9]+)$")
448+
.Execute(coord_s, &matches) &&
449+
!RegularExpression("^([0-9]+),([0-9]+)$").Execute(coord_s, &matches) &&
450+
!RegularExpression("^([0-9]+)$").Execute(coord_s, &matches))
460451
return false;
461452

462453
auto get_index = [&](size_t idx, uint32_t &i) -> bool {

‎lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,11 +332,12 @@ GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote &packet,
332332
std::string regex_str = "^";
333333
regex_str += echo_packet;
334334
regex_str += "$";
335-
response_regex.Compile(regex_str);
335+
response_regex = RegularExpression(regex_str);
336336
} else {
337337
echo_packet_len =
338338
::snprintf(echo_packet, sizeof(echo_packet), "qC");
339-
response_regex.Compile(llvm::StringRef("^QC[0-9A-Fa-f]+$"));
339+
response_regex =
340+
RegularExpression(llvm::StringRef("^QC[0-9A-Fa-f]+$"));
340341
}
341342

342343
PacketResult echo_packet_result =

‎lldb/source/Target/ThreadPlanStepInRange.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -313,10 +313,10 @@ bool ThreadPlanStepInRange::ShouldStop(Event *event_ptr) {
313313

314314
void ThreadPlanStepInRange::SetAvoidRegexp(const char *name) {
315315
auto name_ref = llvm::StringRef::withNullAsEmpty(name);
316-
if (!m_avoid_regexp_up)
316+
if (m_avoid_regexp_up)
317+
*m_avoid_regexp_up = RegularExpression(name_ref);
318+
else
317319
m_avoid_regexp_up.reset(new RegularExpression(name_ref));
318-
319-
m_avoid_regexp_up->Compile(name_ref);
320320
}
321321

322322
void ThreadPlanStepInRange::SetDefaultFlagValue(uint32_t new_value) {

‎lldb/source/Utility/RegularExpression.cpp

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,19 @@
1212

1313
using namespace lldb_private;
1414

15-
RegularExpression::RegularExpression(llvm::StringRef str) { Compile(str); }
15+
RegularExpression::RegularExpression(llvm::StringRef str)
16+
: m_regex_text(str),
17+
// m_regex does not reference str anymore after it is constructed.
18+
m_regex(llvm::Regex(str)) {}
1619

1720
RegularExpression::RegularExpression(const RegularExpression &rhs)
18-
: RegularExpression() {
19-
Compile(rhs.GetText());
20-
}
21-
22-
bool RegularExpression::Compile(llvm::StringRef str) {
23-
m_regex_text = str;
24-
m_regex = llvm::Regex(str);
25-
return IsValid();
26-
}
21+
: RegularExpression(rhs.GetText()) {}
2722

2823
bool RegularExpression::Execute(
2924
llvm::StringRef str,
3025
llvm::SmallVectorImpl<llvm::StringRef> *matches) const {
26+
if (!IsValid())
27+
return false;
3128
return m_regex.match(str, matches);
3229
}
3330

0 commit comments

Comments
 (0)