diff mbox

std::regex_replace behaviour (LWG DR 2213)

Message ID CAPrifD=ec9Vsx_wQkK_9JKQp_Nwg7c+nzCQRJb5shNxW4-CLNg@mail.gmail.com
State New
Headers show

Commit Message

Tim Shen Feb. 13, 2014, 10:31 p.m. UTC 
On Thu, Feb 13, 2014 at 1:13 PM, Jonathan Wakely <jwakely.gcc@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!

Comments

Paolo Carlini Feb. 14, 2014, 10:59 a.m. UTC | #1 
.. I think it would be cleaner to have new, separate testcases, named 
after 2213. This is what we always did in the past when we implemented 
resolutions of DRs.

At minimum, refer to 2213 in a comment.

Paolo.
diff mbox

Patch

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