diff mbox

std::regex: inserting std::wregex to std::vector loses some std::wregex values

Message ID CAG4ZjNk4Jn2Y2fu_zcMyWwkUbCANtQ4bUsLYfnDBy1s7=iGNpw@mail.gmail.com
State New
Headers show

Commit Message

Tim Shen Oct. 2, 2014, 6:44 a.m. UTC
On Tue, Sep 30, 2014 at 5:15 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
> Please put the PR number in the ChangeLog, so it updates Bugzilla
> automatically.

Good catch ;)

> You've spelled it "ompatibility" everywhere. I think we only need the
> note once, maybe on the move constructor or rvalue basic_regex::assign.

Sorry for misspelling :(. Done.

> This just copies but we could still make it slightly more efficient by
> moving _M_original_str (then resetting __rhs to an empty state).
>
>      basic_regex(basic_regex&& __rhs)
>      : _M_flags(__rhs._M_flags), _M_original_str(std::move(_M_original_str))
>      {
>        this->imbue(__rhs.getloc());
>        __rhs._M_automaton.reset();
>      }
>
> This is only slightly more efficient, and still needs to allocate
> memory for the NFA, so I'm not sure if it's worth it. Your call.

Yes let's do it. Done.

> Because imbue() returns the old locale you can swap them like this:
>
>        imbue(__rhs.imbue(getloc()));
>

Done.

Comments

Jonathan Wakely Oct. 2, 2014, 8:49 a.m. UTC | #1
On 01/10/14 23:44 -0700, Tim Shen wrote:
>> This just copies but we could still make it slightly more efficient by
>> moving _M_original_str (then resetting __rhs to an empty state).
>>
>>      basic_regex(basic_regex&& __rhs)
>>      : _M_flags(__rhs._M_flags), _M_original_str(std::move(_M_original_str))
>>      {
>>        this->imbue(__rhs.getloc());
>>        __rhs._M_automaton.reset();
>>      }
>>
>> This is only slightly more efficient, and still needs to allocate
>> memory for the NFA, so I'm not sure if it's worth it. Your call.
>
>Yes let's do it. Done.
>
>> Because imbue() returns the old locale you can swap them like this:
>>
>>        imbue(__rhs.imbue(getloc()));
>>
>
>Done.

Looks good - thanks.
diff mbox

Patch

diff --git a/libstdc++-v3/include/bits/regex.h b/libstdc++-v3/include/bits/regex.h
index 9dc83fd..772a209 100644
--- a/libstdc++-v3/include/bits/regex.h
+++ b/libstdc++-v3/include/bits/regex.h
@@ -474,17 +474,25 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *
        * @param __rhs A @p regex object.
        */
-      basic_regex(const basic_regex& __rhs) = default;
+      basic_regex(const basic_regex& __rhs)
+      : _M_flags(__rhs._M_flags), _M_original_str(__rhs._M_original_str)
+      { this->imbue(__rhs.getloc()); }
 
       /**
        * @brief Move-constructs a basic regular expression.
        *
        * @param __rhs A @p regex object.
+       *
+       * The implementation is a workaround concerning ABI compatibility. See:
+       * https://gcc.gnu.org/ml/libstdc++/2014-09/msg00067.html
        */
-      basic_regex(const basic_regex&& __rhs) noexcept
-      : _M_flags(__rhs._M_flags), _M_traits(__rhs._M_traits),
-	_M_automaton(std::move(__rhs._M_automaton))
-      { }
+      basic_regex(basic_regex&& __rhs)
+      : _M_flags(__rhs._M_flags),
+      _M_original_str(std::move(__rhs._M_original_str))
+      {
+	this->imbue(__rhs.getloc());
+	__rhs._M_automaton.reset();
+      }
 
       /**
        * @brief Constructs a basic regular expression from the string
@@ -555,9 +563,12 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       /**
        * @brief Move-assigns one regular expression to another.
+       *
+       * The implementation is a workaround concerning ABI compatibility. See:
+       * https://gcc.gnu.org/ml/libstdc++/2014-09/msg00067.html
        */
       basic_regex&
-      operator=(basic_regex&& __rhs) noexcept
+      operator=(basic_regex&& __rhs)
       { return this->assign(std::move(__rhs)); }
 
       /**
@@ -591,8 +602,9 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       basic_regex&
       assign(const basic_regex& __rhs)
       {
-	basic_regex __tmp(__rhs);
-	this->swap(__tmp);
+	_M_flags = __rhs._M_flags;
+	_M_original_str = __rhs._M_original_str;
+	this->imbue(__rhs.getloc());
 	return *this;
       }
 
@@ -600,13 +612,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
        * @brief The move-assignment operator.
        *
        * @param __rhs Another regular expression object.
+       *
+       * The implementation is a workaround concerning ABI compatibility. See:
+       * https://gcc.gnu.org/ml/libstdc++/2014-09/msg00067.html
        */
       basic_regex&
-      assign(basic_regex&& __rhs) noexcept
+      assign(basic_regex&& __rhs)
       {
-	basic_regex __tmp(std::move(__rhs));
-	this->swap(__tmp);
-	return *this;
+	_M_flags = __rhs._M_flags;
+	_M_original_str = std::move(__rhs._M_original_str);
+	__rhs._M_automaton.reset();
+	this->imbue(__rhs.getloc());
       }
 
       /**
@@ -751,8 +767,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       swap(basic_regex& __rhs)
       {
 	std::swap(_M_flags, __rhs._M_flags);
-	std::swap(_M_traits, __rhs._M_traits);
-	std::swap(_M_automaton, __rhs._M_automaton);
+	std::swap(_M_original_str, __rhs._M_original_str);
+	this->imbue(__rhs.imbue(this->getloc()));
       }
 
 #ifdef _GLIBCXX_DEBUG
diff --git a/libstdc++-v3/testsuite/28_regex/algorithms/regex_match/ecma/wchar_t/63199.cc b/libstdc++-v3/testsuite/28_regex/algorithms/regex_match/ecma/wchar_t/63199.cc
new file mode 100644
index 0000000..cbb23f7
--- /dev/null
+++ b/libstdc++-v3/testsuite/28_regex/algorithms/regex_match/ecma/wchar_t/63199.cc
@@ -0,0 +1,69 @@ 
+// { dg-options "-std=gnu++11" }
+
+//
+// Copyright (C) 2014 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <regex>
+#include <testsuite_hooks.h>
+#include <testsuite_regex.h>
+
+using namespace __gnu_test;
+using namespace std;
+
+// libstdc++/63199
+void
+test01()
+{
+  bool test __attribute__((unused)) = true;
+
+  std::setlocale(LC_ALL, "");
+
+  std::wstring current_token(L"II.");
+
+  std::vector<std::wregex> regex_vector;
+
+  for (int i = 0; i < 4; ++i)
+  {
+    std::regex_constants::syntax_option_type flag;
+    flag = std::regex_constants::ECMAScript | std::regex_constants::icase;
+
+    std::wregex reg;
+    reg.imbue(std::locale(""));
+    reg.assign(L"^(M*(?:CM|DC{1,3}|D|CD|C{1,3}){0,1}(?:XC|LX{1,3}|L|XL|X{1,3}){0,1}(?:IX|VI{0,3}|IV|I{1,3}){0,1}\\.)$", flag);
+
+    regex_vector.emplace_back(reg);
+  }
+
+  for (auto cit = regex_vector.cbegin(); cit != regex_vector.cend(); ++cit)
+  {
+    std::wstring::const_iterator it1 = current_token.begin();
+    std::wstring::const_iterator it2 = current_token.end();
+    std::wsmatch current_token_match;
+
+    regex_match_debug(it1, it2, current_token_match, *cit);
+    VERIFY(current_token_match[0] == current_token);
+    VERIFY(current_token_match[1] == current_token);
+  }
+}
+
+int
+main()
+{
+  test01();
+  return 0;
+}