Merge pull request #479 from biojppm/fix/477_vector_vs_map

changelog/current.md

@@ -3,3 +3,4 @@
 - [PR#488](https://github.com/biojppm/rapidyaml/pull/488):
   - add workarounds for problems with codegen of gcc 11,12,13
   - improve CI coverage of gcc and clang optimization levels
+- [BREAKING] Fix [#477](https://github.com/biojppm/rapidyaml/issues/477): changed `read<std::map>()` to overwrite existing entries. The provided implementations had an inconsistency between `std::map` (which wasn't overwriting) and `std::vector` (which *was* overwriting).

src/c4/yml/std/map.hpp

@@ -25,16 +25,17 @@ void write(c4::yml::NodeRef *n, std::map<K, V, Less, Alloc> const& m)
     }
 }
 
+/** read the node members, assigning into the existing map. If a key
+ * is already present in the map, then its value will be
+ * move-assigned. */
 template<class K, class V, class Less, class Alloc>
 bool read(c4::yml::ConstNodeRef const& n, std::map<K, V, Less, Alloc> * m)
 {
-    K k{};
-    V v{};
     for(auto const& C4_RESTRICT ch : n)
     {
+        K k{};
         ch >> c4::yml::key(k);
-        ch >> v;
-        m->emplace(std::make_pair(std::move(k), std::move(v)));
+        ch >> (*m)[k];
     }
     return true;
 }

src/c4/yml/std/vector.hpp

@@ -21,6 +21,7 @@ void write(c4::yml::NodeRef *n, std::vector<V, Alloc> const& vec)
         n->append_child() << v;
 }
 
+/** read the node members, overwriting existing vector entries. */
 template<class V, class Alloc>
 bool read(c4::yml::ConstNodeRef const& n, std::vector<V, Alloc> *vec)
 {
@@ -33,7 +34,8 @@ bool read(c4::yml::ConstNodeRef const& n, std::vector<V, Alloc> *vec)
     return true;
 }
 
-/** specialization: std::vector<bool> uses std::vector<bool>::reference as
+/** read the node members, overwriting existing vector entries.
+ * specialization: std::vector<bool> uses std::vector<bool>::reference as
  * the return value of its operator[]. */
 template<class Alloc>
 bool read(c4::yml::ConstNodeRef const& n, std::vector<bool, Alloc> *vec)

test/test_serialize.cpp

@@ -804,6 +804,39 @@ TEST(serialize, issue442_61)
 }
 
 
+TEST(serialize, issue477_vec)
+{
+    const Tree t = parse_in_arena(R"([0, 10, 20, 30, 40])");
+    std::vector<int> vec = {100, 1, 2, 3};
+    ASSERT_EQ(vec.size(), 4);
+    EXPECT_EQ(vec[0], 100);
+    EXPECT_EQ(vec[1], 1);
+    EXPECT_EQ(vec[2], 2);
+    EXPECT_EQ(vec[3], 3);
+    t.rootref() >> vec;
+    ASSERT_EQ(vec.size(), 5);
+    EXPECT_EQ(vec[0], 0);
+    EXPECT_EQ(vec[1], 10);
+    EXPECT_EQ(vec[2], 20);
+    EXPECT_EQ(vec[3], 30);
+    EXPECT_EQ(vec[4], 40);
+}
+
+TEST(serialize, issue477_map)
+{
+    const Tree t = parse_in_arena(R"({0: 10,  2: 30,  4: 50})");
+    std::map<int,int> map = {{0, 1}, {2, 3}};
+    ASSERT_EQ(map.size(), 2);
+    EXPECT_EQ(map[0], 1);
+    EXPECT_EQ(map[2], 3);
+    t.rootref() >> map;
+    ASSERT_EQ(map.size(), 3); // added a new member
+    EXPECT_EQ(map[0], 10); // modified
+    EXPECT_EQ(map[2], 30); // modified
+    EXPECT_EQ(map[4], 50);
+}
+
+
 //-------------------------------------------
 // this is needed to use the test case library
 Case const* get_case(csubstr /*name*/)