[libcxx-commits] [libcxx] r370502 - Make `vector` unconditionally move elements when exceptions are disabled.

Eric Fiselier via libcxx-commits libcxx-commits at lists.llvm.org
Fri Aug 30 12:01:03 PDT 2019


Author: ericwf
Date: Fri Aug 30 12:01:03 2019
New Revision: 370502

URL: http://llvm.org/viewvc/llvm-project?rev=370502&view=rev
Log:
Make `vector` unconditionally move elements when exceptions are disabled.

Summary:
`std::vector<T>` is free choose between using copy or move operations when it needs to resize. The standard only candidates that the correct exception safety guarantees are provided. When exceptions are disabled these guarantees are trivially satisfied. Meaning vector is free to optimize it's implementation by moving instead of copying.

This patch makes `std::vector` unconditionally move elements when exceptions are disabled.

This optimization is conforming according to the current standard wording.

There are concerns that moving in `-fno-noexceptions`mode will be a surprise to users. For example, a user may be surprised to find their code is slower with exceptions enabled than it is disabled. I'm sympathetic to this surprised, but I don't think it should block this optimization.


Reviewers: mclow.lists, ldionne, rsmith

Reviewed By: ldionne

Subscribers: zoecarver, christof, dexonsmith, libcxx-commits

Tags: #libc

Differential Revision: https://reviews.llvm.org/D62228

Added:
    libcxx/trunk/test/libcxx/containers/sequences/vector/exception_safety_exceptions_disabled.sh.cpp
Modified:
    libcxx/trunk/include/memory
    libcxx/trunk/include/vector

Modified: libcxx/trunk/include/memory
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/memory?rev=370502&r1=370501&r2=370502&view=diff
==============================================================================
--- libcxx/trunk/include/memory (original)
+++ libcxx/trunk/include/memory Fri Aug 30 12:01:03 2019
@@ -1609,10 +1609,16 @@ struct _LIBCPP_TEMPLATE_VIS allocator_tr
         _LIBCPP_INLINE_VISIBILITY
         static
         void
-        __construct_forward(allocator_type& __a, _Ptr __begin1, _Ptr __end1, _Ptr& __begin2)
+        __construct_forward_with_exception_guarantees(allocator_type& __a, _Ptr __begin1, _Ptr __end1, _Ptr& __begin2)
         {
             for (; __begin1 != __end1; ++__begin1, (void) ++__begin2)
-                construct(__a, _VSTD::__to_raw_pointer(__begin2), _VSTD::move_if_noexcept(*__begin1));
+              construct(__a, _VSTD::__to_raw_pointer(__begin2),
+#ifdef _LIBCPP_NO_EXCEPTIONS
+                        _VSTD::move(*__begin1)
+#else
+                        _VSTD::move_if_noexcept(*__begin1)
+#endif
+                        );
         }
 
     template <class _Tp>
@@ -1625,7 +1631,7 @@ struct _LIBCPP_TEMPLATE_VIS allocator_tr
              is_trivially_move_constructible<_Tp>::value,
             void
         >::type
