public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Jonathan Wakely <redi@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org
Subject: [gcc r14-2052] libstdc++: Use RAII in std::vector::_M_realloc_insert
Date: Fri, 23 Jun 2023 18:05:03 +0000 (GMT)	[thread overview]
Message-ID: <20230623180503.F142C3858C2F@sourceware.org> (raw)

https://gcc.gnu.org/g:dd2eb972a5b063e10c83878d5c9336a818fa8291

commit r14-2052-gdd2eb972a5b063e10c83878d5c9336a818fa8291
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jun 20 13:39:29 2023 +0100

    libstdc++: Use RAII in std::vector::_M_realloc_insert
    
    Replace the try-block with RAII types for deallocating storage and
    destroying elements.
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/vector.tcc (_M_realloc_insert): Replace try-block
            with RAII types.

Diff:
---
 libstdc++-v3/include/bits/vector.tcc | 138 ++++++++++++++++++++++-------------
 1 file changed, 87 insertions(+), 51 deletions(-)

diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc
index acd11e2dc68..cda52fbbc4a 100644
--- a/libstdc++-v3/include/bits/vector.tcc
+++ b/libstdc++-v3/include/bits/vector.tcc
@@ -458,73 +458,109 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
     _M_realloc_insert(iterator __position, const _Tp& __x)
 #endif
     {
-      const size_type __len =
-	_M_check_len(size_type(1), "vector::_M_realloc_insert");
+      const size_type __len = _M_check_len(1u, "vector::_M_realloc_insert");
       pointer __old_start = this->_M_impl._M_start;
       pointer __old_finish = this->_M_impl._M_finish;
       const size_type __elems_before = __position - begin();
       pointer __new_start(this->_M_allocate(__len));
       pointer __new_finish(__new_start);
-      __try
+
+      // RAII guard for allocated storage.
+      struct _Guard
+      {
+	pointer _M_storage;	    // Storage to deallocate
+	size_type _M_len;
+	_Tp_alloc_type& _M_alloc;
+
+	_GLIBCXX20_CONSTEXPR
+	_Guard(pointer __s, size_type __l, _Tp_alloc_type& __a)
+	: _M_storage(__s), _M_len(__l), _M_alloc(__a)
+	{ }
+
+	_GLIBCXX20_CONSTEXPR
+	~_Guard()
 	{
-	  // The order of the three operations is dictated by the C++11
-	  // case, where the moves could alter a new element belonging
-	  // to the existing vector.  This is an issue only for callers
-	  // taking the element by lvalue ref (see last bullet of C++11
-	  // [res.on.arguments]).
-	  _Alloc_traits::construct(this->_M_impl,
-				   __new_start + __elems_before,
+	  if (_M_storage)
+	    __gnu_cxx::__alloc_traits<_Tp_alloc_type>::
+	      deallocate(_M_alloc, _M_storage, _M_len);
+	}
+
+      private:
+	_Guard(const _Guard&);
+      };
+      _Guard __guard(__new_start, __len, _M_impl);
+
+      // The order of the three operations is dictated by the C++11
+      // case, where the moves could alter a new element belonging
+      // to the existing vector.  This is an issue only for callers
+      // taking the element by lvalue ref (see last bullet of C++11
+      // [res.on.arguments]).
+
+      // If this throws, the existing elements are unchanged.
 #if __cplusplus >= 201103L
-				   std::forward<_Args>(__args)...);
+      _Alloc_traits::construct(this->_M_impl,
+			       std::__to_address(__new_start + __elems_before),
+			       std::forward<_Args>(__args)...);
 #else
-				   __x);
+      _Alloc_traits::construct(this->_M_impl,
+			       __new_start + __elems_before,
+			       __x);
 #endif
-	  __new_finish = pointer();
 
 #if __cplusplus >= 201103L
