labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
I agree that it is better to have just one way to initialize the regex object.
This looks good to me modulo the llvm parts. I'd suggest just dropping those,
or creating a separate patch for that...
================
Comment at: lldb/include/lldb/Interpreter/OptionValueRegex.h:53-55
- m_regex.Compile(llvm::StringRef(value));
+ m_regex = RegularExpression(llvm::StringRef(value));
else
m_regex = RegularExpression();
----------------
The apparent inconsistency here is a pretty good argument against the `Compile`
method imo.
================
Comment at: lldb/include/lldb/Utility/RegularExpression.h:34
+ /// \param[in] string
+ /// A NULL terminated C string that represents the regular
+ /// expression to compile.
----------------
Null terminated? I hope not..
================
Comment at: lldb/source/Commands/CommandObjectFrame.cpp:536-537
llvm::StringRef name_str = entry.ref;
RegularExpression regex(name_str);
- if (regex.Compile(name_str)) {
size_t num_matches = 0;
----------------
lol
================
Comment at:
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp:445
RegularExpression regex;
llvm::SmallVector<llvm::StringRef, 4> matches;
----------------
Here I'd just declare a fresh variable in each of the blocks instead of
continuously recycling this one.
================
Comment at: lldb/source/Target/ThreadPlanStepInRange.cpp:317-319
m_avoid_regexp_up.reset(new RegularExpression(name_ref));
-
- m_avoid_regexp_up->Compile(name_ref);
+ else
+ *m_avoid_regexp_up = RegularExpression(name_ref);
----------------
maybe swap the then and else blocks around to reduce the amount of negation
================
Comment at: lldb/source/Utility/RegularExpression.cpp:16-19
m_regex_text = str;
+
+ // m_regex does not reference str anymore after it is constructed.
m_regex = llvm::Regex(str);
----------------
Initialize this stuff in the member initializer list?
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66392/new/
https://reviews.llvm.org/D66392
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
