On 07/05/19 12:22 +0100, Jonathan Wakely wrote:
On 07/05/19 12:01 +0100, Nina Dinka Ranns wrote:
Ack. I've put the use of _Alloc_traits::is_always_equal within #if
__cplusplus >= 201703L block since it is officially a C++17 feature.
Let me know if you think that's an overkill.
Yes, that's overkill, we provide is_always_equal unconditionally from
C++11 onwards (to avoid ODR violations in code using different -std
options). Since it's defined fo C++11 we can use it for C++11.
I can remove that #if and test and commit the result for you though,
no need for another revision of the patch.
New changelog below. I didn't change the description of
operator+(basic_string&&,basic_string&&) as it's still technically
always resulting in an allocator from the first parameter.
Yes, that looks fine. Thanks!
I realised that for !_GLIBCXX_USE_CXX11_ABI we can always use the
optimization, because we don't support stateful allocators for the old
std::string ABI. So I adjusted the patch slightly for that.
I've attached what I'll commit.
Tested powerpc64le-linux (old and new ABI), committing to trunk.
commit 409ae90af9472e649df14c8d8147225d1cfa20ea
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Tue May 7 15:21:47 2019 +0100
Make allocator propagation more consistent for operator+(basic_string) (P1165R1)
2019-05-01 Nina Dinka Ranns <dinka.ra...@gmail.com>
Make allocator propagation more consistent for
operator+(basic_string) (P1165R1)
* include/bits/basic_string.h
(operator+(basic_string&&, basic_string&&): Changed resulting
allocator to always be the one from the first parameter.
* include/bits/basic_string.tcc
(operator+(const _CharT*, const basic_string&)): Changed
resulting allocator to be SOCCC on the second parameter's allocator.
(operator+(_CharT, const basic_string&)): Likewise.
* testsuite/21_strings/basic_string/allocator/char/operator_plus.cc:
New.
* testsuite/21_strings/basic_string/allocator/wchar_t/operator_plus.cc:
New.
diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index 4a6332a8968..5ebe86bad7d 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -6096,11 +6096,21 @@ _GLIBCXX_END_NAMESPACE_CXX11
operator+(basic_string<_CharT, _Traits, _Alloc>&& __lhs,
basic_string<_CharT, _Traits, _Alloc>&& __rhs)
{
- const auto __size = __lhs.size() + __rhs.size();
- const bool __cond = (__size > __lhs.capacity()
- && __size <= __rhs.capacity());
- return __cond ? std::move(__rhs.insert(0, __lhs))
- : std::move(__lhs.append(__rhs));
+#if _GLIBCXX_USE_CXX11_ABI
+ using _Alloc_traits = allocator_traits<_Alloc>;
+ bool __use_rhs = false;
+ if _GLIBCXX17_CONSTEXPR (typename _Alloc_traits::is_always_equal{})
+ __use_rhs = true;
+ else if (__lhs.get_allocator() == __rhs.get_allocator())
+ __use_rhs = true;
+ if (__use_rhs)
+#endif
+ {
+ const auto __size = __lhs.size() + __rhs.size();
+ if (__size > __lhs.capacity() && __size <= __rhs.capacity())
+ return std::move(__rhs.insert(0, __lhs));
+ }
+ return std::move(__lhs.append(__rhs));
}
template<typename _CharT, typename _Traits, typename _Alloc>
diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index 314b8fe207f..e2315cb1467 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -1161,8 +1161,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__glibcxx_requires_string(__lhs);
typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
typedef typename __string_type::size_type __size_type;
+ typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
+ rebind<_CharT>::other _Char_alloc_type;
+ typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
const __size_type __len = _Traits::length(__lhs);
- __string_type __str;
+ __string_type __str(_Alloc_traits::_S_select_on_copy(
+ __rhs.get_allocator()));
__str.reserve(__len + __rhs.size());
__str.append(__lhs, __len);
__str.append(__rhs);
@@ -1175,7 +1179,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
typedef basic_string<_CharT, _Traits, _Alloc> __string_type;
typedef typename __string_type::size_type __size_type;
- __string_type __str;
+ typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template
+ rebind<_CharT>::other _Char_alloc_type;
+ typedef __gnu_cxx::__alloc_traits<_Char_alloc_type> _Alloc_traits;
+ __string_type __str(_Alloc_traits::_S_select_on_copy(
+ __rhs.get_allocator()));
const __size_type __len = __rhs.size();
__str.reserve(__len + 1);
__str.append(__size_type(1), __lhs);
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/operator_plus.cc b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/operator_plus.cc
new file mode 100644
index 00000000000..c4693378d0b
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/operator_plus.cc
@@ -0,0 +1,151 @@
+// 2019-04-30 Nina Dinka Ranns <dinka.ra...@gmail.com>
+// Copyright (C) 2019 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/>.
+
+// { dg-do run { target c++11 } }
+// COW strings don't support C++11 allocators:
+// { dg-require-effective-target cxx11-abi }
+
+#include <string>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+#include <ext/throw_allocator.h>
+
+using C = char;
+using traits = std::char_traits<C>;
+
+using __gnu_test::propagating_allocator;
+
+void test01()
+{
+ typedef propagating_allocator<C, true> alloc_type;
+ typedef std::basic_string<C, traits, alloc_type> test_type;
+
+ test_type v1("something",alloc_type(1));
+ test_type v2("something",alloc_type(2));
+ auto r1 = v1 + v2;
+ VERIFY(r1.get_allocator().get_personality() == 1);
+
+ auto r2 = v1 + std::move(v2);
+ VERIFY(r2.get_allocator().get_personality() == 2);
+
+ test_type v3("something", alloc_type(3));
+ test_type v4("something", alloc_type(4));
+ auto r3 = std::move(v3) + v4;
+ VERIFY(r3.get_allocator().get_personality() == 3);
+
+ auto r4 = std::move(v1) +std::move(v4);
+ VERIFY(r4.get_allocator().get_personality() == 1);
+
+ test_type v5("something", alloc_type(5));
+ auto r5 = v5 + "str";
+ VERIFY(r5.get_allocator().get_personality() == 5);
+
+ auto r6 = v5 + 'c';
+ VERIFY(r6.get_allocator().get_personality() == 5);
+
+ auto r7 = std::move(v5) + "str";
+ VERIFY(r7.get_allocator().get_personality() == 5);
+
+ test_type v6("something", alloc_type(6));
+ auto r8 = std::move(v6) + 'c';
+ VERIFY(r8.get_allocator().get_personality() == 6);
+
+ test_type v7("something", alloc_type(7));
+ auto r9 = "str" + v7;
+ VERIFY(r9.get_allocator().get_personality() == 7);
+
+ auto r10 = 'c' + v7;
+ VERIFY(r10.get_allocator().get_personality() == 7);
+
+ auto r11 = "str" + std::move(v7);
+ VERIFY(r11.get_allocator().get_personality() == 7);
+
+ test_type v8("something", alloc_type(8));
+ auto r12 = 'c' + std::move(v8);
+ VERIFY(r12.get_allocator().get_personality() == 8);
+}
+void test02()
+{
+ typedef propagating_allocator<C, false> alloc_type;
+ typedef std::basic_string<C, traits, alloc_type> test_type;
+
+ test_type v1("something",alloc_type(1));
+ test_type v2("something",alloc_type(2));
+ auto r1 = v1 + v2;
+ VERIFY(r1.get_allocator().get_personality() != 1);
+
+ auto r2 = v1 + std::move(v2);
+ VERIFY(r2.get_allocator().get_personality() == 2);
+
+ test_type v3("something", alloc_type(3));
+ test_type v4("something", alloc_type(4));
+ auto r3 = std::move(v3) + v4;
+ VERIFY(r3.get_allocator().get_personality() == 3);
+
+ auto r4 = std::move(v1) +std::move(v4);
+ VERIFY(r4.get_allocator().get_personality() == 1);
+
+ test_type v5("something", alloc_type(5));
+ auto r5 = v5 + "str";
+ VERIFY(r5.get_allocator().get_personality() != 5);
+
+ auto r6 = v5 + 'c';
+ VERIFY(r6.get_allocator().get_personality() != 5);
+
+ auto r7 = std::move(v5) + "str";
+ VERIFY(r7.get_allocator().get_personality() == 5);
+
+ test_type v6("something", alloc_type(6));
+ auto r8 = std::move(v6) + 'c';
+ VERIFY(r8.get_allocator().get_personality() == 6);
+
+ test_type v7("something", alloc_type(7));
+ auto r9 = "str" + v7;
+ VERIFY(r9.get_allocator().get_personality() != 7);
+
+ auto r10 = 'c' + v7;
+ VERIFY(r10.get_allocator().get_personality() != 7);
+
+ auto r11 = "str" + std::move(v7);
+ VERIFY(r11.get_allocator().get_personality() == 7);
+
+ test_type v8("something", alloc_type(8));
+ auto r12 = 'c' + std::move(v8);
+ VERIFY(r12.get_allocator().get_personality() == 8);
+}
+void test03()
+{
+ typedef propagating_allocator<C, false> alloc_type;
+ typedef std::basic_string<C, traits, alloc_type> test_type;
+
+ test_type v1("s",alloc_type(1));
+ v1.resize(10);
+ v1.shrink_to_fit();
+ test_type v2(10000,'x',alloc_type(2));
+ v2.reserve(10010);
+
+ auto r=std::move(v1)+std::move(v2);
+ VERIFY(r.get_allocator().get_personality() == 1);
+}
+int main()
+{
+ test01();
+ test02();
+ test03();
+ return 0;
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/operator_plus.cc b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/operator_plus.cc
new file mode 100644
index 00000000000..3e1d0c3fbad
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/operator_plus.cc
@@ -0,0 +1,152 @@
+// 2019-04-30 Nina Dinka Ranns <dinka.ra...@gmail.com>
+// Copyright (C) 2019 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/>.
+
+// { dg-do run { target c++11 } }
+// COW strings don't support C++11 allocators:
+// { dg-require-effective-target cxx11-abi }
+
+#include <string>
+#include <testsuite_hooks.h>
+#include <testsuite_allocator.h>
+#include <ext/throw_allocator.h>
+
+using C = wchar_t;
+using traits = std::char_traits<C>;
+
+using __gnu_test::propagating_allocator;
+
+void test01()
+{
+ typedef propagating_allocator<C, true> alloc_type;
+ typedef std::basic_string<C, traits, alloc_type> test_type;
+
+ test_type v1(L"something",alloc_type(1));
+ test_type v2(L"something",alloc_type(2));
+ auto r1 = v1 + v2;
+ VERIFY(r1.get_allocator().get_personality() == 1);
+
+ auto r2 = v1 + std::move(v2);
+ VERIFY(r2.get_allocator().get_personality() == 2);
+
+ test_type v3(L"something", alloc_type(3));
+ test_type v4(L"something", alloc_type(4));
+ auto r3 = std::move(v3) + v4;
+ VERIFY(r3.get_allocator().get_personality() == 3);
+
+ auto r4 = std::move(v1) +std::move(v4);
+ VERIFY(r4.get_allocator().get_personality() == 1);
+
+ test_type v5(L"something", alloc_type(5));
+ auto r5 = v5 + L"str";
+ VERIFY(r5.get_allocator().get_personality() == 5);
+
+ auto r6 = v5 + L'c';
+ VERIFY(r6.get_allocator().get_personality() == 5);
+
+ auto r7 = std::move(v5) + L"str";
+ VERIFY(r7.get_allocator().get_personality() == 5);
+
+ test_type v6(L"something", alloc_type(6));
+ auto r8 = std::move(v6) + L'c';
+ VERIFY(r8.get_allocator().get_personality() == 6);
+
+ test_type v7(L"something", alloc_type(7));
+ auto r9 = L"str" + v7;
+ VERIFY(r9.get_allocator().get_personality() == 7);
+
+ auto r10 = L'c' + v7;
+ VERIFY(r10.get_allocator().get_personality() == 7);
+
+ auto r11 = L"str" + std::move(v7);
+ VERIFY(r11.get_allocator().get_personality() == 7);
+
+ test_type v8(L"something", alloc_type(8));
+ auto r12 = L'c' + std::move(v8);
+ VERIFY(r12.get_allocator().get_personality() == 8);
+}
+
+void test02()
+{
+ typedef propagating_allocator<C, false> alloc_type;
+ typedef std::basic_string<C, traits, alloc_type> test_type;
+
+ test_type v1(L"something",alloc_type(1));
+ test_type v2(L"something",alloc_type(2));
+ auto r1 = v1 + v2;
+ VERIFY(r1.get_allocator().get_personality() != 1);
+
+ auto r2 = v1 + std::move(v2);
+ VERIFY(r2.get_allocator().get_personality() == 2);
+
+ test_type v3(L"something", alloc_type(3));
+ test_type v4(L"something", alloc_type(4));
+ auto r3 = std::move(v3) + v4;
+ VERIFY(r3.get_allocator().get_personality() == 3);
+
+ auto r4 = std::move(v1) +std::move(v4);
+ VERIFY(r4.get_allocator().get_personality() == 1);
+
+ test_type v5(L"something", alloc_type(5));
+ auto r5 = v5 + L"str";
+ VERIFY(r5.get_allocator().get_personality() != 5);
+
+ auto r6 = v5 + L'c';
+ VERIFY(r6.get_allocator().get_personality() != 5);
+
+ auto r7 = std::move(v5) + L"str";
+ VERIFY(r7.get_allocator().get_personality() == 5);
+
+ test_type v6(L"something", alloc_type(6));
+ auto r8 = std::move(v6) + L'c';
+ VERIFY(r8.get_allocator().get_personality() == 6);
+
+ test_type v7(L"something", alloc_type(7));
+ auto r9 = L"str" + v7;
+ VERIFY(r9.get_allocator().get_personality() != 7);
+
+ auto r10 = L'c' + v7;
+ VERIFY(r10.get_allocator().get_personality() != 7);
+
+ auto r11 = L"str" + std::move(v7);
+ VERIFY(r11.get_allocator().get_personality() == 7);
+
+ test_type v8(L"something", alloc_type(8));
+ auto r12 = L'c' + std::move(v8);
+ VERIFY(r12.get_allocator().get_personality() == 8);
+}
+void test03()
+{
+ typedef propagating_allocator<C, false> alloc_type;
+ typedef std::basic_string<C, traits, alloc_type> test_type;
+
+ test_type v1(L"s",alloc_type(1));
+ v1.resize(10);
+ v1.shrink_to_fit();
+ test_type v2(10000,L'x',alloc_type(2));
+ v2.reserve(10010);
+
+ auto r=std::move(v1)+std::move(v2);
+ VERIFY(r.get_allocator().get_personality() == 1);
+}
+int main()
+{
+ test01();
+ test02();
+ test03();
+ return 0;
+}
