[libcxx-commits] [libcxx] r371867 - Recommit r370502: Make `vector` unconditionally move elements when
Eric Fiselier via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Sep 13 09:09:33 PDT 2019
Author: ericwf
Date: Fri Sep 13 09:09:33 2019
New Revision: 371867
URL: http://llvm.org/viewvc/llvm-project?rev=371867&view=rev
Log:
Recommit r370502: Make `vector` unconditionally move elements when
exceptions are disabled.
The patch was reverted due to some confusion about non-movable types. ie
types
that explicitly delete their move constructors. However, such types do
not meet
the requirement for `MoveConstructible`, which is required by
`std::vector`:
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
libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/resize_not_move_insertable.fail.cpp
Removed:
libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/resize.copy_only.pass.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=371867&r1=371866&r2=371867&view=diff
==============================================================================
--- libcxx/trunk/include/memory (original)
+++ libcxx/trunk/include/memory Fri Sep 13 09:09:33 2019
@@ -1507,6 +1507,31 @@ struct __is_default_allocator : false_ty
template <class _Tp>
struct __is_default_allocator<_VSTD::allocator<_Tp> > : true_type {};
+
+
+template <class _Alloc,
+ bool = __has_construct<_Alloc, typename _Alloc::value_type*, typename _Alloc::value_type&&>::value && !__is_default_allocator<_Alloc>::value
+ >
+struct __is_cpp17_move_insertable;
+template <class _Alloc>
+struct __is_cpp17_move_insertable<_Alloc, true> : std::true_type {};
+template <class _Alloc>
+struct __is_cpp17_move_insertable<_Alloc, false> : std::is_move_constructible<typename _Alloc::value_type> {};
+
+template <class _Alloc,
+ bool = __has_construct<_Alloc, typename _Alloc::value_type*, const typename _Alloc::value_type&>::value && !__is_default_allocator<_Alloc>::value
+ >
+struct __is_cpp17_copy_insertable;
+template <class _Alloc>
+struct __is_cpp17_copy_insertable<_Alloc, true> : __is_cpp17_move_insertable<_Alloc> {};
+template <class _Alloc>
+struct __is_cpp17_copy_insertable<_Alloc, false> : integral_constant<bool,
+ std::is_copy_constructible<typename _Alloc::value_type>::value &&
+ __is_cpp17_move_insertable<_Alloc>::value>
+ {};
+
+
+
template <class _Alloc>
struct _LIBCPP_TEMPLATE_VIS allocator_traits
{
@@ -1609,10 +1634,18 @@ 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)
{
+ static_assert(__is_cpp17_move_insertable<allocator_type>::value,
+ "The specified type does not meet the requirements of Cpp17MoveInsertible");
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 +1658,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 +1705,20 @@ 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)
{
+ static_assert(__is_cpp17_move_insertable<allocator_type>::value,
+ "The specified type does not meet the requirements of Cpp17MoveInsertable");
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 +1732,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=371867&r1=371866&r2=371867&view=diff
==============================================================================
--- libcxx/trunk/include/vector (original)
+++ libcxx/trunk/include/vector Fri Sep 13 09:09:33 2019
@@ -947,8 +947,10 @@ template <class _Tp, class _Allocator>
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 +965,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=371867&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 Sep 13 09:09:33 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;
+}
Removed: libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/resize.copy_only.pass.sh.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/resize.copy_only.pass.sh.cpp?rev=371866&view=auto
==============================================================================
--- libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/resize.copy_only.pass.sh.cpp (original)
+++ libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/resize.copy_only.pass.sh.cpp (removed)
@@ -1,45 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-// UNSUPPORTED: c++98, c++03
-
-// RUN: %build -fno-exceptions
-// RUN: %run
-
-// RUN: %build
-// RUN: %run
-
-// UNSUPPORTED: c++98, c++03
-
-// <vector>
-
-// Test that vector won't try to call the move constructor when resizing if
-// the class has a deleted move constructor (but a working copy constructor).
-
-#include <vector>
-
-class CopyOnly {
-public:
- CopyOnly() { }
-
- CopyOnly(CopyOnly&&) = delete;
- CopyOnly& operator=(CopyOnly&&) = delete;
-
- CopyOnly(const CopyOnly&) = default;
- CopyOnly& operator=(const CopyOnly&) = default;
-};
-
-int main() {
- std::vector<CopyOnly> x;
- x.emplace_back();
-
- CopyOnly c;
- x.push_back(c);
-
- return 0;
-}
Added: libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/resize_not_move_insertable.fail.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/resize_not_move_insertable.fail.cpp?rev=371867&view=auto
==============================================================================
--- libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/resize_not_move_insertable.fail.cpp (added)
+++ libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/resize_not_move_insertable.fail.cpp Fri Sep 13 09:09:33 2019
@@ -0,0 +1,45 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+
+
+// <vector>
+
+// Test that vector produces a decent diagnostic for user types that explicitly
+// delete their move constructor. Such types don't meet the Cpp17CopyInsertible
+// requirements.
+
+#include <vector>
+
+template <int>
+class BadUserNoCookie {
+public:
+ BadUserNoCookie() { }
+
+ BadUserNoCookie(BadUserNoCookie&&) = delete;
+ BadUserNoCookie& operator=(BadUserNoCookie&&) = delete;
+
+ BadUserNoCookie(const BadUserNoCookie&) = default;
+ BadUserNoCookie& operator=(const BadUserNoCookie&) = default;
+};
+
+int main() {
+ // expected-error at memory:* 2 {{"The specified type does not meet the requirements of Cpp17MoveInsertable"}}
+ {
+
+ std::vector<BadUserNoCookie<1> > x;
+ x.emplace_back();
+ }
+ {
+ std::vector<BadUserNoCookie<2>> x;
+ BadUserNoCookie<2> c;
+ x.push_back(c);
+ }
+ return 0;
+}
More information about the libcxx-commits
mailing list