[v3] Inconsistency wrt Allocators in basic_string assignment vs. basic_string::assign (LWG2579)

Message ID CAAcHv8cbs6wHKTh+WRqC+YTNrrpkg2ijgip46i9m6r7+3Ui02A@mail.gmail.com
State New
Headers show
Series
  • [v3] Inconsistency wrt Allocators in basic_string assignment vs. basic_string::assign (LWG2579)
Related show

Commit Message

Nina Dinka Ranns May 9, 2019, 8:26 a.m.
Tested on Linux x86_64
Inconsistency wrt Allocators in basic_string assignment vs.
basic_string::assign (LWG2579)

2019-05-09  Nina Dinka Ranns  <dinka.ranns@gmail.com>
        Inconsistency wrt Allocators in basic_string assignment vs.
basic_string::assign (LWG2579)
        * include/bits/basic_string.h:
         operator=(const basic_string& __str): moved allocator
decision from operator= to assign
         assign(const basic_string& __str): moved allocator decision
from operator= to assign
        * testsuite/21_strings/basic_string/allocator/char/copy_assign.cc:
Adding tests
        * testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc:
Adding tests

Comments

Jonathan Wakely May 14, 2019, 11:49 a.m. | #1
On 09/05/19 09:26 +0100, Nina Dinka Ranns wrote:
>Tested on Linux x86_64

>Inconsistency wrt Allocators in basic_string assignment vs.

>basic_string::assign (LWG2579)

>

>2019-05-09  Nina Dinka Ranns  <dinka.ranns@gmail.com>

>        Inconsistency wrt Allocators in basic_string assignment vs.

>basic_string::assign (LWG2579)

>        * include/bits/basic_string.h:

>         operator=(const basic_string& __str): moved allocator

>decision from operator= to assign

>         assign(const basic_string& __str): moved allocator decision

>from operator= to assign

>        * testsuite/21_strings/basic_string/allocator/char/copy_assign.cc:

>Adding tests

>        * testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc:

>Adding tests


Tested and committed to trunk (with some ChangeLog reformatting,
please take a look at the ChangeLog entry I committed for future
reference).

Thanks!

Patch

Index: libstdc++-v3/include/bits/basic_string.h
===================================================================
--- libstdc++-v3/include/bits/basic_string.h	(revision 270943)
+++ libstdc++-v3/include/bits/basic_string.h	(working copy)
@@ -664,35 +664,6 @@ 
       basic_string&
       operator=(const basic_string& __str)
       {
-#if __cplusplus >= 201103L
-	if (_Alloc_traits::_S_propagate_on_copy_assign())
-	  {
-	    if (!_Alloc_traits::_S_always_equal() && !_M_is_local()
-		&& _M_get_allocator() != __str._M_get_allocator())
-	      {
-		// Propagating allocator cannot free existing storage so must
-		// deallocate it before replacing current allocator.
-		if (__str.size() <= _S_local_capacity)
-		  {
-		    _M_destroy(_M_allocated_capacity);
-		    _M_data(_M_local_data());
-		    _M_set_length(0);
-		  }
-		else
-		  {
-		    const auto __len = __str.size();
-		    auto __alloc = __str._M_get_allocator();
-		    // If this allocation throws there are no effects:
-		    auto __ptr = _Alloc_traits::allocate(__alloc, __len + 1);
-		    _M_destroy(_M_allocated_capacity);
-		    _M_data(__ptr);
-		    _M_capacity(__len);
-		    _M_set_length(__len);
-		  }
-	      }
-	    std::__alloc_on_copy(_M_get_allocator(), __str._M_get_allocator());
-	  }
-#endif
 	return this->assign(__str);
       }
 
