* [committed] libstdc++: Simplify std::string constructors
@ 2021-11-17 17:36 Jonathan Wakely
0 siblings, 0 replies; only message in thread
From: Jonathan Wakely @ 2021-11-17 17:36 UTC (permalink / raw)
To: libstdc++, gcc-patches
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
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2021-11-17 17:36 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17 17:36 [committed] libstdc++: Simplify std::string constructors Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).