Re: move constructors likely a requirement

[John W. Eaton]

On 8/21/19 11:49 PM, Rik wrote:
> On 08/21/2019 01:23 PM, John W. Eaton wrote:

> +  octave_value_list& operator = (octave_value_list&& obj)
> +  {
> +    if (this != &obj)
> +      {
> +        m_data = std::move (obj.m_data);
> +        m_names = std::move (obj.m_names);
> +      }
> +
> +    return *this;
> +  }
>
> Wouldn't you need a call such as "m_data.clear ();" to get rid of any
> existing data before assigning to it?

From the explanations and examples I see, I understand "m_data = 
std::move (obj.m_data);" to be the correct way to invoke the move 
assignment operator for the data members.  So those operators should 
take care of the move assignment semantics for these data member objects.

However, I have the move assignment operators that I wrote for reference 
counted classes implemented incorrectly.  To preserve copy-on-write 
semantics, I think that instead of the following (simplified from 
Octave's Array class by omitting slice and dimension data members):

  Array<T>& operator = (Array<T>&& a)
  {
    if (this != &a)
      {
        rep = a.rep;
        a.rep = nullptr;
      }

    return *this;
  }

it should be

  Array<T>& operator = (Array<T>&& a)
  {
    if (this != &a)
      {
        if (--rep->count == 0)
          delete rep;

        rep = a.rep;
        a.rep = nullptr;
      }

    return *this;
  }

This way, we correctly update the reference count and possibly delete 
the object associated with "this" before pointing to the new rep object. 
 We still eliminate one atomic increment on a reference count.  But my 
earlier version will result in incorrect reference counts after move 
assignments.  I'll try to look at fixing that and post another version 
of the patch tomorrow.

> > Instead of looking at whether the destructor is called, maybe it would be
> > better to trace how many times atomic variables are accessed and modified?
>
> I think that is what needs to be monitored.

It should be fairly easy to count all operations on the atomic variables 
that are used in the refcount class.  Are there any others that are not 
wrapped in that class?

> This article on Stack Overflow discusses how the Rule of 3 has now become
> more like the Rule of 5
> (https://stackoverflow.com/questions/4782757/rule-of-three-becomes-rule-of-five-with-c11?noredirect=1).
> If we have classes which are complex enough to require one or more
> non-default constructors then we should (as of C++11) be providing the
> other constructors or indicating to the compiler what we wish to have happen.

Yeah.  More powerful, more complicated.

> Interestingly, I think we could use '= delete' to disable move constructors
> and then use the compiler to find locations in the code base that would use
> a move constructor.  At that point could decide whether to write a move
> constructor for the particular class, or re-structure the API for the
> function such that it is passed a reference to a class object to work on.

OK.

jwe