[libcxx] re.results.form: Format out-of-range subexpression references as null

[dexonsmith]

Summary

Rather than crashing in match_results::format() when a reference to a
marked subexpression is out of range, format the subexpression as
empty (i.e., replace it with an empty string).

The standard doesn't require that marked subexpression references are
valid (the only preconditions are that ready() == true and that Out is
an output iterator), so the implementation shouldn't segfault on
out-of-range marked subexpressions. The most user-friendly behaviour
(and the easiest to implement) is to treat them as "null" sub-matches.
Another option that I rejected is to leave them un-substituted.

I'm not sure about my choice of where to put the tests. I considered
duplicating them across all four form*.pass.cpp, but thought I'd get
some feedback first.

[mclow.lists]

Not crashing is good; but silently returning a "wrong" answer rubs me the wrong way. I'll do some research and get back to you on this, but maybe throwing an exception is a better response.

The code looks fine, though; I'm just not sure it's the right thing to do.

[mclow.lists]

include/regex

5390–5391

A better idea.
Replace this code:

__out = _VSTD::copy(__matches_[__i].first, __matches_[__i].second, __out);

with:

__out = _VSTD::copy((*this)[__i].first, (*this)[__i].second, __out);

match_results::operator[] already has the logic to do something with out of range indexes.

5442–5443

Same as above.

test/std/re/re.results/re.results.form/form1.pass.cpp

61

Keep the tests.

[cfe-commits]

Given your follow-up review, and the behaviour of
match_results::operator[], do you still have reservations?

[dexonsmith]

Addressed Marshall's review comments: deferring to match_results::operator[]() rather than duplicating the logic.

[dexonsmith]

Ping.

Marshall, does this look okay?

[mclow.lists]

LGTM

[dexonsmith]

Committed in r259682.

include/regex

if (__flags & regex_constants::format_sed)
                 __out = _VSTD::copy(__matches_[0].first, __matches_[0].second,
                                    __out);
             else if (*__fmt_first == '\\' && __fmt_first + 1 != __fmt_last)
             {
                 ++__fmt_first;
                 if ('0' <= *__fmt_first && *__fmt_first <= '9')
                 {
                     size_t __i = *__fmt_first - '0';
-                    __out = _VSTD::copy(__matches_[__i].first,
-                                       __matches_[__i].second, __out);
+                    __out = _VSTD::copy((*this)[__i].first,
+                                        (*this)[__i].second, __out);
                 }
                 else
                 {
                     *__out = *__fmt_first;
                     ++__out;
                 }
             }
             else
else
                         ++__fmt_first;
                         size_t __i = *__fmt_first - '0';
                         if (__fmt_first + 1 != __fmt_last &&
                             '0' <= __fmt_first[1] && __fmt_first[1] <= '9')
                         {
                             ++__fmt_first;
                             __i = 10 * __i + *__fmt_first - '0';
                         }
-                        __out = _VSTD::copy(__matches_[__i].first,
-                                           __matches_[__i].second, __out);
+                        __out = _VSTD::copy((*this)[__i].first,
+                                            (*this)[__i].second, __out);
                     }
                     else
                     {
                         *__out = *__fmt_first;
                         ++__out;
                     }
                     break;
                 }

test/std/re/re.results/re.results.form/form1.pass.cpp

