Skip to content

Commit

Permalink
Use move semantic instead of copy in vector::insert() and vector::set…
Browse files Browse the repository at this point in the history 
…_capacity()

* TestVector: add tests to check move in vector::insert() and vector::set_capacity()
* TestVector: add #if EASTL_MOVE_SEMANTICS_ENABLED/endif for test which use move semantic
  • Loading branch information
@Wawha
Wawha committed Nov 9, 2016
1 parent 256c712 commit 59c777b
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 6 deletions.
24 changes: 18 additions & 6 deletions include/EASTL/vector.h
View file
Expand Up @@ -890,7 +890,19 @@ namespace eastl
else if(n < (size_type)(mpEnd - mpBegin))
resize(n);

#if EASTL_MOVE_SEMANTICS_ENABLED
this_type temp;
size_type nNewSize = size();
pointer const pNewData = DoAllocate(nNewSize);

pointer pNewEnd = eastl::uninitialized_move_ptr(mpBegin, mpEnd, pNewData);

temp.mpBegin = pNewData;
temp.mpEnd = pNewEnd;
temp.mpCapacity = pNewEnd;
#else
this_type temp(*this); // This is the simplest way to accomplish this,
#endif
swap(temp); // and it is as efficient as any other.
}
else // Else new capacity > size.
Expand Down Expand Up @@ -1628,16 +1640,16 @@ namespace eastl

if(n < nExtra) // If the inserted values are entirely within initialized memory (i.e. are before mpEnd)...
{
eastl::uninitialized_copy_ptr(mpEnd - n, mpEnd, mpEnd);
eastl::copy_backward(destPosition, mpEnd - n, mpEnd); // We need copy_backward because of potential overlap issues.
eastl::uninitialized_move_ptr(mpEnd - n, mpEnd, mpEnd);
eastl::move_backward(destPosition, mpEnd - n, mpEnd); // We need move_backward because of potential overlap issues.
eastl::copy(first, last, destPosition);
}
else
{
BidirectionalIterator iTemp = first;
eastl::advance(iTemp, nExtra);
eastl::uninitialized_copy_ptr(iTemp, last, mpEnd);
eastl::uninitialized_copy_ptr(destPosition, mpEnd, mpEnd + n - nExtra);
eastl::uninitialized_move_ptr(destPosition, mpEnd, mpEnd + n - nExtra);
eastl::copy_backward(first, iTemp, destPosition + nExtra);
}

Expand Down Expand Up @@ -1702,14 +1714,14 @@ namespace eastl

if(n < nExtra)
{
eastl::uninitialized_copy_ptr(mpEnd - n, mpEnd, mpEnd);
eastl::copy_backward(destPosition, mpEnd - n, mpEnd); // We need copy_backward because of potential overlap issues.
eastl::uninitialized_move_ptr(mpEnd - n, mpEnd, mpEnd);
eastl::move_backward(destPosition, mpEnd - n, mpEnd); // We need move_backward because of potential overlap issues.
eastl::fill(destPosition, destPosition + n, temp);
}
else
{
eastl::uninitialized_fill_n_ptr(mpEnd, n - nExtra, temp);
eastl::uninitialized_copy_ptr(destPosition, mpEnd, mpEnd + n - nExtra);
eastl::uninitialized_move_ptr(destPosition, mpEnd, mpEnd + n - nExtra);
eastl::fill(destPosition, mpEnd, temp);
}

Expand Down
69 changes: 69 additions & 0 deletions test/source/TestVector.cpp
View file
Expand Up @@ -870,6 +870,55 @@ int TestVector()
EATEST_VERIFY(TestObject::IsClear());
TestObject::Reset();

