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

[Daniel Dunbar]

Thanks Richard,

I'm not really a C++ guy, maybe I wasn't expecting copy elision to do
what I wanted. I'll try and look into your patch today and see why my
numbers look like for it.

 - Daniel

On Wed, Mar 7, 2012 at 9:17 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 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?
>
>