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

[Howard Hinnant]

On Mar 8, 2012, at 6:05 PM, Benjamin Kramer wrote:

> 
> On 08.03.2012, at 23:54, David Blaikie wrote:
> 
>> On Thu, Mar 8, 2012 at 2:46 PM, Chris Lattner <clattner at apple.com> wrote:
>>> 
>>> On Mar 8, 2012, at 10:45 AM, Howard Hinnant wrote:
>>> 
>>>> On Mar 8, 2012, at 1:39 PM, Daniel Dunbar wrote:
>>>> 
>>>>> 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.
>>>> 
>>>> I think you're just ahead of your time.  Sounds like you're trying to encourage C++11 move assignment. :-) (which I think would be a very good idea, but requires commitment to using C++11)
>>> 
>>> I think that it would be completely reasonable (maybe even, "insanely great"?) to put move constructors and assignment operators into llvm & clang if they help performance and are *optional* and ifdef'd on compiler support.
>>> 
>>> That would just mean that people with c++'11 compilers can get better build times, but that we remain compatible with older compilers.
>> 
>> Usually we have to/have worked around the cases (you know, the usual
>> "don't return big things by value") - the most common things that
>> would benefit would be stuff that this already happens for (eg:
>> std::string). Anyone know of good candidates that we wouldn't
>> reasonably fix for the 98 case by removing the copies?
> 
> We have some cases of SmallVectors and DenseMaps of objects with non-trivial copy ctors. It would spare us a few deep copies if the container would move them when it reallocates.
> 
> Not sure if it's worth introducing a ton of #ifdef'd code. Sounds like a maintenance nightmare to me.

The #ifdef'ing can be a pain.

I do not want to influence this decision one way or another.  But perhaps it would be helpful to relate my experience/practice in libc++ on how I've handled this issue:

For adding move semantics to a class, I just #ifdef around the added members:

class SmallVector
{
public:
     // The C++03 constructors & assignment
     // ...
#ifdef RVALUE_REFS
     SmallVector(SmallVector&&);
     SmallVector& operator=(SmallVector&&);
#endif
    // ...
};

In /using/ move semantics, I go ahead and define std::move in both C++03 and C++11 modes, and so code always looks like:

x = std::move(y);

For llvm you could do something like:

#ifndef RVALUE_REFS

namespace std
{

template <class T>
inline
const T&
move(const T& t)
{
    return t;
}

}

#endif

So in C++03:

    x = std::move(y);

is equivalent to:

    x = y;

but in C++11 it becomes a move assignment, with no syntax change at the use site.  And that is key to keeping this from becoming a total nightmare.  #ifdef's are limited to class implementation, and do not leak out to class clients.  Class clients can immediately move to C++11 syntax, whether or not they actually get the C++11 optimization.

For factory functions, there is absolutely nothing to do:

SmallVector
make_SmallVector(int data)
{
    SmallVector v(data);
    // ....
    return v;
}

The above syntax works for both C++03 and C++11.  Typically in both, RVO kicks in.  In the relatively rare cases where RVO does not kick in, C++03 will copy the SmallVector out, and C++11 will move it out.  And on the client side, it is also identical:

    SmallVector v = make_SmallVector(data);

or

    v = make_SmallVector(data);

This may not be the right time to migrate llvm to C++11.  But whenever that time comes, the above is a reasonable approach for making that migration.

Howard