From d45f7b6b5dd36b4732dff82ab7c8a856a7b36ae0 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Sun, 17 Jun 2012 12:46:13 +0000 Subject: Bring the return value of SmallVector::insert in line with std::vector::insert. It always returns the iterator for the first inserted element, or the passed in iterator if the inserted range was empty. Flesh out the unit test more and fix all the cases it uncovered so far. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@158645 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/ADT/SmallVector.h | 21 +++++++++++---------- unittests/ADT/SmallVectorTest.cpp | 34 ++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/include/llvm/ADT/SmallVector.h b/include/llvm/ADT/SmallVector.h index 02eee62..2d71249 100644 --- a/include/llvm/ADT/SmallVector.h +++ b/include/llvm/ADT/SmallVector.h @@ -540,14 +540,14 @@ public: } iterator insert(iterator I, size_type NumToInsert, const T &Elt) { + // Convert iterator to elt# to avoid invalidating iterator when we reserve() + size_t InsertElt = I - this->begin(); + if (I == this->end()) { // Important special case for empty vector. append(NumToInsert, Elt); - return NumToInsert == 0 ? this->end() : this->end()-1; + return this->begin()+InsertElt; } - // Convert iterator to elt# to avoid invalidating iterator when we reserve() - size_t InsertElt = I - this->begin(); - // Ensure there is enough space. reserve(static_cast(this->size() + NumToInsert)); @@ -588,14 +588,15 @@ public: template iterator insert(iterator I, ItTy From, ItTy To) { + // Convert iterator to elt# to avoid invalidating iterator when we reserve() + size_t InsertElt = I - this->begin(); + if (I == this->end()) { // Important special case for empty vector. append(From, To); - return From == To ? this->end() : this->end()-1; + return this->begin()+InsertElt; } size_t NumToInsert = std::distance(From, To); - // Convert iterator to elt# to avoid invalidating iterator when we reserve() - size_t InsertElt = I - this->begin(); // Ensure there is enough space. reserve(static_cast(this->size() + NumToInsert)); @@ -628,9 +629,9 @@ public: this->uninitialized_copy(I, OldEnd, this->end()-NumOverwritten); // Replace the overwritten part. - for (; NumOverwritten > 0; --NumOverwritten) { - *I = *From; - ++I; ++From; + for (T *J = I; NumOverwritten > 0; --NumOverwritten) { + *J = *From; + ++J; ++From; } // Insert the non-overwritten middle part. diff --git a/unittests/ADT/SmallVectorTest.cpp b/unittests/ADT/SmallVectorTest.cpp index 8f6d2f4..d35e5bc 100644 --- a/unittests/ADT/SmallVectorTest.cpp +++ b/unittests/ADT/SmallVectorTest.cpp @@ -342,7 +342,9 @@ TEST_F(SmallVectorTest, InsertTest) { SCOPED_TRACE("InsertTest"); makeSequence(theVector, 1, 3); - theVector.insert(theVector.begin() + 1, Constructable(77)); + VectorType::iterator I = + theVector.insert(theVector.begin() + 1, Constructable(77)); + EXPECT_EQ(theVector.begin() + 1, I); assertValuesInOrder(theVector, 4u, 1, 77, 2, 3); } @@ -351,24 +353,48 @@ TEST_F(SmallVectorTest, InsertRepeatedTest) { SCOPED_TRACE("InsertRepeatedTest"); makeSequence(theVector, 10, 15); - theVector.insert(theVector.begin() + 1, 2, Constructable(16)); + VectorType::iterator I = + theVector.insert(theVector.begin() + 1, 2, Constructable(16)); + EXPECT_EQ(theVector.begin() + 1, I); assertValuesInOrder(theVector, 8u, 10, 16, 16, 11, 12, 13, 14, 15); + // Insert at end. + I = theVector.insert(theVector.end(), 2, Constructable(16)); + EXPECT_EQ(theVector.begin() + 8, I); + assertValuesInOrder(theVector, 10u, 10, 16, 16, 11, 12, 13, 14, 15, 16, 16); + + // Empty insert. EXPECT_EQ(theVector.end(), theVector.insert(theVector.end(), 0, Constructable(42))); + EXPECT_EQ(theVector.begin() + 1, + theVector.insert(theVector.begin() + 1, 0, Constructable(42))); } // Insert range. TEST_F(SmallVectorTest, InsertRangeTest) { - SCOPED_TRACE("InsertRepeatedTest"); + SCOPED_TRACE("InsertRangeTest"); + + Constructable Arr[3] = + { Constructable(77), Constructable(77), Constructable(77) }; makeSequence(theVector, 1, 3); - theVector.insert(theVector.begin() + 1, 3, Constructable(77)); + VectorType::iterator I = + theVector.insert(theVector.begin() + 1, Arr, Arr+3); + EXPECT_EQ(theVector.begin() + 1, I); assertValuesInOrder(theVector, 6u, 1, 77, 77, 77, 2, 3); + // Insert at end. + I = theVector.insert(theVector.end(), Arr, Arr+3); + EXPECT_EQ(theVector.begin() + 6, I); + assertValuesInOrder(theVector, 9u, 1, 77, 77, 77, 2, 3, 77, 77, 77); + + // Empty insert. EXPECT_EQ(theVector.end(), theVector.insert(theVector.end(), theVector.begin(), theVector.begin())); + EXPECT_EQ(theVector.begin() + 1, theVector.insert(theVector.begin() + 1, + theVector.begin(), + theVector.begin())); } // Comparison tests. -- cgit v1.1