[libcxx-commits] [libcxx] Optimize __insert_with_sentinel Function in std::vector (PR #113768)

[Louis Dionne via libcxx-commits]

================
@@ -1360,27 +1360,27 @@ vector<_Tp, _Allocator>::__insert_with_sentinel(const_iterator __position, _Inpu
   for (; this->__end_ != this->__end_cap() && __first != __last; ++__first) {
     __construct_one_at_end(*__first);
   }
-  __split_buffer<value_type, allocator_type&> __v(__a);
-  if (__first != __last) {
-#if _LIBCPP_HAS_EXCEPTIONS
-    try {
-#endif // _LIBCPP_HAS_EXCEPTIONS
-      __v.__construct_at_end_with_sentinel(std::move(__first), std::move(__last));
-      difference_type __old_size = __old_last - this->__begin_;
-      difference_type __old_p    = __p - this->__begin_;
-      reserve(__recommend(size() + __v.size()));
-      __p        = this->__begin_ + __old_p;
-      __old_last = this->__begin_ + __old_size;
-#if _LIBCPP_HAS_EXCEPTIONS
-    } catch (...) {
-      erase(__make_iter(__old_last), end());
-      throw;
-    }
-#endif // _LIBCPP_HAS_EXCEPTIONS
+
+  if (__first == __last)
+    (void)std::rotate(__p, __old_last, this->__end_);
+  else {
+    __split_buffer<value_type, allocator_type&> __v(__a);
+    auto __guard =
+        std::__make_exception_guard(_AllocatorDestroyRangeReverse<allocator_type, pointer>(__a, __old_last, __end_));
----------------
ldionne wrote:

This is really subtle: this exception guard is correct, but only because it stores references to `__old_last` and `__end_`, which get updated in this function. This deserves at least a comment, since `_AllocatorDestroyRangeReverse` is a private helper defined in another header.

By the way, if you stored a copy instead of a reference, do any of our tests for `std::vector` start failing? If not, that would point out to missing test coverage.

https://github.com/llvm/llvm-project/pull/113768