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

[Lang Hames]

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.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150122/ca1b9da9/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: SmallVectorMoveAssignmentFromSmallVectorImpl-2.patch
Type: application/octet-stream
Size: 4325 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150122/ca1b9da9/attachment.obj>