[libcxx-commits] [libcxx] 3712edb - Revert "Partially inline basic_string copy constructor in UNSTABLE"

Martijn Vels via libcxx-commits libcxx-commits at lists.llvm.org
Wed Mar 4 11:53:11 PST 2020


Author: Martijn Vels
Date: 2020-03-04T14:52:17-05:00
New Revision: 3712edb152ac9f6efb997484e3cc4e9141b61f1d

URL: https://github.com/llvm/llvm-project/commit/3712edb152ac9f6efb997484e3cc4e9141b61f1d
DIFF: https://github.com/llvm/llvm-project/commit/3712edb152ac9f6efb997484e3cc4e9141b61f1d.diff

LOG: Revert "Partially inline basic_string copy constructor in UNSTABLE"

This reverts commit 8cf76e913b867a98a9843aa1b3d782632ed5d930.

We are investigating why this causes compilation issues under -O3

Added: 
    

Modified: 
    libcxx/include/__string
    libcxx/include/string

Removed: 
    libcxx/test/libcxx/strings/basic.string/string.cons/copy_shrunk_long.pass.cpp


################################################################################
diff  --git a/libcxx/include/__string b/libcxx/include/__string
index d81301453658..45060c282d59 100644
--- a/libcxx/include/__string
+++ b/libcxx/include/__string
@@ -140,14 +140,15 @@ _LIBCPP_BEGIN_NAMESPACE_STD
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>& basic_string<_CharType>::replace(size_type, size_type, value_type const*, size_type)) \
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::size_type basic_string<_CharType>::rfind(value_type const*, size_type, size_type) const) \
   _Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__init(value_type const*, size_type, size_type)) \
+  _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::basic_string(basic_string const&)) \
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>& basic_string<_CharType>::replace(size_type, size_type, value_type const*)) \
+  _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::basic_string(basic_string const&, std::allocator<_CharType> const&)) \
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::size_type basic_string<_CharType>::find_last_not_of(value_type const*, size_type, size_type) const) \
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::~basic_string()) \
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::size_type basic_string<_CharType>::find_first_not_of(value_type const*, size_type, size_type) const) \
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>& basic_string<_CharType>::insert(size_type, size_type, value_type)) \
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>& basic_string<_CharType>::operator=(value_type)) \
   _Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__init(value_type const*, size_type)) \
-  _Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__init_copy_ctor_external(value_type const*, size_type)) \
   _Func(_LIBCPP_FUNC_VIS const _CharType& basic_string<_CharType>::at(size_type) const) \
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>& basic_string<_CharType>::insert(size_type, value_type const*, size_type)) \
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::size_type basic_string<_CharType>::find_first_of(value_type const*, size_type, size_type) const) \

diff  --git a/libcxx/include/string b/libcxx/include/string
index e8245bfc1526..a35c7b1338d1 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1549,16 +1549,6 @@ private:
     inline
     void __init(size_type __n, value_type __c);
 
-    // Slow path for the (inlined) copy constructor for 'long' strings.
-    // Always externally instantiated and not inlined.
-    // Requires that __s is zero terminated.
-    // The main reason for this function to exist is because for unstable, we
-    // want to allow inlining of the copy constructor. However, we don't want
-    // to call the __init() functions as those are marked as inline which may
-    // result in over-aggressive inlining by the compiler, where our aim is
-    // to only inline the fast path code directly in the ctor.
-    void __init_copy_ctor_external(const value_type* __s, size_type __sz);
-
     template <class _InputIterator>
     inline
     _EnableIf
@@ -1871,9 +1861,7 @@ basic_string<_CharT, _Traits, _Allocator>::basic_string(const basic_string& __st
     if (!__str.__is_long())
         __r_.first().__r = __str.__r_.first().__r;
     else
-        __init_copy_ctor_external(_VSTD::__to_address(__str.__get_long_pointer()),
-                                  __str.__get_long_size());
-
+        __init(_VSTD::__to_address(__str.__get_long_pointer()), __str.__get_long_size());
 #if _LIBCPP_DEBUG_LEVEL >= 2
     __get_db()->__insert_c(this);
 #endif
@@ -1887,8 +1875,7 @@ basic_string<_CharT, _Traits, _Allocator>::basic_string(
     if (!__str.__is_long())
         __r_.first().__r = __str.__r_.first().__r;
     else
-        __init_copy_ctor_external(_VSTD::__to_address(__str.__get_long_pointer()),
-                                  __str.__get_long_size());
+        __init(_VSTD::__to_address(__str.__get_long_pointer()), __str.__get_long_size());
 #if _LIBCPP_DEBUG_LEVEL >= 2
     __get_db()->__insert_c(this);
 #endif
@@ -1914,25 +1901,6 @@ basic_string<_CharT, _Traits, _Allocator>::basic_string(basic_string&& __str)
 #endif
 }
 
-template <class _CharT, class _Traits, class _Allocator>
-void basic_string<_CharT, _Traits, _Allocator>::__init_copy_ctor_external(
-    const value_type* __s, size_type __sz) {
-  pointer __p;
-  if (__sz < __min_cap) {
-    __p = __get_short_pointer();
-    __set_short_size(__sz);
-  } else {
-    if (__sz > max_size())
-      this->__throw_length_error();
-    size_t __cap = __recommend(__sz);
-    __p = __alloc_traits::allocate(__alloc(), __cap + 1);
-    __set_long_pointer(__p);
-    __set_long_cap(__cap + 1);
-    __set_long_size(__sz);
-  }
-  traits_type::copy(_VSTD::__to_address(__p), __s, __sz + 1);
-}
-
 template <class _CharT, class _Traits, class _Allocator>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string(basic_string&& __str, const allocator_type& __a)

diff  --git a/libcxx/test/libcxx/strings/basic.string/string.cons/copy_shrunk_long.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.cons/copy_shrunk_long.pass.cpp
deleted file mode 100644
index ec2fe6c3fdb7..000000000000
--- a/libcxx/test/libcxx/strings/basic.string/string.cons/copy_shrunk_long.pass.cpp
+++ /dev/null
@@ -1,50 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-// <string>
-
-// basic_string(const basic_string<charT,traits,Allocator>& str);
-
-#include <string>
-#include <cassert>
-
-#include "test_macros.h"
-#include "test_allocator.h"
-#include "min_allocator.h"
-
-template <class S>
-void
-test()
-{
-  // Tests that a long string holding a SSO size string results in
-  // an SSO copy constructed value.
-  S s1("1234567890123456789012345678901234567890123456789012345678901234567890");
-  s1.resize(7);
-  S s2(s1);
-  LIBCPP_ASSERT(s2.__invariants());
-  assert(s2 == s1);
-  assert(s2.capacity() < sizeof(S));
-}
-
-int main(int, char**)
-{
-    {
-    typedef test_allocator<char> A;
-    typedef std::basic_string<char, std::char_traits<char>, A> S;
-    test<S>();
-    }
-#if TEST_STD_VER >= 11
-    {
-    typedef min_allocator<char> A;
-    typedef std::basic_string<char, std::char_traits<char>, A> S;
-    test<S>();
-    }
-#endif
-
-  return 0;
-}


        


More information about the libcxx-commits mailing list