int main()
         char* r = m.format(output_iterator<char*>(out),
                     fmt, fmt + std::char_traits<char>::length(fmt)).base();
         assert(r == out + 58);
         assert(std::string(out) == "prefix: ab, match: cdefghi, suffix: jk, m[1]: efg, m[2]: e");
     }
     {
         std::match_results<const char*> m;
         const char s[] = "abcdefghijk";
+        assert(std::regex_search(s, m, std::regex("cd((e)fg)hi",
+                                                  std::regex_constants::nosubs)));
+
+        char out[100] = {0};
+        const char fmt[] = "prefix: $`, match: $&, suffix: $', m[1]: $1, m[2]: $2";
+        char* r = m.format(output_iterator<char*>(out),
+                    fmt, fmt + std::char_traits<char>::length(fmt)).base();
+        assert(r == out + 54);
+        assert(std::string(out) == "prefix: ab, match: cdefghi, suffix: jk, m[1]: , m[2]: ");
+    }
+    {
+        std::match_results<const char*> m;
+        const char s[] = "abcdefghijk";
+        assert(std::regex_search(s, m, std::regex("cdefghi")));
+
+        char out[100] = {0};
+        const char fmt[] = "prefix: $`, match: $&, suffix: $', m[1]: $1, m[2]: $2";
+        char* r = m.format(output_iterator<char*>(out),
+                    fmt, fmt + std::char_traits<char>::length(fmt)).base();
+        assert(r == out + 54);
+        assert(std::string(out) == "prefix: ab, match: cdefghi, suffix: jk, m[1]: , m[2]: ");
+    }
+    {
+        std::match_results<const char*> m;
+        const char s[] = "abcdefghijk";
         assert(std::regex_search(s, m, std::regex("cd((e)fg)hi")));
 
         char out[100] = {0};
         const char fmt[] = "prefix: $`, match: $&, suffix: $', m[1]: $1, m[2]: $2";
         char* r = m.format(output_iterator<char*>(out),
                     fmt, fmt + std::char_traits<char>::length(fmt),
                     std::regex_constants::format_sed).base();
         assert(r == out + 59);
         assert(std::string(out) == "prefix: $`, match: $cdefghi, suffix: $', m[1]: $1, m[2]: $2");
     }
     {
         std::match_results<const char*> m;
         const char s[] = "abcdefghijk";
         assert(std::regex_search(s, m, std::regex("cd((e)fg)hi")));
 
         char out[100] = {0};
         const char fmt[] = "match: &, m[1]: \\1, m[2]: \\2";
         char* r = m.format(output_iterator<char*>(out),
                     fmt, fmt + std::char_traits<char>::length(fmt),
                     std::regex_constants::format_sed).base();
         assert(r == out + 34);
         assert(std::string(out) == "match: cdefghi, m[1]: efg, m[2]: e");
     }
+    {
+        std::match_results<const char*> m;
+        const char s[] = "abcdefghijk";
+        assert(std::regex_search(s, m, std::regex("cd((e)fg)hi",
+                                                  std::regex_constants::nosubs)));
+
+        char out[100] = {0};
+        const char fmt[] = "match: &, m[1]: \\1, m[2]: \\2";
+        char* r = m.format(output_iterator<char*>(out),
+                    fmt, fmt + std::char_traits<char>::length(fmt),
+                    std::regex_constants::format_sed).base();
+        assert(r == out + 30);
+        assert(std::string(out) == "match: cdefghi, m[1]: , m[2]: ");
+    }
+    {
+        std::match_results<const char*> m;
+        const char s[] = "abcdefghijk";
+        assert(std::regex_search(s, m, std::regex("cdefghi")));
+
+        char out[100] = {0};
+        const char fmt[] = "match: &, m[1]: \\1, m[2]: \\2";
+        char* r = m.format(output_iterator<char*>(out),
+                    fmt, fmt + std::char_traits<char>::length(fmt),
+                    std::regex_constants::format_sed).base();
+        assert(r == out + 30);
+        assert(std::string(out) == "match: cdefghi, m[1]: , m[2]: ");
+    }
 
     {
         std::match_results<const wchar_t*> m;
         const wchar_t s[] = L"abcdefghijk";
         assert(std::regex_search(s, m, std::wregex(L"cd((e)fg)hi")));
 
         wchar_t out[100] = {0};
         const wchar_t fmt[] = L"prefix: $`, match: $&, suffix: $', m[1]: $1, m[2]: $2";