Re: move constructors likely a requirement

[John W. Eaton]

On 8/20/19 6:59 PM, Rik wrote:

> However, there is no
> decrease in the number of times (9) the ~octave_value_destructor is called
> because I couldn't find a clean way to "zero out" the rvalue object that is
> being moved.

>      std::swap (m_data, obj.m_data);
>      std::swap (m_names, obj.m_names);

I think these should be

  m_data = std::move (obj.m_data);
  m_names = std::move (obj.m_names);

and not calls to std::swap.

But in any case, whether a copy or move happens, my understanding is 
that the destructor for the moved object will still be called, but it 
doesn't have to do anything.

For example, for a reference-counted class like octave_value, you would 
do something like

  octave_value (octave_value&& other)
    : rep (other.rep)
  {
    other.rep = nullptr;
  }

to transfer the ownership of REP from the OTHER to *THIS instead of 
manipulating the reference count.  Then the destructor should also be 
changed so that it will skip decrementing the reference count if REP is 
nullptr:

  ~octave_value (void)
  {
    if (rep && --rep->count == 0)
      delete rep;
  }

Currently, we just have "if (--rep->count == 0)" but that will fail if 
we introduce a move constructor.

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 have been experimenting with the attached patches:

  diffs-1.txt: Change the string_vector class so it contains an 
Array<std::string> object instead of deriving from Array<std::string> 
(next step is to use some other container instead of the heavyweight 
Array<T>).

  diffs-2.txt: Introduce move constructors and assignment operators for 
the Array, string_vector and octave_value_list classses.

  diffs-3.txt: Improve the tree_evaluator::convert_to_const_vecctor 
function so that it doesn't create as many octave_value_list objects.

I think it is worth attempting to add move constructors and assignment 
operators to any classes we define that need to be copied.  As we do for 
copy constructors and assignment operators, it would also make sense to 
use the "= default" specification where possible so that the compiler 
can generate these functions, or to use "= delete" to explicitly state 
that they should not be defined.

jwe

diffs-1.txt
Description: Text document

diffs-2.txt
Description: Text document

diffs-3.txt
Description: Text document