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

[Daniel Dunbar]

On Wed, Mar 7, 2012 at 5:22 PM, Howard Hinnant <hhinnant at apple.com> wrote:
> On Mar 7, 2012, at 6:05 PM, Daniel Dunbar 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?
>
> I note that the patch also introduces:
>
>  void copy_from(const APValue &RHS);
>
> and changes all:
>
> -    Result = Frame->Arguments[PVD->getFunctionScopeIndex()];
>
> to:
>
> +    Result.copy_from(Frame->Arguments[PVD->getFunctionScopeIndex()]);
>
> My observation is that in C++ the syntax:
>
>   x = y;
>
> is universally understood.  Whereas the syntax:
>
>   x.copy_from(y);

I wanted to make the copy assignment operator private to encourage
callers to see if they could use swap instead.

My motivation here seems confused though, per subsequent discussion.

 - Daniel

>
> is obviously readable, and yet it does immediately raise questions for the reader of the code:  What does copy_from do that copy assignment doesn't?  Additionally if APValue is ever going to be used in generic code, that generic code is probably going to use the syntax:
>
>   x = y;
>
> I guess my message is:  if there is common syntax for doing something in C++, prefer that syntax over inventing new syntax.
>
> For swap, the member swap you've added looks good.  Most types also add a namespace scope function:
>
>    inline void swap(APValue& x, APValue& y) {x.swap(y);}
>
> to aid in generic code which will do:
>
>   using std::swap;
>   swap(x, y);
>
> Disclaimer:  These comments are made without the benefit of knowledge of APValue.
>
> Howard
>