jingham added a comment.
I think we need to fix the behavior for "", other than that, this looks fine to
me.
================
Comment at: lldb/source/Host/common/Socket.cpp:287
+ if (g_regex.Execute(host_and_port, &matches)) {
+ if (matches.size() >= 3) {
+ host_str = matches[1].str();
----------------
labath wrote:
> Are there any cases where evaluation of the regex can succeed, but the result
> will not contain the expected number of matches? Should this be changed into
> an assert, or just dropped completely?
Jonas introduced this pattern everywhere he calls Execute. It does seem a
little odd, since there's no way any of the patterns that he checks matches for
could match but not match the () part. It makes the reader wonder if they
don't understand regular expressions after all...
================
Comment at: lldb/source/Target/ThreadPlanStepInRange.cpp:367
+ avoid_regexp_to_use->Execute(frame_function_name, &matches);
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
+ if (return_value && log && matches.size() >= 2) {
----------------
You can move this into the "if (return_value" It doesn't get used outside the
check, and this test mostly fails so it would be good not to do more work in
that case.
================
Comment at: lldb/unittests/Utility/NameMatchesTest.cpp:52-53
EXPECT_TRUE(NameMatches("foobar", NameMatch::RegularExpression, "f[oa]o"));
- EXPECT_TRUE(NameMatches("foo", NameMatch::RegularExpression, ""));
- EXPECT_TRUE(NameMatches("", NameMatch::RegularExpression, ""));
+ EXPECT_FALSE(NameMatches("", NameMatch::RegularExpression, ""));
+ EXPECT_FALSE(NameMatches("foo", NameMatch::RegularExpression, ""));
EXPECT_FALSE(NameMatches("foo", NameMatch::RegularExpression, "b"));
----------------
labath wrote:
> This is interesting. So, llvm::regex considers an empty pattern to not match
> anything? That doesn't sound right. I've just tried grep, python and perl,
> and all of them seem perfectly happy to accept an empty string as a pattern
> which matches everything..
>
> I'd say this is a bug in llvm::regex we should fix.
Yes, that does seem odd to me. I would expect:
(lldb) break set -r ""
to match everything, not nothing.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66174/new/
https://reviews.llvm.org/D66174
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
