[committed] libstdc++: Simplify std::string constructors

[Jonathan Wakely <jwakely@redhat.com>]

Tested powerpc64le-linux, pushed to trunk.


Several std::basic_string constructors dispatch to one of the
two-argument overloads of _M_construct, which then dispatches again to
_M_construct_aux to detect whether the arguments are iterators or not.
That then dispatches to one of _M_construct(size_type, char_type) or
_M_construct(Iter, Iter, iterator_traits<Iter>::iterator_category{}).

For most of those constructors this is a waste of time, because we know
the arguments are already iterators. For basic_string(const CharT*) and
basic_string(initializer_list<C>) we know that we call _M_construct with
two pointers, and for basic_string(const basic_string&) we call it with
two const_iterators.  Those constructors can call the three-argument
overload of _M_construct with the iterator category tag right away,
without the intermediate dispatching.

The case where this doesn't apply is basic_string(InputIter, InputIter),
but for C++11 and later this is constrained so we know it's an iterator
here as well. We can restrict the dispatching in this constructor to
only be done for C++98 and to call _M_construct_aux directly, which
allows us to remove the two-argument _M_construct(InputIter, InputIter)
overload entirely.

N.B. When calling the three-arg _M_construct with pointers or string
iterators, we pass forward_iterator_tag not random_access_iterator_tag.
This is because it makes no difference which overload gets called, and
simplifies overload resolution to not have to do a base-to-derived
check. If we ever add a new overload of M_construct for random access
iterators we would have to revisit this, but that seems unlikely.

This patch also moves the __is_null_pointer checks from the three-arg
_M_construct into the constructors where a null pointer argument is
actually possible. This avoids redundant checks where we know we have a
non-null pointer, or don't have a pointer at all.

Finally, this patch replaces some try-blocks with an RAII type, so that
memory is deallocated during unwinding. This avoids the overhead of
catching and rethrowing an exception.

libstdc++-v3/ChangeLog:

	* include/bits/basic_string.h (_M_construct_aux): Only define
	for C++98. Remove constexpr.
	(_M_construct_aux_2): Likewise.
	(_M_construct(InputIter, InputIter)): Remove.
	(basic_string(const basic_string&)): Call _M_construct with
	iterator category argument.
	(basic_string(const basic_string&, size_type, const Alloc&)):
	Likewise.
	(basic_string(const basic_string&, size_type, size_type)):
	Likewise.
	(basic_string(const charT*, size_type, const Alloc&)): Likewise.
	Check for null pointer.
	(basic_string(const charT*, const Alloc&)): Likewise.
	(basic_string(initializer_list<charT>, const Alloc&)): Call
	_M_construct with iterator category argument.
	(basic_string(const basic_string&, const Alloc&)): Likewise.
	(basic_string(basic_string&&, const Alloc&)): Likewise.
	(basic_string(_InputIter, _InputIter, const Alloc&)): Likewise
	for C++11 and later, call _M_construct_aux for C++98.
	* include/bits/basic_string.tcc
	(_M_construct(I, I, input_iterator_tag)): Replace try-block with
	RAII type.
	(_M_construct(I, I, forward_iterator_tag)): Likewise. Remove
	__is_null_pointer check.
---
 libstdc++-v3/include/bits/basic_string.h   | 61 +++++++++++--------
 libstdc++-v3/include/bits/basic_string.tcc | 69 ++++++++++++----------
 2 files changed, 74 insertions(+), 56 deletions(-)

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index 9d281f5daf2..d29c9cdc410 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -262,10 +262,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       _M_destroy(size_type __size) throw()
       { _Alloc_traits::deallocate(_M_get_allocator(), _M_data(), __size + 1); }
 
+#if __cplusplus < 201103L || defined _GLIBCXX_DEFINING_STRING_INSTANTIATIONS
       // _M_construct_aux is used to implement the 21.3.1 para 15 which
       // requires special behaviour if _InIterator is an integral type
       template<typename _InIterator>
-	_GLIBCXX20_CONSTEXPR
         void
         _M_construct_aux(_InIterator __beg, _InIterator __end,
 			 std::__false_type)
@@ -277,24 +277,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
       // 438. Ambiguity in the "do the right thing" clause
       template<typename _Integer>