-	  if _GLIBCXX17_CONSTEXPR (_S_use_relocate())
-	    {
-	      __new_finish = _S_relocate(__old_start, __position.base(),
-					 __new_start, _M_get_Tp_allocator());
+      if _GLIBCXX17_CONSTEXPR (_S_use_relocate())
+	{
+	  // Relocation cannot throw.
+	  __new_finish = _S_relocate(__old_start, __position.base(),
+				     __new_start, _M_get_Tp_allocator());
+	  ++__new_finish;
+	  __new_finish = _S_relocate(__position.base(), __old_finish,
+				     __new_finish, _M_get_Tp_allocator());
+	}
+      else
+#endif
+	{
+	  // RAII type to destroy initialized elements.
+	  struct _Guard_elts
+	  {
+	    pointer _M_first, _M_last;  // Elements to destroy
+	    _Tp_alloc_type& _M_alloc;
 
-	      ++__new_finish;
+	    _GLIBCXX20_CONSTEXPR
+	    _Guard_elts(pointer __elt, _Tp_alloc_type& __a)
+	    : _M_first(__elt), _M_last(__elt + 1), _M_alloc(__a)
+	    { }
 
-	      __new_finish = _S_relocate(__position.base(), __old_finish,
-					 __new_finish, _M_get_Tp_allocator());
-	    }
-	  else
-#endif
-	    {
-	      __new_finish
-		= std::__uninitialized_move_if_noexcept_a
-		(__old_start, __position.base(),
-		 __new_start, _M_get_Tp_allocator());
+	    _GLIBCXX20_CONSTEXPR
+	    ~_Guard_elts()
+	    { std::_Destroy(_M_first, _M_last, _M_alloc); }
 
-	      ++__new_finish;
+	  private:
+	    _Guard_elts(const _Guard_elts&);
+	  };
 
-	      __new_finish
-		= std::__uninitialized_move_if_noexcept_a
-		(__position.base(), __old_finish,
-		 __new_finish, _M_get_Tp_allocator());
-	    }
-	}
-      __catch(...)
-	{
-	  if (!__new_finish)
-	    _Alloc_traits::destroy(this->_M_impl,
-				   __new_start + __elems_before);
-	  else
-	    std::_Destroy(__new_start, __new_finish, _M_get_Tp_allocator());
-	  _M_deallocate(__new_start, __len);
-	  __throw_exception_again;
+	  // Guard the new element so it will be destroyed if anything throws.
+	  _Guard_elts __guard_elts(__new_start + __elems_before, _M_impl);
+
+	  __new_finish = std::__uninitialized_move_if_noexcept_a(
+			   __old_start, __position.base(),
+			   __new_start, _M_get_Tp_allocator());
+
+	  ++__new_finish;
+	  // Guard everything before the new element too.
+	  __guard_elts._M_first = __new_start;
+
+	  __new_finish = std::__uninitialized_move_if_noexcept_a(
+			    __position.base(), __old_finish,
+			    __new_finish, _M_get_Tp_allocator());
+
+	  // New storage has been fully initialized, destroy the old elements.
+	  __guard_elts._M_first = __old_start;
+	  __guard_elts._M_last = __old_finish;
 	}
-#if __cplusplus >= 201103L
-      if _GLIBCXX17_CONSTEXPR (!_S_use_relocate())
-#endif
-	std::_Destroy(__old_start, __old_finish, _M_get_Tp_allocator());
-      _GLIBCXX_ASAN_ANNOTATE_REINIT;
-      _M_deallocate(__old_start,
-		    this->_M_impl._M_end_of_storage - __old_start);
+      __guard._M_storage = __old_start;
+      __guard._M_len = this->_M_impl._M_end_of_storage - __old_start;
+
       this->_M_impl._M_start = __new_start;
       this->_M_impl._M_finish = __new_finish;
       this->_M_impl._M_end_of_storage = __new_start + __len;

                 reply	other threads:[~2023-06-23 18:05 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230623180503.F142C3858C2F@sourceware.org \
    --to=redi@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    --cc=libstdc++-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).