forked from llvm-mirror/libcxx
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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 git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@371867 91177308-0d34-0410-b5e6-96231b3b80d8
- Loading branch information
Showing
5 changed files
with
157 additions
and
55 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
57 changes: 57 additions & 0 deletions
57
test/libcxx/containers/sequences/vector/exception_safety_exceptions_disabled.sh.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
} |
45 changes: 0 additions & 45 deletions
45
test/std/containers/sequences/vector/vector.modifiers/resize.copy_only.pass.sh.cpp
This file was deleted.
Oops, something went wrong.
45 changes: 45 additions & 0 deletions
45
test/std/containers/sequences/vector/vector.modifiers/resize_not_move_insertable.fail.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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@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; | ||
} |