-        __construct_forward(allocator_type&, _Tp* __begin1, _Tp* __end1, _Tp*& __begin2)
+        __construct_forward_with_exception_guarantees(allocator_type&, _Tp* __begin1, _Tp* __end1, _Tp*& __begin2)
         {
             ptrdiff_t _Np = __end1 - __begin1;
             if (_Np > 0)
@@ -1672,12 +1678,18 @@ struct _LIBCPP_TEMPLATE_VIS allocator_tr
         _LIBCPP_INLINE_VISIBILITY
         static
         void
-        __construct_backward(allocator_type& __a, _Ptr __begin1, _Ptr __end1, _Ptr& __end2)
+        __construct_backward_with_exception_guarantees(allocator_type& __a, _Ptr __begin1, _Ptr __end1, _Ptr& __end2)
         {
             while (__end1 != __begin1)
             {
-                construct(__a, _VSTD::__to_raw_pointer(__end2-1), _VSTD::move_if_noexcept(*--__end1));
-                --__end2;
+              construct(__a, _VSTD::__to_raw_pointer(__end2 - 1),
+#ifdef _LIBCPP_NO_EXCEPTIONS
+                        _VSTD::move(*--__end1)
+#else
+                        _VSTD::move_if_noexcept(*--__end1)
+#endif
+                        );
+              --__end2;
             }
         }
 
@@ -1691,7 +1703,7 @@ struct _LIBCPP_TEMPLATE_VIS allocator_tr
              is_trivially_move_constructible<_Tp>::value,
             void
         >::type
-        __construct_backward(allocator_type&, _Tp* __begin1, _Tp* __end1, _Tp*& __end2)
+        __construct_backward_with_exception_guarantees(allocator_type&, _Tp* __begin1, _Tp* __end1, _Tp*& __end2)
         {
             ptrdiff_t _Np = __end1 - __begin1;
             __end2 -= _Np;

Modified: libcxx/trunk/include/vector
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/vector?rev=370502&r1=370501&r2=370502&view=diff
==============================================================================
--- libcxx/trunk/include/vector (original)
+++ libcxx/trunk/include/vector Fri Aug 30 12:01:03 2019
@@ -948,7 +948,8 @@ void
 vector<_Tp, _Allocator>::__swap_out_circular_buffer(__split_buffer<value_type, allocator_type&>& __v)
 {
     __annotate_delete();
-    __alloc_traits::__construct_backward(this->__alloc(), this->__begin_, this->__end_, __v.__begin_);
+    __alloc_traits::__construct_backward_with_exception_guarantees(
+        this->__alloc(), this->__begin_, this->__end_, __v.__begin_);
     _VSTD::swap(this->__begin_, __v.__begin_);
     _VSTD::swap(this->__end_, __v.__end_);
     _VSTD::swap(this->__end_cap(), __v.__end_cap());
@@ -963,8 +964,10 @@ vector<_Tp, _Allocator>::__swap_out_circ
 {
     __annotate_delete();
     pointer __r = __v.__begin_;
-    __alloc_traits::__construct_backward(this->__alloc(), this->__begin_, __p, __v.__begin_);
-    __alloc_traits::__construct_forward(this->__alloc(), __p, this->__end_, __v.__end_);
+    __alloc_traits::__construct_backward_with_exception_guarantees(
+        this->__alloc(), this->__begin_, __p, __v.__begin_);
+    __alloc_traits::__construct_forward_with_exception_guarantees(
+        this->__alloc(), __p, this->__end_, __v.__end_);
     _VSTD::swap(this->__begin_, __v.__begin_);
     _VSTD::swap(this->__end_, __v.__end_);
     _VSTD::swap(this->__end_cap(), __v.__end_cap());

Added: libcxx/trunk/test/libcxx/containers/sequences/vector/exception_safety_exceptions_disabled.sh.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/containers/sequences/vector/exception_safety_exceptions_disabled.sh.cpp?rev=370502&view=auto
==============================================================================
--- libcxx/trunk/test/libcxx/containers/sequences/vector/exception_safety_exceptions_disabled.sh.cpp (added)
+++ libcxx/trunk/test/libcxx/containers/sequences/vector/exception_safety_exceptions_disabled.sh.cpp Fri Aug 30 12:01:03 2019
@@ -0,0 +1,57 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// RUN: %build -fno-exceptions
+// RUN: %run
+
+// UNSUPPORTED: c++98, c++03
+
+// <vector>
+
+// Test that vector always moves elements when exceptions are disabled.
+// vector is allowed to move or copy elements while resizing, so long as
+// it still provides the strong exception safety guarantee.
+
+#include <vector>
+#include <cassert>
+
+#include "test_macros.h"
+
+#ifndef TEST_HAS_NO_EXCEPTIONS
+#error exceptions should be disabled.
+#endif
+
+bool allow_moves = false;
+
+class A {
+public:
+  A() {}
+  A(A&&) { assert(allow_moves); }
+  explicit A(int) {}
+  A(A const&) { assert(false); }
+};
+
+int main(int, char**) {
+  std::vector<A> v;
+
+  // Create a vector containing some number of elements that will
+  // have to be moved when it is resized.
+  v.reserve(10);
+  size_t old_cap = v.capacity();
+  for (int i = 0; i < v.capacity(); ++i) {
+    v.emplace_back(42);
+  }
+  assert(v.capacity() == old_cap);
+  assert(v.size() == v.capacity());
+
+  // The next emplace back should resize.
+  allow_moves = true;
+  v.emplace_back(42);
+
+  return 0;
+}




More information about the libcxx-commits mailing list