[libcxx] Support std::regex_constants::match_not_null

[timshen]

Summary

Fixes PR21597.

[mclow.lists]

I like the fix. :-)

However, I think that the test, rather than going in a bug specific file (pr21597.pass.cpp), should be added to the existing tests - where it should have been in the first place. (If this test had been there in the first place, we would have realized that this feature didn't work)

Also, putting the test in as test/std/re/re.const/re.matchflag/match_not_null.pass.cpp might cause someone to look at the contents of that directory and say "Crap! We're missing tests for match_not_bow, match_not_eow, match_any, match_continuous and match_prev_avail as well" (and probably others).

[mclow.lists]

libcxx/test/std/re/re.alg/re.alg.search/pr21597.pass.cpp

12 ↗

(On Diff #74621)

Rather than this comment about regex_search, there should be a comment about match_not_null, which is really what we're testing here.

Something like:

// match_not_null:
//     The expression shall not match an empty sequence.

[timshen]

Updated file location and documentation.

[timshen]

In D25595#571171, @mclow.lists wrote:

I like the fix. :-)

However, I think that the test, rather than going in a bug specific file (pr21597.pass.cpp), should be added to the existing tests - where it should have been in the first place. (If this test had been there in the first place, we would have realized that this feature didn't work)

Agree.

Also, putting the test in as test/std/re/re.const/re.matchflag/match_not_null.pass.cpp might cause someone to look at the contents of that directory and say "Crap! We're missing tests for match_not_bow, match_not_eow, match_any, match_continuous and match_prev_avail as well" (and probably others).

Making people realize the fact that we don't have tests for match_not_bow, etc. isn't bad, is it? Hopefully it motivates people to add tests. So I'll move the file to re.const/re.matchflag/match_not_null.pass.cpp for now.

BTW, re.const/re.matchflag/match_flag_type.pass.cpp contains match_not_bow, but it doesn't actually test the functionality.

[mclow.lists]

In D25595#572043, @timshen wrote:

BTW, re.const/re.matchflag/match_flag_type.pass.cpp contains match_not_bow, but it doesn't actually test the functionality.

It tests that it exists and that it has a sane value - which are a good things to test.
But there are no tests that show that when passed to regex_search it does the right thing.

[mclow.lists]

Do we need to test calling regex_match with match_not_null?
If not, then I think this is good to go.

Do you have commit access, or would you rather I committed it?

[timshen]

Add tests for regex_match.

[timshen]

In D25595#574364, @mclow.lists wrote:

Do we need to test calling regex_match with match_not_null?
If not, then I think this is good to go.

Done.

Do you have commit access, or would you rather I committed it?

Yes, I have the commit access.

Thanks for reviewing!

[mclow.lists]

LGTM. Thanks!

libcxx/include/regex

if (__st)
         do
         {
             __state& __s = __states.back();
             if (__s.__node_)
                 __s.__node_->__exec(__s);
             switch (__s.__do_)
             {
             case __state::__end_state:
+                if (__flags & regex_constants::match_not_null &&
+                    __s.__current_ == __first)
+                {
+                  __states.pop_back();
+                  break;
+                }
                 __m.__matches_[0].first = __first;
                 __m.__matches_[0].second = _VSTD::next(__first, __s.__current_ - __first);
                 __m.__matches_[0].matched = true;
                 for (unsigned __i = 0; __i < __s.__sub_matches_.size(); ++__i)
                     __m.__matches_[__i+1] = __s.__sub_matches_[__i];
                 return true;
             case __state::__accept_and_consume:
             case __state::__repeat:
if (__st)
         do
         {
             __state& __s = __states.back();
             if (__s.__node_)
                 __s.__node_->__exec(__s);
             switch (__s.__do_)
             {
             case __state::__end_state:
+                if (__flags & regex_constants::match_not_null &&
+                    __s.__current_ == __first)
+                {
+                  __states.pop_back();
+                  break;
+                }
                 if (!__matched || __highest_j < __s.__current_ - __s.__first_)
                     __highest_j = __s.__current_ - __s.__first_;
                 __matched = true;
                 if (__highest_j == _Np)
                     __states.clear();
                 else
                     __states.pop_back();
                 break;
if (__st)
         do
         {
             __state& __s = __states.back();
             if (__s.__node_)
                 __s.__node_->__exec(__s);
             switch (__s.__do_)
             {
             case __state::__end_state:
+                if (__flags & regex_constants::match_not_null &&
+                    __s.__current_ == __first)
+                {
+                  __states.pop_back();
+                  break;
+                }
                 if (!__matched || __highest_j < __s.__current_ - __s.__first_)
                 {
                     __highest_j = __s.__current_ - __s.__first_;
                     __best_state = __s;
                 }
                 __matched = true;
                 if (__highest_j == _Np)
                     __states.clear();

libcxx/test/std/re/re.const/re.matchflag/match_not_null.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// <regex>
+
+// match_not_null:
+//     The regular expression shall not match an empty sequence.
+
+#include "test_macros.h"
+#include <cassert>
+#include <regex>
+
+int main()
+{
+  // When match_not_null is on, the regex engine should reject empty matches and
+  // move on to try other solutions.
+  std::cmatch m;
+  assert(!std::regex_search("a", m, std::regex("b*"),
+                            std::regex_constants::match_not_null));
+  assert(std::regex_search("aa", m, std::regex("a*?"),
+                           std::regex_constants::match_not_null));
+  assert(m[0].length() == 1);
+  assert(!std::regex_search("a", m, std::regex("b*", std::regex::extended),
+                            std::regex_constants::match_not_null));
+  assert(!std::regex_search(
+      "a", m,
+      std::regex("b*", std::regex::extended | std::regex_constants::nosubs),
+      std::regex_constants::match_not_null));
+}