[Lldb-commits] [PATCH] D66174: [Utility] Reimplement RegularExpression on top of llvm::Regex

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
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"));
----------------
jingham wrote:
> 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.
The POSIX standard says that an empty regex is not a thing, it is rejected by the grammar as defined in https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html. The BSD implementation in LLVM agrees. Unfortunately GNU seems to disagree, which is probably why you expect the old behavior. If you look at the old code, the LLDB implementation worked around this.

I've put the old workaround back, but I don't think we should "fix" the llvm::Regex implementation. Personally I think we should follow the standard in LLDB as well. I tried a few things and I couldn't find a compelling scenario. For example, setting a breakpoint with an empty regex is already not supported.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66174/new/

https://reviews.llvm.org/D66174