Close open submessage when move-assigning onto a writer

include/protozero/basic_pbf_writer.hpp

@@ -282,17 +282,15 @@ class basic_pbf_writer {
 
     /**
      * A basic_pbf_writer object can be moved. After this the other
-     * basic_pbf_writer will be invalid.
+     * basic_pbf_writer will be invalid. If this basic_pbf_writer is for an
+     * open submessage, that submessage is closed first, the same way the
+     * destructor does.
      */
     basic_pbf_writer& operator=(basic_pbf_writer&& other) noexcept {
-        m_data = other.m_data;
-        m_parent_writer = other.m_parent_writer;
-        m_rollback_pos = other.m_rollback_pos;
-        m_pos = other.m_pos;
-        other.m_data = nullptr;
-        other.m_parent_writer = nullptr;
-        other.m_rollback_pos = 0;
-        other.m_pos = 0;
+        // The temporary takes over the old state of *this and closes its
+        // submessage (if any) when it is destructed.
+        basic_pbf_writer tmp{std::move(other)};
+        swap(tmp);
         return *this;
     }
 

test/t/message/reader_test_cases.cpp

@@ -133,3 +133,48 @@ TEST_CASE("write optional message field") {
     }
 }
 
+
+TEST_CASE("move-assigning onto an open subwriter closes its submessage") {
+    std::string buffer;
+    protozero::pbf_writer pbf_test{buffer};
+
+    protozero::pbf_writer pbf_submessage{pbf_test, 1};
+    pbf_submessage.add_string(1, "foobar");
+
+    SECTION("move-assign default constructed writer") {
+        pbf_submessage = protozero::pbf_writer{};
+        REQUIRE_FALSE(pbf_submessage.valid());
+        REQUIRE(buffer == load_data("message/data-message"));
+    }
+
+    SECTION("move-assign subwriter of another parent") {
+        std::string other_buffer;
+        protozero::pbf_writer other_pbf{other_buffer};
+        protozero::pbf_writer other_submessage{other_pbf, 1};
+        other_submessage.add_string(1, "foobar");
+
+        pbf_submessage = std::move(other_submessage);
+        REQUIRE(pbf_submessage.valid());
+        REQUIRE(buffer == load_data("message/data-message"));
+
+        pbf_submessage.commit();
+        REQUIRE(other_buffer == load_data("message/data-message"));
+    }
+
+    // The parent must be usable again after the submessage was closed.
+    REQUIRE_NOTHROW(pbf_test.add_string(2, "x"));
+}
+
+TEST_CASE("self-move-assigning an open subwriter keeps it open") {
+    std::string buffer;
+    protozero::pbf_writer pbf_test{buffer};
+
+    protozero::pbf_writer pbf_submessage{pbf_test, 1};
+    auto& alias = pbf_submessage;
+    pbf_submessage = std::move(alias);
+    REQUIRE(pbf_submessage.valid());
+
+    pbf_submessage.add_string(1, "foobar");
+    pbf_submessage.commit();
+    REQUIRE(buffer == load_data("message/data-message"));
+}