-	_GLIBCXX20_CONSTEXPR
         void
         _M_construct_aux(_Integer __beg, _Integer __end, std::__true_type)
 	{ _M_construct_aux_2(static_cast<size_type>(__beg), __end); }
 
-      _GLIBCXX20_CONSTEXPR
       void
       _M_construct_aux_2(size_type __req, _CharT __c)
       { _M_construct(__req, __c); }
-
-      template<typename _InIterator>
-	_GLIBCXX20_CONSTEXPR
-        void
-        _M_construct(_InIterator __beg, _InIterator __end)
-	{
-	  typedef typename std::__is_integer<_InIterator>::__type _Integral;
-	  _M_construct_aux(__beg, __end, _Integral());
-        }
+#endif
 
       // For Input Iterators, used in istreambuf_iterators, etc.
       template<typename _InIterator>
@@ -514,7 +504,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       basic_string(const basic_string& __str)
       : _M_dataplus(_M_local_data(),
 		    _Alloc_traits::_S_select_on_copy(__str._M_get_allocator()))
-      { _M_construct(__str._M_data(), __str._M_data() + __str.length()); }
+      {
+	_M_construct(__str._M_data(), __str._M_data() + __str.length(),
+		     std::forward_iterator_tag());
+      }
 
       // _GLIBCXX_RESOLVE_LIB_DEFECTS
       // 2583. no way to supply an allocator for basic_string(str, pos)
