[llvm] r226899 - [ADT] Add move operations to SmallVector<T, N> from SmallVectorImpl<T>.

[Lang Hames]

Hi Dave,

I didn't delve into the code to check,  I just cribbed from the existing
implementations. My inclination is that that check should be pushed down
into SmallVectorImpl's methods though.

- Lang.

On Fri, Jan 23, 2015 at 12:02 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Thu, Jan 22, 2015 at 10:25 PM, Lang Hames <lhames at gmail.com> wrote:
>
>> Author: lhames
>> Date: Fri Jan 23 00:25:17 2015
>> New Revision: 226899
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=226899&view=rev
>> Log:
>> [ADT] Add move operations to SmallVector<T,N> from SmallVectorImpl<T>.
>>
>> This makes it possible to move between SmallVectors of different sizes.
>>
>> Thanks to Dave Blaikie and Duncan Smith for patch feedback.
>>
>> Modified:
>>     llvm/trunk/include/llvm/ADT/SmallVector.h
>>     llvm/trunk/unittests/ADT/SmallVectorTest.cpp
>>
>> Modified: llvm/trunk/include/llvm/ADT/SmallVector.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallVector.h?rev=226899&r1=226898&r2=226899&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/ADT/SmallVector.h (original)
>> +++ llvm/trunk/include/llvm/ADT/SmallVector.h Fri Jan 23 00:25:17 2015
>> @@ -921,6 +921,17 @@ public:
>>      SmallVectorImpl<T>::operator=(::std::move(RHS));
>>      return *this;
>>    }
>> +
>> +  SmallVector(SmallVectorImpl<T> &&RHS) : SmallVectorImpl<T>(N) {
>> +    if (!RHS.empty())
>>
>
> Is this ^ check necessary/useful for some reason? (why in the ctor, but
> not in the assignment operator?)
>
>
>> +      SmallVectorImpl<T>::operator=(::std::move(RHS));
>> +  }
>> +
>> +  const SmallVector &operator=(SmallVectorImpl<T> &&RHS) {
>> +    SmallVectorImpl<T>::operator=(::std::move(RHS));
>> +    return *this;
>> +  }
>> +
>>  };
>>
>>  template<typename T, unsigned N>
>>
>> Modified: llvm/trunk/unittests/ADT/SmallVectorTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/SmallVectorTest.cpp?rev=226899&r1=226898&r2=226899&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/ADT/SmallVectorTest.cpp (original)
>> +++ llvm/trunk/unittests/ADT/SmallVectorTest.cpp Fri Jan 23 00:25:17 2015
>> @@ -153,17 +153,14 @@ LLVM_ATTRIBUTE_USED void CompileTest() {
>>    V.resize(42);
>>  }
>>
>> -// Test fixture class
>> -template <typename VectorT>
>> -class SmallVectorTest : public testing::Test {
>> +class SmallVectorTestBase : public testing::Test {
>>  protected:
>> -  VectorT theVector;
>> -  VectorT otherVector;
>>
>>    void SetUp() {
>>      Constructable::reset();
>>    }
>>
>> +  template <typename VectorT>
>>    void assertEmpty(VectorT & v) {
>>      // Size tests
>>      EXPECT_EQ(0u, v.size());
>> @@ -173,7 +170,8 @@ protected:
>>      EXPECT_TRUE(v.begin() == v.end());
>>    }
>>
>> -  // Assert that theVector contains the specified values, in order.
>> +  // Assert that v contains the specified values, in order.
>> +  template <typename VectorT>
>>    void assertValuesInOrder(VectorT & v, size_t size, ...) {
>>      EXPECT_EQ(size, v.size());
>>
>> @@ -188,6 +186,7 @@ protected:
>>    }
>>
>>    // Generate a sequence of values to initialize the vector.
>> +  template <typename VectorT>
>>    void makeSequence(VectorT & v, int start, int end) {
>>      for (int i = start; i <= end; ++i) {
>>        v.push_back(Constructable(i));
>> @@ -195,6 +194,15 @@ protected:
>>    }
>>  };
>>
>> +// Test fixture class
>> +template <typename VectorT>
>> +class SmallVectorTest : public SmallVectorTestBase {
>> +protected:
>> +  VectorT theVector;
>> +  VectorT otherVector;
>> +};
>> +
>> +
>>  typedef ::testing::Types<SmallVector<Constructable, 0>,
>>                           SmallVector<Constructable, 1>,
>>                           SmallVector<Constructable, 2>,
>> @@ -664,6 +672,67 @@ TYPED_TEST(SmallVectorTest, IteratorTest
>>    this->theVector.insert(this->theVector.end(), L.begin(), L.end());
>>  }
>>
>> +template <typename InvalidType> class DualSmallVectorsTest;
>> +
>> +template <typename VectorT1, typename VectorT2>
>> +class DualSmallVectorsTest<std::pair<VectorT1, VectorT2>> : public
>> SmallVectorTestBase {
>> +protected:
>> +  VectorT1 theVector;
>> +  VectorT2 otherVector;
>> +
>> +  template <typename T, unsigned N>
>> +  static unsigned NumBuiltinElts(const SmallVector<T, N>&) { return N; }
>> +};
>> +
>> +typedef ::testing::Types<
>> +    // Small mode -> Small mode.
>> +    std::pair<SmallVector<Constructable, 4>, SmallVector<Constructable,
>> 4>>,
>> +    // Small mode -> Big mode.
>> +    std::pair<SmallVector<Constructable, 4>, SmallVector<Constructable,
>> 2>>,
>> +    // Big mode -> Small mode.
>> +    std::pair<SmallVector<Constructable, 2>, SmallVector<Constructable,
>> 4>>,
>> +    // Big mode -> Big mode.
>> +    std::pair<SmallVector<Constructable, 2>, SmallVector<Constructable,
>> 2>>
>> +  > DualSmallVectorTestTypes;
>> +
>> +TYPED_TEST_CASE(DualSmallVectorsTest, DualSmallVectorTestTypes);
>> +
>> +TYPED_TEST(DualSmallVectorsTest, MoveAssignment) {
>> +  SCOPED_TRACE("MoveAssignTest-DualVectorTypes");
>> +
>> +  // Set up our vector with four elements.
>> +  for (unsigned I = 0; I < 4; ++I)
>> +    this->otherVector.push_back(Constructable(I));
>> +
>> +  const Constructable *OrigDataPtr = this->otherVector.data();
>> +
>> +  // Move-assign from the other vector.
>> +  this->theVector =
>> +
>> std::move(static_cast<SmallVectorImpl<Constructable>&>(this->otherVector));
>> +
>> +  // Make sure we have the right result.
>> +  this->assertValuesInOrder(this->theVector, 4u, 0, 1, 2, 3);
>> +
>> +  // Make sure the # of constructor/destructor calls line up. There
>> +  // are two live objects after clearing the other vector.
>> +  this->otherVector.clear();
>> +  EXPECT_EQ(Constructable::getNumConstructorCalls()-4,
>> +            Constructable::getNumDestructorCalls());
>> +
>> +  // If the source vector (otherVector) was in small-mode, assert that
>> we just
>> +  // moved the data pointer over.
>> +  EXPECT_TRUE(this->NumBuiltinElts(this->otherVector) == 4 ||
>> +              this->theVector.data() == OrigDataPtr);
>> +
>> +  // There shouldn't be any live objects any more.
>> +  this->theVector.clear();
>> +  EXPECT_EQ(Constructable::getNumConstructorCalls(),
>> +            Constructable::getNumDestructorCalls());
>> +
>> +  // We shouldn't have copied anything in this whole process.
>> +  EXPECT_EQ(Constructable::getNumCopyConstructorCalls(), 0);
>> +}
>> +
>>  struct notassignable {
>>    int &x;
>>    notassignable(int &x) : x(x) {}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150123/24946b5f/attachment.html>