This revision was automatically updated to reflect the committed changes.
Closed by commit rG2a6dbedf5a92: [lldb] Fix (unintentional) recursion in
CommandObjectRegexCommand (authored by JDevlieghere).
Herald added a project: LLDB.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.
LGTM !
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120101/new/
https://reviews.llvm.org/D120101
___
lldb-commits mailing list
JDevlieghere updated this revision to Diff 410326.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.
- Allow using `%0`
- Return an error when using an out of range index
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120101/new/
clayborg added inline comments.
Comment at: lldb/source/Commands/CommandObjectRegexCommand.h:47
+ /// matched with replacements[1] and not replacements[0].
+ static std::string SubstituteVariables(
+ llvm::StringRef input,
clayborg wrote:
> Do we want a
clayborg added inline comments.
Comment at: lldb/source/Commands/CommandObjectRegexCommand.h:47
+ /// matched with replacements[1] and not replacements[0].
+ static std::string SubstituteVariables(
+ llvm::StringRef input,
Do we want a "Expected" as the
JDevlieghere updated this revision to Diff 409963.
JDevlieghere added a comment.
Use Pavel's //not-as-cool// solution.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120101/new/
https://reviews.llvm.org/D120101
Files:
lldb/source/Commands/CommandObjectRegexCommand.cpp
labath added a comment.
Wouldn't want to miss a good bikeshed. Logarithms are cool, but I think
something like this (untested) snippet would be simpler
Input.split(Parts, '%');
Result << Parts[0];
for (llvm::StringRef Part : drop_begin(Parts)) {
if (!Part.consumeInteger(10, Idx) &&
clayborg added a comment.
We also have StringExtractor that we could call StringExtractor::GetU32(...)
on. We would need to add a new method like:
bool StringExtractor::SkipTo(char ch);
And then we could call StringExtractor::GetU32(..) if that returns true. The
current string position only
clayborg added inline comments.
Comment at: lldb/source/Commands/CommandObjectRegexCommand.cpp:13
+#include
+
I cringe every time I see an old C header being imported as C++ because of the
huge amounts of junk it causes the DWARF to incur with many import
JDevlieghere added inline comments.
Comment at: lldb/source/Commands/CommandObjectRegexCommand.cpp:38
+// Parse the number following '%'.
+const size_t idx = std::atoi(str.c_str() + pos + 1);
+
mib wrote:
> Are we assuming that the number following `%`
JDevlieghere updated this revision to Diff 409824.
JDevlieghere marked an inline comment as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120101/new/
https://reviews.llvm.org/D120101
Files:
lldb/source/Commands/CommandObjectRegexCommand.cpp
mib added inline comments.
Comment at: lldb/source/Commands/CommandObjectRegexCommand.cpp:38
+// Parse the number following '%'.
+const size_t idx = std::atoi(str.c_str() + pos + 1);
+
Are we assuming that the number following `%` will always have a
JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, clayborg, aprantl, mib.
Herald added a subscriber: mgorny.
JDevlieghere requested review of this revision.
Jim noticed that the regex command is unintentionally recursive:
Let's use the following command regex as an
13 matches
Mail list logo
