Commit dd89c56c by Aaron Jacobs Committed by Copybara-Service

inlined_vector: relax the requirements on the move-construction fast path.

Don't require a trivial move constructor and trivial destructor. This excludes
types that have declared themselves trivially relocatable by another means, like
std::unique_ptr. Instead use "is trivially relocatable" directly, which includes
all previous types as well as those that have opted in.
PiperOrigin-RevId: 523557136
Change-Id: Icea2dbb8f36f99623308155f2e5b1edd8e5bd36b
parent 29273402
...@@ -217,20 +217,12 @@ class InlinedVector { ...@@ -217,20 +217,12 @@ class InlinedVector {
absl::allocator_is_nothrow<allocator_type>::value || absl::allocator_is_nothrow<allocator_type>::value ||
std::is_nothrow_move_constructible<value_type>::value) std::is_nothrow_move_constructible<value_type>::value)
: storage_(other.storage_.GetAllocator()) { : storage_(other.storage_.GetAllocator()) {
// Fast path: if the value type can be trivally move constructed and // Fast path: if the value type can be trivially relocated (i.e. moved from
// destroyed, and we know the allocator doesn't do anything fancy, then it's // and destroyed), and we know the allocator doesn't do anything fancy, then
// safe for us to simply adopt the contents of the storage for `other` and // it's safe for us to simply adopt the contents of the storage for `other`
// remove its own reference to them. It's as if we had individually // and remove its own reference to them. It's as if we had individually
// move-constructed each value and then destroyed the original. // move-constructed each value and then destroyed the original.
// if (absl::is_trivially_relocatable<value_type>::value &&
// TODO(b/274984172): a move construction followed by destroying the source
// is a "relocation" in the language of P1144R4. So actually the minimum
// condition we need here (in addition to the allocator) is "trivially
// relocatable". Relaxing this would allow using memcpy with types like
// std::unique_ptr that opt in to declaring themselves trivially relocatable
// despite not being trivially move-constructible and/oror destructible.
if (absl::is_trivially_move_constructible<value_type>::value &&
absl::is_trivially_destructible<value_type>::value &&
std::is_same<A, std::allocator<value_type>>::value) { std::is_same<A, std::allocator<value_type>>::value) {
storage_.MemcpyFrom(other.storage_); storage_.MemcpyFrom(other.storage_);
other.storage_.SetInlinedSize(0); other.storage_.SetInlinedSize(0);
...@@ -271,20 +263,12 @@ class InlinedVector { ...@@ -271,20 +263,12 @@ class InlinedVector {
const allocator_type& const allocator_type&
allocator) noexcept(absl::allocator_is_nothrow<allocator_type>::value) allocator) noexcept(absl::allocator_is_nothrow<allocator_type>::value)
: storage_(allocator) { : storage_(allocator) {
// Fast path: if the value type can be trivally move constructed and // Fast path: if the value type can be trivially relocated (i.e. moved from
// destroyed and we know the allocator doesn't do anything fancy, then it's // and destroyed), and we know the allocator doesn't do anything fancy, then
// safe for us to simply adopt the contents of the storage for `other` and // it's safe for us to simply adopt the contents of the storage for `other`
// remove its own reference to them. It's as if we had individually // and remove its own reference to them. It's as if we had individually
// move-constructed each value and then destroyed the original. // move-constructed each value and then destroyed the original.
// if (absl::is_trivially_relocatable<value_type>::value &&
// TODO(b/274984172): a move construction followed by destroying the source
// is a "relocation" in the language of P1144R4. So actually the minimum
// condition we need here (in addition to the allocator) is "trivially
// relocatable". Relaxing this would allow using memcpy with types like
// std::unique_ptr that opt in to declaring themselves trivially relocatable
// despite not being trivially move-constructible and/oror destructible.
if (absl::is_trivially_move_constructible<value_type>::value &&
absl::is_trivially_destructible<value_type>::value &&
std::is_same<A, std::allocator<value_type>>::value) { std::is_same<A, std::allocator<value_type>>::value) {
storage_.MemcpyFrom(other.storage_); storage_.MemcpyFrom(other.storage_);
other.storage_.SetInlinedSize(0); other.storage_.SetInlinedSize(0);
......
...@@ -301,7 +301,7 @@ class Storage { ...@@ -301,7 +301,7 @@ class Storage {
struct ElementwiseConstructPolicy {}; struct ElementwiseConstructPolicy {};
using MoveAssignmentPolicy = absl::conditional_t< using MoveAssignmentPolicy = absl::conditional_t<
// Fast path: if the value type can be trivally move assigned and // Fast path: if the value type can be trivially move assigned and
// destroyed, and we know the allocator doesn't do anything fancy, then // destroyed, and we know the allocator doesn't do anything fancy, then
// it's safe for us to simply adopt the contents of the storage for // it's safe for us to simply adopt the contents of the storage for
// `other` and remove its own reference to them. It's as if we had // `other` and remove its own reference to them. It's as if we had
...@@ -332,7 +332,7 @@ class Storage { ...@@ -332,7 +332,7 @@ class Storage {
// The policy to be used specifically when swapping inlined elements. // The policy to be used specifically when swapping inlined elements.
using SwapInlinedElementsPolicy = absl::conditional_t< using SwapInlinedElementsPolicy = absl::conditional_t<
// Fast path: if the value type can be trivally move constructed/assigned // Fast path: if the value type can be trivially move constructed/assigned
// and destroyed, and we know the allocator doesn't do anything fancy, // and destroyed, and we know the allocator doesn't do anything fancy,
// then it's safe for us to simply swap the bytes in the inline storage. // then it's safe for us to simply swap the bytes in the inline storage.
// It's as if we had move-constructed a temporary vector, move-assigned // It's as if we had move-constructed a temporary vector, move-assigned
...@@ -505,8 +505,10 @@ class Storage { ...@@ -505,8 +505,10 @@ class Storage {
// we know the allocator doesn't do anything fancy, and one of the following // we know the allocator doesn't do anything fancy, and one of the following
// holds: // holds:
// //
// * It's possible to trivially move construct/assign the elements and // * The elements are trivially relocatable.
// then destroy the source. //
// * It's possible to trivially assign the elements and then destroy the
// source.
// //
// * It's possible to trivially copy construct/assign the elements. // * It's possible to trivially copy construct/assign the elements.
// //
...@@ -517,10 +519,11 @@ class Storage { ...@@ -517,10 +519,11 @@ class Storage {
(std::is_same<A, std::allocator<V>>::value && (std::is_same<A, std::allocator<V>>::value &&
( (
// First case above // First case above
((absl::is_trivially_move_constructible<V>::value || absl::is_trivially_relocatable<V>::value ||
absl::is_trivially_move_assignable<V>::value) &&
absl::is_trivially_destructible<V>::value) ||
// Second case above // Second case above
(absl::is_trivially_move_assignable<V>::value &&
absl::is_trivially_destructible<V>::value) ||
// Third case above
(absl::is_trivially_copy_constructible<V>::value || (absl::is_trivially_copy_constructible<V>::value ||
absl::is_trivially_copy_assignable<V>::value)))); absl::is_trivially_copy_assignable<V>::value))));
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment