Add move assignment operator for Json::Value class and overload append member function for RValue references

[ghost]

TODO: add move assignment for CZString class also.

One problem is that I had to change the copy assignment from pass by value to pass by const reference because json_reader was giving errors of ambiguous overloads.
Though to make all tests pass I had to do a const_cast inside the assignment operator since I couldn't do a copy but rather a swap.

I don't like that and would like to fix this. But why is the swap necessary ? shouldn't it be a copy ?

[ghost]

should fix all things for #621

[ghost]

Well TODO completed too. Added move assignment for CZString also.

[cdunn2001]

Thanks. I'll bump the minor version before the next release.

[cfyzium]

I don't like that and would like to fix this. But why is the swap necessary ? shouldn't it be a copy ?

I believe it should be a copy. Moving/swapping objects on assignment is counterintuitive, especially so since argument of assignment operator is declared to be a const reference. It also breaks the code that relies on value semantics, e. g. working with containers. AFAIK it was one of the main reasons to remove auto_ptr.

[cdunn2001]

@cfyzium, I'm mainly interested with maintaining backward-compatibility with this library. If we've altered behavior, please help us to fix it.

[cfyzium]

@cdunn2001 yes, it clearly changes behavior. I've made pull request #640 to fix that.

[evandrocoan]

Accordingly to https://docs.microsoft.com/en-us/cpp/cpp/move-constructors-and-move-assignment-operators-cpp?view=vs-2019

1. You should zero the original objects after moving the value.
2. Check if you are not moving *this into *this.

   // Copy the data pointer and its length from the
   // source object.
   _data = other._data;
   _length = other._length;

   // Release the data pointer from the source object so that
   // the destructor does not free the memory multiple times.
   other._data = nullptr;
   other._length = 0;

...
   if (this != &other)

[BillyDonahue]

Thanks for the link. I am a little skeptical of some sloppiness in the article, but it's fine for beginners I guess.

You don't have to zero everything. Only do enough work to make ~T and T::operator= valid and enforce your postconditions regarding the moved-from state of your class, which is up to you. Moved-from object's destructor and reassignment need to be correct, and maybe nothing else.

You don't have to do check against self-move and do nothing. You only have to anticipate that self-move-assign is possible and prove that it leaves the object in a safe moved-from state. Adding these checks without thinking about it can slow down the operation, just to support an outlier case.

All that said! CZString is not safe against self-move-assign. It will leak its allocation.
That's a for-real bug.

Value::CZString& Value::CZString::operator=(CZString&& other) {
  cstr_ = other.cstr_;
  index_ = other.index_;
  other.cstr_ = nullptr;  // cstr_ is leaked if this is self-move-assignment
  return *this;
}