Implement move semantics for xml::attributes

This allows to move them cheaply (or at least cheaper than by copying
everything).

Switch to representing null pimpl_ pointer for representing the "empty"
state, as is more compatible with move semantics, but this required
adding checks for the pointer being null in a number of places.

Author: vadz

include/xmlwrapp/attributes.h

@@ -70,6 +70,11 @@ struct node_impl;
 
     The iterator classes allow you to access one XML attribute. This is done
     using the xml::attributes::attr class interface.
+
+    Objects of this class are copyable and movable, with the latter being more
+    efficient, but leaving the source object in a special state in which only
+    asignment operator and empty() and size() member functions can be used on
+    it.
  */
 class XMLWRAPP_API attributes
 {
@@ -89,6 +94,15 @@ class XMLWRAPP_API attributes
      */
     attributes(const attributes& other);
 
+    /**
+        Move construct a xml::attributes object.
+
+        @param other The xml::attributes object to move from.
+
+        @since 0.10.0
+     */
+    attributes(attributes&& other);
+
     /**
         Copy the given xml::attributes object into this one.
 
@@ -97,6 +111,16 @@ class XMLWRAPP_API attributes
      */
     attributes& operator=(const attributes& other);
 
+    /**
+        Move the given xml::attributes object into this one.
+
+        @param other The xml::attributes object to move from.
+        @return   *this.
+
+        @since 0.10.0
+     */
+    attributes& operator=(attributes&& other);
+
     /**
         Swap this xml::attributes object with another one.
 
@@ -162,7 +186,9 @@ class XMLWRAPP_API attributes
 
         iterator();
         iterator(const iterator& other);
+        iterator(iterator&& other);
         iterator& operator=(const iterator& other);
+        iterator& operator=(iterator&& other);
         ~iterator();
 
         reference operator*() const;
@@ -203,8 +229,10 @@ class XMLWRAPP_API attributes
 
         const_iterator();
         const_iterator(const const_iterator& other);
+        const_iterator(const_iterator&& other);
         const_iterator(const iterator& other);
         const_iterator& operator=(const const_iterator& other);
+        const_iterator& operator=(const_iterator&& other);
         ~const_iterator();
 
         reference operator*() const;

src/libxml/ait_impl.cxx

@@ -132,7 +132,6 @@ ait_impl ait_impl::operator++(int)
 // ------------------------------------------------------------------------
 
 attributes::iterator::iterator()
-    : pimpl_{new ait_impl(nullptr, nullptr)}
 {
 }
 
@@ -163,6 +162,10 @@ attributes::iterator& attributes::iterator::operator=(const iterator& other)
 }
 
 
+attributes::iterator::iterator (iterator&& other) = default;
+attributes::iterator& attributes::iterator::operator=(iterator&& other) = default;
+
+
 void attributes::iterator::swap(iterator& other)
 {
     std::swap(pimpl_, other.pimpl_);
@@ -210,7 +213,6 @@ attributes::iterator attributes::iterator::operator++(int)
 // ------------------------------------------------------------------------
 
 attributes::const_iterator::const_iterator()
-    : pimpl_{new ait_impl(nullptr, nullptr)}
 {
 }
 
@@ -247,6 +249,10 @@ attributes::const_iterator& attributes::const_iterator::operator=(const const_it
 }
 
 
+attributes::const_iterator::const_iterator(const_iterator&& other) = default;
+attributes::const_iterator& attributes::const_iterator::operator=(const_iterator&& other) = default;
+
+
 void attributes::const_iterator::swap(const_iterator& other)
 {
     std::swap(pimpl_, other.pimpl_);
@@ -355,7 +361,7 @@ const char* attributes::attr::get_value() const
 
 bool operator==(const attributes::iterator& lhs, const attributes::iterator& rhs)
 {
-    return *(lhs.pimpl_) == *(rhs.pimpl_);
+    return ait_impl::are_equal(lhs.pimpl_.get(), rhs.pimpl_.get());
 }
 
 bool operator!=(const attributes::iterator& lhs, const attributes::iterator& rhs)
@@ -365,7 +371,7 @@ bool operator!=(const attributes::iterator& lhs, const attributes::iterator& rhs
 
 bool operator==(const attributes::const_iterator& lhs, const attributes::const_iterator& rhs)
 {
-    return *(lhs.pimpl_) == *(rhs.pimpl_);
+    return ait_impl::are_equal(lhs.pimpl_.get(), rhs.pimpl_.get());
 }
 
 bool operator!=(const attributes::const_iterator& lhs, const attributes::const_iterator& rhs)
@@ -414,16 +420,35 @@ xmlAttributePtr find_default_prop(xmlNodePtr xmlnode, const char *name)
     return nullptr;
 }
 
-bool operator==(const ait_impl& lhs, const ait_impl& rhs)
+bool ait_impl::are_equal(const ait_impl* lhs, const ait_impl* rhs)
 {
-    if (lhs.fake_ || rhs.fake_)
-        return false;
-    return lhs.xmlattr_ == rhs.xmlattr_;
-}
+    xmlAttrPtr lhsAttr;
+    if (lhs)
+    {
+        if (lhs->fake_)
+            return false;
 
-bool operator!=(const ait_impl& lhs, const ait_impl& rhs)
-{
-    return !(lhs == rhs);
+        lhsAttr = lhs->xmlattr_;
+    }
+    else
+    {
+        lhsAttr = nullptr;
+    }
+
+    xmlAttrPtr rhsAttr;
+    if (rhs)
+    {
+        if (rhs->fake_)
+            return false;
+
+        rhsAttr = rhs->xmlattr_;
+    }
+    else
+    {
+        rhsAttr = nullptr;
+    }
+
+    return lhsAttr == rhsAttr;
 }
 
 } // namespace impl

src/libxml/ait_impl.h

@@ -61,8 +61,8 @@ class ait_impl
     ait_impl& operator++();
     ait_impl  operator++(int);
 
-    friend bool operator==(const ait_impl& lhs, const ait_impl& rhs);
-    friend bool operator!=(const ait_impl& lhs, const ait_impl& rhs);
+    // Pointer arguments may be null, representing invalid attribute.
+    static bool are_equal(const ait_impl* lhs, const ait_impl* rhs);
 
 private:
     xmlNodePtr xmlnode_;

src/libxml/attributes.cxx

@@ -112,24 +112,27 @@ attributes::attributes()
 
 
 attributes::attributes(int)
-    : pimpl_{new pimpl(nullptr)}
 {
 }
 
 
 attributes::attributes(const attributes& other)
-    : pimpl_{new pimpl(*other.pimpl_)}
 {
+    if (other.pimpl_)
+        pimpl_.reset(new pimpl(*other.pimpl_));
 }
 
 
 attributes& attributes::operator=(const attributes& other)
 {
-    attributes tmp(other);
-    swap(tmp);
+    pimpl_.reset(other.pimpl_ ? new pimpl(*other.pimpl_) : nullptr);
     return *this;
 }
 
+attributes::attributes(attributes&& other) = default;
+
+attributes& attributes::operator=(attributes&& other) = default;
+
 
 void attributes::swap(attributes& other)
 {
@@ -150,9 +153,16 @@ void attributes::set_data(void *node)
 {
     auto x = static_cast<xmlNodePtr>(node);
 
-    pimpl_->free_node_if_owned();
-    pimpl_->owner_ = false;
-    pimpl_->xmlnode_ = x;
+    if (pimpl_)
+    {
+        pimpl_->free_node_if_owned();
+        pimpl_->owner_ = false;
+        pimpl_->xmlnode_ = x;
+    }
+    else
+    {
+        pimpl_.reset(new pimpl(x));
+    }
 }
 
 
@@ -241,15 +251,15 @@ void attributes::erase(const char *name)
 
 bool attributes::empty() const
 {
-    return pimpl_->xmlnode_->properties == nullptr;
+    return !pimpl_ || pimpl_->xmlnode_->properties == nullptr;
 }
 
 
 attributes::size_type attributes::size() const
 {
     size_type count = 0;
 
-    xmlAttrPtr prop = pimpl_->xmlnode_->properties;
+    xmlAttrPtr prop = pimpl_ ? pimpl_->xmlnode_->properties : nullptr;
     while (prop != nullptr)
     {
         ++count;

tests/attributes/test_attributes.cxx

@@ -296,3 +296,36 @@ TEST_CASE_METHOD( SrcdirConfig, "attributes/compare_attr_iterators", "[attribute
     CHECK( ci == i );
     CHECK( !(ci != i) );
 }
+
+/*
+ * Test to see if moving attribute objects works.
+ */
+TEST_CASE_METHOD( SrcdirConfig, "attributes/move", "[attributes]" )
+{
+    xml::tree_parser parser(test_file_path("attributes/data/06b.xml").c_str());
+
+    xml::attributes attrs1 = parser.get_document().get_root_node().get_attributes();
+    CHECK( !attrs1.empty() );
+
+    xml::attributes attrs2{std::move(attrs1)};
+    CHECK( attrs1.empty() );
+    CHECK( !attrs2.empty() );
+
+    xml::attributes attrs3;
+    attrs3 = std::move(attrs2);
+    CHECK( attrs2.empty() );
+    CHECK( !attrs3.empty() );
+
+    auto const end = xml::attributes::iterator{};
+    auto i1 = attrs3.begin();
+    CHECK( i1 != end );
+
+    auto i2{std::move(i1)};
+    CHECK( i1 == end );
+    CHECK( i2 != end );
+
+    decltype(i2) i3;
+    i3 = std::move(i2);
+    CHECK( i2 == end );
+    CHECK( i3 != end );
+}