#if EASTL_MOVE_SEMANTICS_ENABLED
{
// Test insert move objects
eastl::vector<TestObject> toVector1;
toVector1.reserve(20);
for(int idx = 0; idx < 2; ++idx)
toVector1.push_back(TestObject(idx));

eastl::vector<TestObject> toVector2;
for(int idx = 0; idx < 3; ++idx)
toVector2.push_back(TestObject(10 + idx));

// Insert more objects than the existing number using insert with iterator
TestObject::Reset();
toVector1.insert(toVector1.begin(), toVector2.begin(), toVector2.end());
EATEST_VERIFY(VerifySequence(toVector1.begin(), toVector1.end(), int(), "vector.insert", 10, 11, 12, 0, 1, -1));
EATEST_VERIFY(TestObject::sTOMoveCtorCount + TestObject::sTOMoveAssignCount == 2 &&
TestObject::sTOCopyCtorCount + TestObject::sTOCopyAssignCount == 3); // Move 2 existing elements and copy the 3 inserted

eastl::vector<TestObject> toVector3;
toVector3.push_back(TestObject(20));

// Insert less objects than the existing number using insert with iterator
TestObject::Reset();
toVector1.insert(toVector1.begin(), toVector3.begin(), toVector3.end());
EATEST_VERIFY(VerifySequence(toVector1.begin(), toVector1.end(), int(), "vector.insert", 20, 10, 11, 12, 0, 1, -1));
EATEST_VERIFY(TestObject::sTOMoveCtorCount + TestObject::sTOMoveAssignCount == 5 &&
TestObject::sTOCopyCtorCount + TestObject::sTOCopyAssignCount == 1); // Move 5 existing elements and copy the 1 inserted

// Insert more objects than the existing number using insert without iterator
TestObject::Reset();
toVector1.insert(toVector1.begin(), 1, TestObject(17));
EATEST_VERIFY(VerifySequence(toVector1.begin(), toVector1.end(), int(), "vector.insert", 17, 20, 10, 11, 12, 0, 1, -1));
EATEST_VERIFY(TestObject::sTOMoveCtorCount + TestObject::sTOMoveAssignCount == 6 &&
TestObject::sTOCopyCtorCount + TestObject::sTOCopyAssignCount == 2); // Move 6 existing element and copy the 1 inserted +
// the temporary one inside the function

// Insert less objects than the existing number using insert without iterator
TestObject::Reset();
toVector1.insert(toVector1.begin(), 10, TestObject(18));
EATEST_VERIFY(VerifySequence(toVector1.begin(), toVector1.end(), int(), "vector.insert", 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 17, 20, 10, 11, 12, 0, 1, -1));
EATEST_VERIFY(TestObject::sTOMoveCtorCount + TestObject::sTOMoveAssignCount == 7 &&
TestObject::sTOCopyCtorCount + TestObject::sTOCopyAssignCount == 11); // Move 7 existing element and copy the 10 inserted +
// the temporary one inside the function
}
#endif

TestObject::Reset();

{
using namespace eastl;

Expand Down Expand Up @@ -955,8 +1004,20 @@ int TestVector()
allocator.deallocate(pData, n);
EATEST_VERIFY(v.capacity() == 0);
EATEST_VERIFY(VerifySequence(v.begin(), v.end(), int(), "vector.reset", -1));

#if EASTL_MOVE_SEMANTICS_ENABLED
// Test set_capacity make a move when reducing size
vector<TestObject> toArray2(10, TestObject(7));
TestObject::Reset();
toArray2.set_capacity(5);
EATEST_VERIFY(TestObject::sTOMoveCtorCount == 5 &&
TestObject::sTOCopyCtorCount + TestObject::sTOCopyAssignCount == 0); // Move the 5 existing elements, no copy
EATEST_VERIFY(VerifySequence(toArray2.begin(), toArray2.end(), int(), "vector.set_capacity", 7, 7, 7, 7, 7, -1));
#endif
}

TestObject::Reset();

{
using namespace eastl;

Expand Down Expand Up @@ -1306,12 +1367,14 @@ int TestVector()
v2.push_back(StructWithConstRefToInt(j));
}

#if EASTL_MOVE_SEMANTICS_ENABLED
{
// Regression for issue with vector containing non-copyable values reported by user
eastl::vector<testmovable> moveablevec;
testmovable moveable;
moveablevec.insert(moveablevec.end(), eastl::move(moveable));
}
#endif

#if defined(EASTL_TEST_CONCEPT_IMPLS)

Expand Down Expand Up @@ -1352,6 +1415,7 @@ int TestVector()
EATEST_VERIFY(v4.size() == 2 && v4[0].value == v4[1].value && v4[0].value == CopyConstructible::defaultValue);
}

#if EASTL_MOVE_SEMANTICS_ENABLED
{
// vector::reserve() should only require MoveInsertible
eastl::vector<MoveConstructible> v5;
Expand All @@ -1374,6 +1438,7 @@ int TestVector()
eastl::move_iterator<MoveConstructible*>(eastl::end(moveConstructibleArray)));
EATEST_VERIFY(v7.size() == 1 && v7[0].value == MoveConstructible::defaultValue);
}
#endif

{
// vector::swap() should only require Destructible. We also test with DefaultConstructible as it gives us a
Expand All @@ -1394,6 +1459,7 @@ int TestVector()
*/
}

#if EASTL_MOVE_SEMANTICS_ENABLED
{
// vector::resize() should only require MoveInsertable and DefaultInsertable
eastl::vector<MoveAndDefaultConstructible> v8;
Expand All @@ -1411,6 +1477,7 @@ int TestVector()
v1.erase(begin(v1));
EATEST_VERIFY(v1.empty());
}
#endif

#endif // EASTL_TEST_CONCEPT_IMPLS

Expand Down Expand Up @@ -1450,6 +1517,7 @@ int TestVector()
}


#if EASTL_MOVE_SEMANTICS_ENABLED
// unique_ptr tests
{
// Simple move-assignment test to prevent regressions where eastl::vector utilizes operations on T that are not necessary.
Expand Down Expand Up @@ -1489,6 +1557,7 @@ int TestVector()
VERIFY(InstanceAllocator::mMismatchCount == 0);
}
}
#endif

return nErrorCount;
}

0 comments on commit 59c777b

Please sign in to comment.