[cfe-dev] [PATCH] Add APValue::swap

[Richard Smith]

The attached patch is the direction I was thinking of. I believe the
changes outside APValue are all redundant with this approach; this is a
pure optimization of the existing interface. The ExtractSubobject change
isn't necessary, but it avoids some potentially-expensive copying. Some
care is required to avoid it creating a cyclic value.

I've not got any performance numbers for this change yet.

On Wed, Mar 7, 2012 at 5:15 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> On Wed, Mar 7, 2012 at 3:05 PM, Daniel Dunbar <daniel at zuster.org> wrote:
>
>> Here is a patch which adds APValue::swap and uses it.
>>
>> I'm not sure I like it, anyone else have an opinion for putting it in or
>> not?
>>
>
> This was actually something I was intending to investigate soon. A few
> thoughts:
>
> I'd prefer to see operator= changed as follows, all the code which says
> 'foo = APValue(...);' left alone:
>
> APValue &operator=(APValue RHS) {
>   swap(RHS);
>   return *this;
> }
>
> Copy elision will do the rest. I'd then prefer to see copy_from killed,
> with the implementation moved to APValue's copy constructor (which should
> then be out-of-line).
>
> Do you have any performance numbers for this?
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120307/8f24e806/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: apvalue-move-semantics.diff
Type: text/x-patch
Size: 5715 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120307/8f24e806/attachment.bin>