[PATCH][ADT] Add move-construction and move-assignment from SmallVectorImpl<T> to SmallVector<T,N>.

[Duncan P. N. Exon Smith]

> On 2015-Jan-22, at 17:11, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> This tests that `Constructable` was moved, but it still doesn't test
> whether the heap allocation was moved (as opposed to individual
> elements being moved to a new allocation).  You need something like
> this to test that:
> 
>     SmallVector<int, 4> V1(8), V2;
>     auto First = V1.begin();
>     V2 = std::move(V1);
>     EXPECT_EQ(First, V2.begin());

Or maybe you could check that no constructors had been called at all
in the move?

> 
> Of course, you only want to check this when you expect the whole
> allocation to move over: when the source vector is in big mode.
> 
>> On 2015-Jan-22, at 16:43, Lang Hames <lhames at gmail.com> wrote:
>> 
>> Hi Guys,
>> 
>> Thanks for catching that - I've attached an updated patch. I'm still just testing correctness, rather than implementation specifics, but it does at least verify that it's move-constructing rather than copy-constructing now.
>> 
>> Cheers,
>> Lang.
>> 
>> 
>> On Thu, Jan 22, 2015 at 4:19 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> 
>> 
>> On Thu, Jan 22, 2015 at 4:17 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>>> On 2015-Jan-22, at 16:04, David Blaikie <dblaikie at gmail.com> wrote:
>>> 
>>> This looks like it's only adding copy, not move - did I miss something?
>>> 
>>> On Thu, Jan 22, 2015 at 3:54 PM, Lang Hames <lhames at gmail.com> wrote:
>>> Hi All,
>>> 
>>> The attached patch adds move-construction and move-assignment from SmallVectorImpl<T> to SmallVector<T, N>. It enables us to move between SmallVectors of different types, which will allow me to fix one of the most commented-upon warts from the new JIT APIs.
>>> 
>>> Unit-test included.
>>> 
>>> Cheers,
>>> Lang.
>>> 
>> 
>> Yeah, the tests don't actually confirm that the thing was moved.  In
>> particular, you want to check that whenever the source is in big-mode,
>> the destination points at the same memory that the source used to (well,
>> assuming that's the behaviour you want, which the naive implementation
>> will give you IIUC).
>> 
>> There's also a "big mode -> small mode" funny state thing that'll happen
>> with the naive implementation.  I'm not sure it's *wrong*, but...
>> 
>> Given:
>> 
>>    SmallVector<int, 1> InBigMode(2);
>>    SmallVector<int, 2> InBigModeToo = std::move(InBigMode);
>> 
>> I think this will pass:
>> 
>>    assert(!InBigModeToo.isSmall());
>> 
>> This a new state -- a SmallVector can be in "big" mode with a heap
>> allocation, even though its capacity would fit in its "small" mode
>> stack allocation.
>> 
>> While weird, I don't really have a problem with it (but maybe others
>> do?).  Might be worth testing a few operations in this untested state
>> though.
>> 
>> It's probably valuable that this is the outcome - it'd be wasteful to throw away the dynamic allocation (going down to small mode) - better to keep it even though it's < small_size. So I'd be in favor of this. 
>> 
>> 
>> <SmallVectorMoveAssignmentFromSmallVectorImpl-2.patch>
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits