Update operator+ for basic_string (P1165R1)

[zoecarver]

Summary

This patch re-arranges the + operator overloads and implements P1165R1.

Currently, it is a bit of a WIP because I had some questions about why it is causing tests to fail. I spent a long time trying to debug why test_neg_one fails but couldn't figure it out. I thought maybe @EricWF would know, otherwise I will keep debugging it.

[mclow.lists]

How do the tests look? Do they need updating to ensure that we have the (new) correct behavior?

include/string

4168

Yes, it should, because in C++03 we don't have move semantics.

[zoecarver]

@mclow.lists Yes, I need to update the tests. I also want to try to figure out why test_neg_one is failing. I will look into that today.

include/string

4168

Sounds good. I will disable overloads that use std::move for C++03.

include/string

 operator>=(const _CharT* __lhs,
            const basic_string<_CharT, _Traits, _Allocator>& __rhs) _NOEXCEPT
 {
     return !(__lhs < __rhs);
 }
 
 // operator +
 
+// append
 template<class _CharT, class _Traits, class _Allocator>
+inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
 operator+(const basic_string<_CharT, _Traits, _Allocator>& __lhs,
           const basic_string<_CharT, _Traits, _Allocator>& __rhs)
 {
-    basic_string<_CharT, _Traits, _Allocator> __r(__lhs.get_allocator());
-    typename basic_string<_CharT, _Traits, _Allocator>::size_type __lhs_sz = __lhs.size();
-    typename basic_string<_CharT, _Traits, _Allocator>::size_type __rhs_sz = __rhs.size();
-    __r.__init(__lhs.data(), __lhs_sz, __lhs_sz + __rhs_sz);
-    __r.append(__rhs.data(), __rhs_sz);
+    basic_string<_CharT, _Traits, _Allocator> __r = __lhs;
+    __r.append(__rhs);
     return __r;
 }
 
 template<class _CharT, class _Traits, class _Allocator>
+inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
-operator+(const _CharT* __lhs , const basic_string<_CharT,_Traits,_Allocator>& __rhs)
+operator+(const basic_string<_CharT, _Traits, _Allocator>& __lhs, const _CharT* __rhs)
 {
-    basic_string<_CharT, _Traits, _Allocator> __r(__rhs.get_allocator());
-    typename basic_string<_CharT, _Traits, _Allocator>::size_type __lhs_sz = _Traits::length(__lhs);
-    typename basic_string<_CharT, _Traits, _Allocator>::size_type __rhs_sz = __rhs.size();
-    __r.__init(__lhs, __lhs_sz, __lhs_sz + __rhs_sz);
-    __r.append(__rhs.data(), __rhs_sz);
+    basic_string<_CharT, _Traits, _Allocator> __r = __lhs;
+    __r.append(__rhs);
     return __r;
-}
+} // append
 
+// append move
 template<class _CharT, class _Traits, class _Allocator>
+inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
-operator+(_CharT __lhs, const basic_string<_CharT,_Traits,_Allocator>& __rhs)
+operator+(basic_string<_CharT, _Traits, _Allocator>&& __lhs,
+          const basic_string<_CharT, _Traits, _Allocator>& __rhs)
 {
-    basic_string<_CharT, _Traits, _Allocator> __r(__rhs.get_allocator());
-    typename basic_string<_CharT, _Traits, _Allocator>::size_type __rhs_sz = __rhs.size();
-    __r.__init(&__lhs, 1, 1 + __rhs_sz);
-    __r.append(__rhs.data(), __rhs_sz);
-    return __r;
+    __lhs.append(__rhs);
+    return _VSTD::move(__lhs);
 }
 
 template<class _CharT, class _Traits, class _Allocator>
-inline
+inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
-operator+(const basic_string<_CharT, _Traits, _Allocator>& __lhs, const _CharT* __rhs)
+operator+(basic_string<_CharT, _Traits, _Allocator>&& __lhs, const _CharT* __rhs)
 {
-    basic_string<_CharT, _Traits, _Allocator> __r(__lhs.get_allocator());
-    typename basic_string<_CharT, _Traits, _Allocator>::size_type __lhs_sz = __lhs.size();
-    typename basic_string<_CharT, _Traits, _Allocator>::size_type __rhs_sz = _Traits::length(__rhs);
-    __r.__init(__lhs.data(), __lhs_sz, __lhs_sz + __rhs_sz);
-    __r.append(__rhs, __rhs_sz);
-    return __r;
+    __lhs.append(__rhs);
+    return _VSTD::move(__lhs);
 }
 
 template<class _CharT, class _Traits, class _Allocator>
+inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
-operator+(const basic_string<_CharT, _Traits, _Allocator>& __lhs, _CharT __rhs)
+operator+(basic_string<_CharT, _Traits, _Allocator>&& __lhs,
+          basic_string<_CharT, _Traits, _Allocator>&& __rhs)
 {
-    basic_string<_CharT, _Traits, _Allocator> __r(__lhs.get_allocator());
-    typename basic_string<_CharT, _Traits, _Allocator>::size_type __lhs_sz = __lhs.size();
-    __r.__init(__lhs.data(), __lhs_sz, __lhs_sz + 1);
-    __r.push_back(__rhs);
-    return __r;
-}
-
-#ifndef _LIBCPP_CXX03_LANG
+    __lhs.append(__rhs);
+    return _VSTD::move(__lhs);
+} // append move
 
+// insert move
 template<class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
-operator+(basic_string<_CharT, _Traits, _Allocator>&& __lhs, const basic_string<_CharT, _Traits, _Allocator>& __rhs)
+operator+(const basic_string<_CharT, _Traits, _Allocator>& __lhs,
+          basic_string<_CharT, _Traits, _Allocator>&& __rhs)
 {
-    return _VSTD::move(__lhs.append(__rhs));
+    __rhs.insert(0, __lhs);
+    return _VSTD::move(__rhs);
 }
 
 template<class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
-operator+(const basic_string<_CharT, _Traits, _Allocator>& __lhs, basic_string<_CharT, _Traits, _Allocator>&& __rhs)
+operator+(const _CharT* __lhs, basic_string<_CharT,_Traits,_Allocator>&& __rhs)
 {
-    return _VSTD::move(__rhs.insert(0, __lhs));
-}
+    __rhs.insert(0, __lhs);
+    return _VSTD::move(__rhs);
+} // insert move
 
+// insert 0
 template<class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
-operator+(basic_string<_CharT, _Traits, _Allocator>&& __lhs, basic_string<_CharT, _Traits, _Allocator>&& __rhs)
+operator+(const _CharT* __lhs, const basic_string<_CharT, _Traits, _Allocator>& __rhs)
 {
-    return _VSTD::move(__lhs.append(__rhs));
-}
+    basic_string<_CharT, _Traits, _Allocator> __r = __rhs;
+    __r.insert(0, __lhs);
+    return __r;
+} // insert 0
 
+// insert begin
 template<class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
-operator+(const _CharT* __lhs , basic_string<_CharT,_Traits,_Allocator>&& __rhs)
+operator+(_CharT __lhs, const basic_string<_CharT, _Traits, _Allocator>& __rhs)
 {
-    return _VSTD::move(__rhs.insert(0, __lhs));
-}
+    basic_string<_CharT, _Traits, _Allocator> __r(__rhs.get_allocator());
+    __r.insert(__r.begin(), __lhs);
+    return __r;
+} // insert begin
 
+// insert begin move
 template<class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
 operator+(_CharT __lhs, basic_string<_CharT,_Traits,_Allocator>&& __rhs)
 {
     __rhs.insert(__rhs.begin(), __lhs);
     return _VSTD::move(__rhs);
-}
+} // intert begin move
 
+// push back
 template<class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
-operator+(basic_string<_CharT, _Traits, _Allocator>&& __lhs, const _CharT* __rhs)
+operator+(const basic_string<_CharT, _Traits, _Allocator>& __lhs, _CharT __rhs)
 {
-    return _VSTD::move(__lhs.append(__rhs));
-}
+    basic_string<_CharT, _Traits, _Allocator> __r(__lhs.get_allocator());
+    __r.push_back(__rhs);
+    return __r;
+} // push back
 
+// push back move
 template<class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_INLINE_VISIBILITY
 basic_string<_CharT, _Traits, _Allocator>
 operator+(basic_string<_CharT, _Traits, _Allocator>&& __lhs, _CharT __rhs)
 {
     __lhs.push_back(__rhs);
     return _VSTD::move(__lhs);
-}
+} // push back move
 
-#endif  // _LIBCPP_CXX03_LANG
+// XXX: I don't think this should be just for _LIBCPP_CXX03_LANG
 
 // swap
 
 template<class _CharT, class _Traits, class _Allocator>
 inline _LIBCPP_INLINE_VISIBILITY
 void
 swap(basic_string<_CharT, _Traits, _Allocator>& __lhs,
      basic_string<_CharT, _Traits, _Allocator>& __rhs)

test/std/localization/locale.categories/category.numeric/locale.num.get/facet.num.get.members/test_neg_one.pass.cpp

T v = 42;
         const char* str = std_str.data();
         size_t size = std_str.size();
         std::ios_base::iostate err = ios.goodbit;
         input_iterator<const char*> iter =
             f.get(input_iterator<const char*>(str),
                   input_iterator<const char*>(str+size+1),
                   ios, err, v);
         assert(iter.base() == str+size);
-        assert(err == ios.goodbit);
+        assert(err == ios.goodbit);  // XXX: This fails but I cannot figure out why.
         T expected = -value;
         assert(v == expected);
     }
     v = 42;
     {
         T value = std::numeric_limits<SignedT>::max();
         ++value;
         ++value;