@@ -1363,6 +1334,35 @@ 
       basic_string&
       assign(const basic_string& __str)
       {
+#if __cplusplus >= 201103L
+	if (_Alloc_traits::_S_propagate_on_copy_assign())
+	  {
+	    if (!_Alloc_traits::_S_always_equal() && !_M_is_local()
+		&& _M_get_allocator() != __str._M_get_allocator())
+	      {
+		// Propagating allocator cannot free existing storage so must
+		// deallocate it before replacing current allocator.
+		if (__str.size() <= _S_local_capacity)
+		  {
+		    _M_destroy(_M_allocated_capacity);
+		    _M_data(_M_local_data());
+		    _M_set_length(0);
+		  }
+		else
+		  {
+		    const auto __len = __str.size();
+		    auto __alloc = __str._M_get_allocator();
+		    // If this allocation throws there are no effects:
+		    auto __ptr = _Alloc_traits::allocate(__alloc, __len + 1);
+		    _M_destroy(_M_allocated_capacity);
+		    _M_data(__ptr);
+		    _M_capacity(__len);
+		    _M_set_length(__len);
+		  }
+	      }
+	    std::__alloc_on_copy(_M_get_allocator(), __str._M_get_allocator());
+	  }
+#endif
 	this->_M_assign(__str);
 	return *this;
       }
Index: libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/copy_assign.cc
===================================================================
--- libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/copy_assign.cc	(revision 270943)
+++ libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/copy_assign.cc	(working copy)
@@ -133,10 +133,47 @@ 
   VERIFY( v1.get_allocator() == a2 );
 }
 
+void test04()
+{
+  // LWG2579
+  typedef propagating_allocator<C, true> alloc_type;
+
+  typedef std::basic_string<C, traits, alloc_type> test_type;
+
+  test_type v1("tralalala",alloc_type(1));
+  test_type v2("content", alloc_type(2));
+  test_type v3("content2", alloc_type(3));
+
+  v1.assign(v2);
+  v3 = v2;
+  VERIFY(2 == v1.get_allocator().get_personality());
+  VERIFY(2 == v3.get_allocator().get_personality());
+
+}
+
+void test05()
+{
+  // LWG2579
+  typedef propagating_allocator<C, false> alloc_type;
+
+  typedef std::basic_string<C, traits, alloc_type> test_type;
+
+  test_type v1("tralalala",alloc_type(1));
+  test_type v2("content", alloc_type(2));
+  test_type v3("content2", alloc_type(3));
+
+  v1.assign(v2);
+  v3 = v2;
+  VERIFY(1 == v1.get_allocator().get_personality());
+  VERIFY(3 == v3.get_allocator().get_personality());
+}
+
 int main()
 {
   test01();
   test02();
   test03();
+  test04();
+  test05();
   return 0;
 }
Index: libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc
===================================================================
--- libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc	(revision 270943)
+++ libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc	(working copy)
@@ -133,10 +133,46 @@ 
   VERIFY( v1.get_allocator() == a2 );
 }
 
+void test04()
+{
+  // LWG2579
+  typedef propagating_allocator<C, true> alloc_type;
+
+  typedef std::basic_string<C, traits, alloc_type> test_type;
+
+  test_type v1(L"tralalala",alloc_type(1));
+  test_type v2(L"content", alloc_type(2));
+  test_type v3(L"content2", alloc_type(3));
+
+  v1.assign(v2);
+  v3 = v2;
+  VERIFY(2 == v1.get_allocator().get_personality());
+  VERIFY(2 == v3.get_allocator().get_personality());
+
+}
+
+void test05()
+{
+  // LWG2579
+  typedef propagating_allocator<C, false> alloc_type;
+
+  typedef std::basic_string<C, traits, alloc_type> test_type;
+
+  test_type v1(L"tralalala",alloc_type(1));
+  test_type v2(L"content", alloc_type(2));
+  test_type v3(L"content2", alloc_type(3));
+
+  v1.assign(v2);
+  v3 = v2;
+  VERIFY(1 == v1.get_allocator().get_personality());
+  VERIFY(3 == v3.get_allocator().get_personality());
+}
 int main()
 {
   test01();
   test02();
   test03();
+  test04();
+  test05();
   return 0;
 }