[libc++] Fix regex_search to match $ alone with match_default flag

[SanjayMarreddi]

Using regex_search with the regex_constant match_default and a simple regex pattern $ is expected to match general strings such as "a", "ab", "abc"... at [last, last) positions. But, the current implementation fails to do so.

Fixes #75042

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Sanjay Marreddi (SanjayMarreddi)

Changes

Using regex_search with the regex_constant match_default and a simple regex pattern $ is expected to match general strings such as "a", "ab", "abc"... at [last, last) positions. But, the current implementation fails to do so.

Fixes #75042

---

Full diff: https://github.com/llvm/llvm-project/pull/77256.diff

2 Files Affected:

• (modified) libcxx/include/regex (+3)
• (modified) libcxx/test/std/re/re.const/re.matchflag/match_not_eol.pass.cpp (+7)

diff --git a/libcxx/include/regex b/libcxx/include/regex
index b575a267583b5f..0761d9de54a95c 100644
--- a/libcxx/include/regex
+++ b/libcxx/include/regex
@@ -1889,6 +1889,9 @@ void __r_anchor_multiline<_CharT>::__exec(__state& __s) const {
   if (__s.__current_ == __s.__last_ && !(__s.__flags_ & regex_constants::match_not_eol)) {
     __s.__do_   = __state::__accept_but_not_consume;
     __s.__node_ = this->first();
+  } else if (__s.__current_ == __s.__first_ && !(__s.__flags_ & regex_constants::match_not_eol)) {
+    __s.__do_   = __state::__accept_but_not_consume;
+    __s.__node_ = this->first();
   } else if (__multiline_ && std::__is_eol(*__s.__current_)) {
     __s.__do_   = __state::__accept_but_not_consume;
     __s.__node_ = this->first();
diff --git a/libcxx/test/std/re/re.const/re.matchflag/match_not_eol.pass.cpp b/libcxx/test/std/re/re.const/re.matchflag/match_not_eol.pass.cpp
index edeea517d2537a..76f8986b2d539f 100644
--- a/libcxx/test/std/re/re.const/re.matchflag/match_not_eol.pass.cpp
+++ b/libcxx/test/std/re/re.const/re.matchflag/match_not_eol.pass.cpp
@@ -47,5 +47,12 @@ int main(int, char**)
     assert( std::regex_search(target, re, std::regex_constants::match_not_eol));
     }
 
+    {
+      std::string target = "foo";
+      std::regex re("$");
+      assert(std::regex_search(target, re));
+      assert(!std::regex_search(target, re, std::regex_constants::match_not_eol));
+    }
+
   return 0;
 }

[mordante]

Thanks looks good, I'd like one more test before approval.

[mordante]

Thanks! LGTM! Do you need assistance to land this patch?

[SanjayMarreddi]

I'm sorry, I don't think I fully understood what you asked. If you have asked, if I need assistance to get this merged, yes!!

[mordante]

I'm sorry, I don't think I fully understood what you asked. If you have asked, if I need assistance to get this merged, yes!!

Yes it was about merging. Before we used Phabricator and used the word landing.

[SanjayMarreddi]

I'm sorry, I don't think I fully understood what you asked. If you have asked, if I need assistance to get this merged, yes!!

Yes it was about merging. Before we used Phabricator and used the word landing.

Now, I get it :) Thanks!!

[alexfh]

It looks like the patch changes the behavior of search for patterns anchored both to the start and to the end of the input, e.g. after this change std::regex_search finds a pattern like ^b*$ in strings that don't match it (like "a"). Is this intended?

The behavior is now different to that of libstdc++: https://gcc.godbolt.org/z/hcd4acrb6

#include <regex>
#include <string>

int main() {
  std::string t = "a";
  std::regex re {"^b*$"};
  std::smatch matches;
  return std::regex_search(t, matches, re);
}

returns 1 on libc++ and 0 on libstdc++ (and libc++ before this change).

[SanjayMarreddi]

@alexfh Thanks much for letting us know about this. I will look into it.

[alexfh]

As usual: if the behavior is not intended (which I suspect is true) and there's no obvious and reliable fix, I'd suggest to revert first.

[ldionne]

This seems broken to me -- the pattern ^b*$ shouldn't match a. @SanjayMarreddi if you're unsure how to fix this forward, let's revert the commit and add this regression test case, and then work on re-landing the patch without breaking that case. Let me know if you need help with the revert.

[SanjayMarreddi]

I have created the revert. Will add this test case and work on relanding the patch.

[SanjayMarreddi]

I have fixed the above test case along with the $ alone test case and created a PR: #78845