On Tue, Apr 29, 2025 at 10:49 AM Jonathan Wakely <jwak...@redhat.com> wrote:
> I made a last-minute change to Nina's r10-200-gf4e678ef74b272
> implementation of P1165R1 (consistent allocator propagation for
> operator+ on strings), so that the rvalue+rvalue case assumes that COW
> strings do not support stateful allocators. I don't think that was true
> when the change went in, and certainly isn't true now. COW strings don't
> support allocator propagation on assignment and swap, but they do
> support non-equal stateful allocators, which are correctly propagated on
> move construction.
>
> This removes the preprocessor conditional in the rvalue+rvalue overload
> so that COW strings are handled equivalently. Also use constexpr-if
> unconditionally, disabling diagnostics with pragmas.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/basic_string.h (operator+(string&&, string&&)):
> Do not assume that COW strings have equal allocators. Use
> constexpr-if unconditionally.
> *
> testsuite/21_strings/basic_string/allocator/char/operator_plus.cc:
> Remove { dg-require-effective-target cxx11_abi }.
> *
> testsuite/21_strings/basic_string/allocator/wchar_t/operator_plus.cc:
> Likewise.
> ---
>
> Tested x86_64-linux.
>
LGTM.
>
> libstdc++-v3/include/bits/basic_string.h | 10 ++++++----
> .../basic_string/allocator/char/operator_plus.cc | 2 --
> .../basic_string/allocator/wchar_t/operator_plus.cc | 2 --
> 3 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/basic_string.h
> b/libstdc++-v3/include/bits/basic_string.h
> index c90bd099b63..a087e637805 100644
> --- a/libstdc++-v3/include/bits/basic_string.h
> +++ b/libstdc++-v3/include/bits/basic_string.h
> @@ -3938,21 +3938,23 @@ _GLIBCXX_END_NAMESPACE_CXX11
> operator+(basic_string<_CharT, _Traits, _Alloc>&& __lhs,
> basic_string<_CharT, _Traits, _Alloc>&& __rhs)
> {
> -#if _GLIBCXX_USE_CXX11_ABI
> - using _Alloc_traits = allocator_traits<_Alloc>;
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
> + // Return value must use __lhs.get_allocator(), but if __rhs has
> equal
> + // allocator then we can choose which parameter to modify in-place.
> bool __use_rhs = false;
> - if _GLIBCXX17_CONSTEXPR (typename _Alloc_traits::is_always_equal{})
> + if constexpr (allocator_traits<_Alloc>::is_always_equal::value)
> __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));
> +#pragma GCC diagnostic pop
> }
>
> template<typename _CharT, typename _Traits, typename _Alloc>
> 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
> index 571f8535176..92e05690b19 100644
> ---
> 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
> @@ -17,8 +17,6 @@
> // <http://www.gnu.org/licenses/>.
>
> // { dg-do run { target c++11 } }
> -// COW strings don't support C++11 allocator propagation:
> -// { dg-require-effective-target cxx11_abi }
>
> #include <string>
> #include <testsuite_hooks.h>
> 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
> index 0da684360ab..b75b26ae85c 100644
> ---
> 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
> @@ -17,8 +17,6 @@
> // <http://www.gnu.org/licenses/>.
>
> // { dg-do run { target c++11 } }
> -// COW strings don't support C++11 allocator propagation:
> -// { dg-require-effective-target cxx11_abi }
>
> #include <string>
> #include <testsuite_hooks.h>
> --
> 2.49.0
>
>
