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

[Howard Hinnant]

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);

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