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