[v3 PATCH] Make stateful allocator propagation more consistent for operator+(basic_string) (P1165R1)

[Jonathan Wakely]

On 02/05/19 17:54 +0100, Nina Dinka Ranns wrote:
>Tested on Linux x86_64
>Make stateful allocator propagation more consistent for
>operator+(basic_string) (P1165R1)
>
>2019-05-01  Nina Dinka Ranns  <dinka.ranns@gmail.com>
>        Make stateful allocator propagation more consistent for
>operator+(basic_string) (P1165R1)
>        * 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

The next change is in include/bits/basic_string.h but that file isn't
named here:

>         (operator+(basic_string&&,basic_string&&) : Changed resulting
>allocator to always be the one from the first parameter
>        * testsuite/21_strings/basic_string/allocator/char/operator_plus.cc: New
>        * testsuite/21_strings/basic_string/allocator/wchar_t/operator_plus.cc
>: New

>Index: libstdc++-v3/include/bits/basic_string.h
>===================================================================
>--- libstdc++-v3/include/bits/basic_string.h	(revision 270655)
>+++ libstdc++-v3/include/bits/basic_string.h	(working copy)
>@@ -6096,13 +6096,7 @@
>     inline basic_string<_CharT, _Traits, _Alloc>
>     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));
>-    }
>+    { return std::move(__lhs.append(__rhs)); }

I know we already discussed this off-list and said just using the LHS
is correct here, even if though it means losing the optimization, but
something occured to me today.

If the allocator traits say is_always_equal is true, or if the two
allocators happen to be equal, then we could still do this
optimization to use the RHS string if doing so will avoid a
reallocation.

So something like this (untested):

    {
      using _Alloc_traits = allocator_traits<_Alloc>;
      bool __use_rhs = false;
      if _GLIBCXX17_CONSTEXPR (_Alloc_traits::is_always_equal())
        __use_rhs = true;
      else if (__lhs.get_allocator() == __rhs.get_allocator())
        __use_rhs = true;
      if (__use_rhs)
        {
          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));
    }

I believe that should pass all your existing tests, because it will
only use the RHS string when the allocators are equal, and so you
can't detect whether the allocator from the LHS or RHS was used.

Otherwise the patch looks good.