On Thu, Feb 13, 2014 at 1:13 PM, Jonathan Wakely <jwakely....@gmail.com> wrote:
> The LWG have decided that
> http://cplusplus.github.io/LWG/lwg-active.html#2213 is a defect.
>
> In our std::regex_replace we do not appear to update out in all places
> that we should.
1) Yes, the current implementation is buggy for not updating __out
after calling std::copy;
2) I'd rather say the standard is misleading but well intended (return
the new out iterator) rather than ill intended (return the original
out iterator). It'll be a little troubler if match_results<>::format()
do not return the new out iterator, which regex_replace<>() the caller
needs. Boost and libc++ as well return the new iterator.
So my suggestion is just following the LWG proposal, as well as Boost
and libc++.
Here's the patch tested with -m32 and -m64 respectively.
Thanks!
--
Regards,
Tim Shen
commit 3f8621b5f7ced00e21e7038f1e9737eea1bb4251
Author: tim <timshe...@gmail.com>
Date: Thu Feb 13 17:23:48 2014 -0500
2014-02-13 Tim Shen <timshe...@gmail.com>
* include/bits/regex.tcc (match_results<>::format,
regex_replace<>): Update __out after calling std::copy.
* testsuite/28_regex/algorithms/regex_replace/char/basic_replace.cc:
Add testcase.
* testsuite/28_regex/match_results/format.cc: Likewise.
diff --git a/libstdc++-v3/include/bits/regex.tcc
b/libstdc++-v3/include/bits/regex.tcc
index 73f55df..5fa1f01 100644
--- a/libstdc++-v3/include/bits/regex.tcc
+++ b/libstdc++-v3/include/bits/regex.tcc
@@ -425,7 +425,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
auto& __sub = _Base_type::operator[](__idx);
if (__sub.matched)
- std::copy(__sub.first, __sub.second, __out);
+ __out = std::copy(__sub.first, __sub.second, __out);
};
if (__flags & regex_constants::format_sed)
@@ -455,7 +455,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
if (__next == __fmt_last)
break;
- std::copy(__fmt_first, __next, __out);
+ __out = std::copy(__fmt_first, __next, __out);
auto __eat = [&](char __ch) -> bool
{
@@ -493,7 +493,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
*__out++ = '$';
__fmt_first = __next;
}
- std::copy(__fmt_first, __fmt_last, __out);
+ __out = std::copy(__fmt_first, __fmt_last, __out);
}
return __out;
}
@@ -512,7 +512,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
if (__i == __end)
{
if (!(__flags & regex_constants::format_no_copy))
- std::copy(__first, __last, __out);
+ __out = std::copy(__first, __last, __out);
}
else
{
@@ -521,14 +521,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
for (; __i != __end; ++__i)
{
if (!(__flags & regex_constants::format_no_copy))
- std::copy(__i->prefix().first, __i->prefix().second, __out);
+ __out = std::copy(__i->prefix().first, __i->prefix().second,
+ __out);
__out = __i->format(__out, __fmt, __fmt + __len, __flags);
__last = __i->suffix();
if (__flags & regex_constants::format_first_only)
break;
}
if (!(__flags & regex_constants::format_no_copy))
- std::copy(__last.first, __last.second, __out);
+ __out = std::copy(__last.first, __last.second, __out);
}
return __out;
}
diff --git
a/libstdc++-v3/testsuite/28_regex/algorithms/regex_replace/char/basic_replace.cc
b/libstdc++-v3/testsuite/28_regex/algorithms/regex_replace/char/basic_replace.cc
index 28f78a0..38ef970 100644
---
a/libstdc++-v3/testsuite/28_regex/algorithms/regex_replace/char/basic_replace.cc
+++
b/libstdc++-v3/testsuite/28_regex/algorithms/regex_replace/char/basic_replace.cc
@@ -41,6 +41,14 @@ test01()
VERIFY(regex_replace(string("This is a string"), regex("\\b\\w*\\b"), "|$0|",
regex_constants::format_first_only)
== "|This| is a string");
+
+ char buff[4096] = {0};
+ regex re("asdf");
+ string s = "asdf";
+ string res = "|asdf|asdf|";
+ regex_replace(buff, s.data(), s.data() + s.size(), re, "|&|\\0|",
+ regex_constants::format_sed);
+ VERIFY(res == buff);
}
int
diff --git a/libstdc++-v3/testsuite/28_regex/match_results/format.cc
b/libstdc++-v3/testsuite/28_regex/match_results/format.cc
index 11e3bdb..097a0d7 100644
--- a/libstdc++-v3/testsuite/28_regex/match_results/format.cc
+++ b/libstdc++-v3/testsuite/28_regex/match_results/format.cc
@@ -43,6 +43,14 @@ test01()
VERIFY(m.format("&|\\3|\\4|\\2|\\1|\\",
regex_constants::format_sed)
== "this is a string|a|string|is|this|\\");
+
+ regex re("asdf");
+ regex_match("asdf", m, re);
+ string fmt = "|&|\\0|";
+ char buff[4096] = {0};
+ m.format(buff, fmt.data(), fmt.data() + fmt.size(),
+ regex_constants::format_sed);
+ VERIFY(string(buff) == "|asdf|asdf|");
}
int
