Assign the member that lookups find when set() meets duplicate keys

An editable document reads the last member of a repeated key (as the
read-only document does), but set(object, key, value) assigned the first
one: a view taken from object[key] before the call did not show the new
value. It now assigns the last member's value, keeps the key at the
position of its first occurrence (where materialize() puts it), and drops
the other members.

The seeded differential test now also edits documents whose objects
repeat keys and compares every lookup (operator[], at, find, value,
contains, count, and JSON pointers) with the parsed basic_json value.
A targeted test covers duplicates before and after edits that move the
object, in large objects with an index, in moved arrays, and in values
copied from other documents.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

include/nlohmann/detail/view/edit.hpp

@@ -152,25 +152,23 @@ class editor
         {
             become_empty(o, value_t::object);
         }
-        // an existing member: assign it (and drop later duplicates, so that
-        // lookups, iteration, and materialize() agree)
+        // an existing member: assign the one that lookups find (the last
+        // one, should the key occur more than once), and drop the others, so
+        // that lookups, iteration, and materialize() agree. The key stays at
+        // the position of its first occurrence, as materialize() puts it.
         node* slot = nullptr;
-        bool duplicates = false;
+        std::size_t matches = 0;
         for (const node* k = nav::first(m_doc, o), *end = nav::end(m_doc, o); k != end; k = document_data::after(k + 1))
         {
             if (key_equals(*k, key))
             {
-                if (slot != nullptr)
-                {
-                    duplicates = true;
-                    break;
-                }
                 slot = const_cast<node*>(nav::value(k + 1)); // NOLINT(cppcoreguidelines-pro-type-const-cast): the nodes belong to this document
+                ++matches;
             }
         }
         if (slot != nullptr)
         {
-            if (duplicates)
+            if (matches > 1)
             {
                 erase_members(o, key, true);
             }
@@ -316,27 +314,38 @@ class editor
         return k.len == key.size() && (key.size() == 0 || std::memcmp(m_doc.str(k), key.data(), key.size()) == 0);
     }
 
-    /// remove the members with this key (all, or all but the first) from an object
-    std::size_t erase_members(node* o, string_view_t key, bool keep_first)
+    /// Remove the members with this key from an object: all of them, or all
+    /// but one. That one stays where the first occurrence is, but holds the
+    /// value of the last (the one that lookups find, which views may refer to).
+    std::size_t erase_members(node* o, string_view_t key, bool keep_one)
     {
         node* const h = block_of(m_doc, o, 0);
+        node last_value{}; // the entry of the value of the last member
+        node* const end = h + h->next;
+        if (keep_one)
+        {
+            for (node* r = h + 1; r != end; r += 2)
+            {
+                if (key_equals(*r, key))
+                {
+                    last_value = r[1];
+                }
+            }
+        }
         node* w = h + 1;
         std::size_t erased = 0;
         bool kept = false;
-        for (node* r = h + 1, *end = h + h->next; r != end; r += 2)
+        for (node* r = h + 1; r != end; r += 2)
         {
             const bool match = key_equals(*r, key);
-            if (match && (kept || !keep_first))
+            if (match && (kept || !keep_one))
             {
                 ++erased;
                 continue;
             }
+            w[0] = r[0];
+            w[1] = match ? last_value : r[1];
             kept = kept || match;
-            if (w != r)
-            {
-                w[0] = r[0];
-                w[1] = r[1];
-            }
             w += 2;
         }
         h->next = static_cast<std::uint32_t>(w - h);