@@ -531,7 +524,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       {
 	const _CharT* __start = __str._M_data()
 	  + __str._M_check(__pos, "basic_string::basic_string");
-	_M_construct(__start, __start + __str._M_limit(__pos, npos));
+	_M_construct(__start, __start + __str._M_limit(__pos, npos),
+		     std::forward_iterator_tag());
       }
 
       /**
@@ -547,7 +541,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       {
 	const _CharT* __start = __str._M_data()
 	  + __str._M_check(__pos, "basic_string::basic_string");
-	_M_construct(__start, __start + __str._M_limit(__pos, __n));
+	_M_construct(__start, __start + __str._M_limit(__pos, __n),
+		     std::forward_iterator_tag());
       }
 
       /**
@@ -564,7 +559,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       {
 	const _CharT* __start
 	  = __str._M_data() + __str._M_check(__pos, "string::string");
-	_M_construct(__start, __start + __str._M_limit(__pos, __n));
+	_M_construct(__start, __start + __str._M_limit(__pos, __n),
+		     std::forward_iterator_tag());
       }
 
       /**
@@ -580,7 +576,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       basic_string(const _CharT* __s, size_type __n,
 		   const _Alloc& __a = _Alloc())
       : _M_dataplus(_M_local_data(), __a)
-      { _M_construct(__s, __s + __n); }
+      {
+	// NB: Not required, but considered best practice.
+	if (__s == 0 && __n > 0)
+	  std::__throw_logic_error(__N("basic_string: "
+				       "construction from null is not valid"));
+	_M_construct(__s, __s + __n, std::forward_iterator_tag());
+      }
 
       /**
        *  @brief  Construct string as copy of a C string.
@@ -596,10 +598,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       basic_string(const _CharT* __s, const _Alloc& __a = _Alloc())
       : _M_dataplus(_M_local_data(), __a)
       {
-	const _CharT* __end = __s ? __s + traits_type::length(__s)
-	  // We just need a non-null pointer here to get an exception:
-	  : reinterpret_cast<const _CharT*>(__alignof__(_CharT));
-	_M_construct(__s, __end, random_access_iterator_tag());
+	// NB: Not required, but considered best practice.
+	if (__s == 0)
+	  std::__throw_logic_error(__N("basic_string: "
+				       "construction from null is not valid"));
+	const _CharT* __end = __s + traits_type::length(__s);
+	_M_construct(__s, __end, forward_iterator_tag());
       }
 
       /**
@@ -657,12 +661,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       _GLIBCXX20_CONSTEXPR
       basic_string(initializer_list<_CharT> __l, const _Alloc& __a = _Alloc())
       : _M_dataplus(_M_local_data(), __a)
-      { _M_construct(__l.begin(), __l.end()); }
+      { _M_construct(__l.begin(), __l.end(), std::forward_iterator_tag()); }
 
       _GLIBCXX20_CONSTEXPR
       basic_string(const basic_string& __str, const _Alloc& __a)
       : _M_dataplus(_M_local_data(), __a)
-      { _M_construct(__str.begin(), __str.end()); }
+      { _M_construct(__str.begin(), __str.end(), std::forward_iterator_tag()); }
 
       _GLIBCXX20_CONSTEXPR
       basic_string(basic_string&& __str, const _Alloc& __a)
@@ -686,7 +690,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	    __str._M_set_length(0);
 	  }
 	else
-	  _M_construct(__str.begin(), __str.end());
+	  _M_construct(__str.begin(), __str.end(), std::forward_iterator_tag());
       }
 
       basic_string(nullptr_t) = delete;
@@ -709,7 +713,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
         basic_string(_InputIterator __beg, _InputIterator __end,
 		     const _Alloc& __a = _Alloc())
 	: _M_dataplus(_M_local_data(), __a)
-	{ _M_construct(__beg, __end); }
+	{
+#if __cplusplus >= 201103L
+	  _M_construct(__beg, __end, std::__iterator_category(__beg));
+#else
+	  typedef typename std::__is_integer<_InputIterator>::__type _Integral;
+	  _M_construct_aux(__beg, __end, _Integral());
+#endif
+	}
 
 #if __cplusplus >= 201703L
       /**
diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index 5a51f7e21b5..9a54b63b933 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -178,29 +178,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    ++__beg;
 	  }
 
-	__try
+	struct _Guard
+	{
+	  _GLIBCXX20_CONSTEXPR
+	  explicit _Guard(basic_string* __s) : _M_guarded(__s) { }
+
+	  _GLIBCXX20_CONSTEXPR
+	  ~_Guard() { if (_M_guarded) _M_guarded->_M_dispose(); }
+
+	  basic_string* _M_guarded;
+	} __guard(this);
+
+	while (__beg != __end)
 	  {
-	    while (__beg != __end)
+	    if (__len == __capacity)
 	      {
-		if (__len == __capacity)
-		  {
-		    // Allocate more space.
-		    __capacity = __len + 1;
-		    pointer __another = _M_create(__capacity, __len);
-		    this->_S_copy(__another, _M_data(), __len);
-		    _M_dispose();
-		    _M_data(__another);
-		    _M_capacity(__capacity);
-		  }
-		_M_data()[__len++] = *__beg;
-		++__beg;
+		// Allocate more space.
+		__capacity = __len + 1;
+		pointer __another = _M_create(__capacity, __len);
+		this->_S_copy(__another, _M_data(), __len);
+		_M_dispose();
+		_M_data(__another);
+		_M_capacity(__capacity);
 	      }
+	    _M_data()[__len++] = *__beg;
+	    ++__beg;
 	  }
-	__catch(...)
-	  {
-	    _M_dispose();
-	    __throw_exception_again;
-	  }
+
+	__guard._M_guarded = 0;
 
 	_M_set_length(__len);
       }
@@ -213,11 +218,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _M_construct(_InIterator __beg, _InIterator __end,
 		   std::forward_iterator_tag)
       {
-	// NB: Not required, but considered best practice.
-	if (__gnu_cxx::__is_null_pointer(__beg) && __beg != __end)
-	  std::__throw_logic_error(__N("basic_string::"
-				       "_M_construct null not valid"));
-
 	size_type __dnew = static_cast<size_type>(std::distance(__beg, __end));
 
 	if (__dnew > size_type(_S_local_capacity))
@@ -229,13 +229,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  _M_use_local_data();
 
 	// Check for out_of_range and length_error exceptions.
-	__try
-	  { this->_S_copy_chars(_M_data(), __beg, __end); }
-	__catch(...)
-	  {
-	    _M_dispose();
-	    __throw_exception_again;
-	  }
+	struct _Guard
+	{
+	  _GLIBCXX20_CONSTEXPR
+	  explicit _Guard(basic_string* __s) : _M_guarded(__s) { }
+
+	  _GLIBCXX20_CONSTEXPR
+	  ~_Guard() { if (_M_guarded) _M_guarded->_M_dispose(); }
+
+	  basic_string* _M_guarded;
+	} __guard(this);
+
+	this->_S_copy_chars(_M_data(), __beg, __end);
+
+	__guard._M_guarded = 0;
 
 	_M_set_length(__dnew);
       }
-- 
2.31.1