From cab673f638be3fdd1b99edbb91820f9fea4ddaa9 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Sun, 8 Jun 2014 19:33:40 +0000 Subject: [PATCH] SmallVector: Improve test coverage for insert with repetition To test cases that involve actual repetition (> 1 elements), at least one element before the insertion point, and some elements of the original range that still fit in that range space after insertion. Actually we need coverage for the inverse case too (where no elements after the insertion point fit into the previously allocated space), but this'll do for now, and I might end up rewriting bits of SmallVector to avoid that special case anyway. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@210436 91177308-0d34-0410-b5e6-96231b3b80d8 --- unittests/ADT/SmallVectorTest.cpp | 40 ++++++++++++++----------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/unittests/ADT/SmallVectorTest.cpp b/unittests/ADT/SmallVectorTest.cpp index 972f39652b1..760d7343868 100644 --- a/unittests/ADT/SmallVectorTest.cpp +++ b/unittests/ADT/SmallVectorTest.cpp @@ -184,7 +184,8 @@ protected: typedef ::testing::Types, SmallVector, SmallVector, - SmallVector + SmallVector, + SmallVector > SmallVectorTestTypes; TYPED_TEST_CASE(SmallVectorTest, SmallVectorTestTypes); @@ -465,50 +466,45 @@ TYPED_TEST(SmallVectorTest, InsertCopy) { TYPED_TEST(SmallVectorTest, InsertRepeatedTest) { SCOPED_TRACE("InsertRepeatedTest"); - // FIXME: This range is too big (it's outside the "small" zone to start with, - // and the buffer allocated is large enough that none of these tests ever - // reallocate) to test all the code paths, specifically none of them test - // reallocation. - this->makeSequence(this->theVector, 10, 15); + this->makeSequence(this->theVector, 1, 4); Constructable::reset(); auto I = this->theVector.insert(this->theVector.begin() + 1, 2, Constructable(16)); - // Once this tests reallocation, there could be more move constructions here. - // Move construct the top two elements into newly allocated space. - EXPECT_EQ(2, Constructable::getNumMoveConstructorCalls()); + // Move construct the top element into newly allocated space, and optionally + // reallocate the whole buffer, move constructing into it. + // FIXME: This is inefficient, we shouldn't move things into newly allocated + // space, then move them up/around, there should only be 2 or 4 move + // constructions here. + EXPECT_TRUE(Constructable::getNumMoveConstructorCalls() == 2 || + Constructable::getNumMoveConstructorCalls() == 6); // Move assign the next two to shift them up and make a gap. - EXPECT_EQ(3, Constructable::getNumMoveAssignmentCalls()); + EXPECT_EQ(1, Constructable::getNumMoveAssignmentCalls()); // Copy construct the two new elements from the parameter. EXPECT_EQ(2, Constructable::getNumCopyAssignmentCalls()); // All without any copy construction. EXPECT_EQ(0, Constructable::getNumCopyConstructorCalls()); EXPECT_EQ(this->theVector.begin() + 1, I); - this->assertValuesInOrder(this->theVector, 8u, - 10, 16, 16, 11, 12, 13, 14, 15); + this->assertValuesInOrder(this->theVector, 6u, 1, 16, 16, 2, 3, 4); } TYPED_TEST(SmallVectorTest, InsertRepeatedAtEndTest) { SCOPED_TRACE("InsertRepeatedTest"); - // FIXME: This range is too big (it's outside the "small" zone to start with, - // and the buffer allocated is large enough that none of these tests ever - // reallocate) to test all the code paths, specifically none of them test - // reallocation. - this->makeSequence(this->theVector, 10, 15); + this->makeSequence(this->theVector, 1, 4); Constructable::reset(); auto I = this->theVector.insert(this->theVector.end(), 2, Constructable(16)); // Just copy construct them into newly allocated space EXPECT_EQ(2, Constructable::getNumCopyConstructorCalls()); - // Move everything across during reallocation. - EXPECT_EQ(0, Constructable::getNumMoveConstructorCalls()); + // Move everything across if reallocation is needed. + EXPECT_TRUE(Constructable::getNumMoveConstructorCalls() == 0 || + Constructable::getNumMoveConstructorCalls() == 4); // Without ever moving or copying anything else. EXPECT_EQ(0, Constructable::getNumCopyAssignmentCalls()); EXPECT_EQ(0, Constructable::getNumMoveAssignmentCalls()); - EXPECT_EQ(this->theVector.begin() + 6, I); - this->assertValuesInOrder(this->theVector, 8u, - 10, 11, 12, 13, 14, 15, 16, 16); + EXPECT_EQ(this->theVector.begin() + 4, I); + this->assertValuesInOrder(this->theVector, 6u, 1, 2, 3, 4, 16, 16); } TYPED_TEST(SmallVectorTest, InsertRepeatedEmptyTest) {