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

[David Blaikie]

On Fri, Jan 23, 2015 at 10:57 AM, Lang Hames <lhames at gmail.com> wrote:

> 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.
>

*frysquint* r40983.

Not clear whether the win is in avoiding the function call entirely, or
just in having the short-cut before the rest of the assignment
implementation.

(+Chris for history. +Chandler for inlining)


>
> - 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/380f5724/attachment.html>