0
struct F
{
private:
    int* data;

public:
    F( int n ) 
    { 
        data = new int; 
        *data = n; 
    }

    F( int* p ) 
    { 
        data = p; 
    }

    F& operator=( const F& f ) 
    { 
        *data = *(f.get_data()); 
        return *this; 
    }

    F& operator=( F&& f ) 
    {
        delete data;
        data = f.data;
        f.data = nullptr;
        return *this;
    }

    F& operator=( int n ) { *data = n; return *this; }

    F operator()() 
    {
        F cpy_f( data );
        return std::move( cpy_f );
    }

    int* get_data() const { return data; }
};

int main()
{
    F f( 12 );
    F g( 14 );
    f() = g();
    cout << *(f.get_data()) << endl;
}

In this example, f() and g() respectively return a temporary object, so f()=g() results in an expression of temporary object equal to temporary object. I would have expected that the answer is 14 if the value is correctly copied. However, it is NOT calling the copy assignment, BUT calling the move assignment! As a result, the answer is not 14.

This makes me really confused. Although the returning objects from f() and g() are temporary, they are sharing some information with some other objects. It implies the temporary objects might be doing some works shortly for the shared information. So I am supposed that semantically calling copy assignment would be the correct behaviors.

ps. My compiler is g++4.7 20110430

ildjarn
  • 59,718
  • 8
  • 115
  • 201

2 Answers2

4

Your operator() returns a value, rather than a reference or pointer. Therefore, it returns a temporary. Temporaries implicitly bind to && preferentially (they are the only types to do so). Therefore, if there is a move assignment operator for them to use, they will prefer to use it.

Your problem is that you stopped doing reasonable things when you did this in your operator() function:

F cpy_f( data );

The constructor of F that takes a pointer uses the pointer value itself, effectively adopting the pointer. At that moment, you now have two F instances that point to the same data.

If that's something you want to be reasonable, then you cannot have your move assignment operator delete the pointer. You need to reconcile the fact that you can have two F instances that point to the same thing. The two need to share ownership of the pointer. So... how do you plan to do that?

I would suggest getting rid of this constructor altogether. It will only create more problems for you. I would also suggest using a std::unique_ptr<int> data; instead of a naked pointer. That way, you don't have to write a move constructor/assignment operator (though you do need a copy constructor/assignment operator).

Nicol Bolas
  • 378,677
  • 53
  • 635
  • 829
0

You are creating an instance which has a pointer pointing the same location. The instance created with F::operator() does not share the pointer itself.

f()=g() is equivalent to f().operator=(g()) and g() creates a temporary instance, so it is correct to call move assignment.

The problem is that f has a dangling pointer after the execution of f()=g(). f() creates an temporary instance, it deletes the pointer in the move assignment operator, but f itself still has a pointer which points the deleted location.

I'm not sure what you're doing with those complicated code but you'd better rewrite the code with std::shared_ptr or something.

p.s. You don't need std::move in returning an instance. Your compiler has RVO features if you didn't turn these off.

Inbae Jeong
  • 3,823
  • 23
  • 36