Don't change key ordering on FlatMap::erase (#1840)

Fixes: #1839
Signed-off-by: Juan Cruz Viotti <[email protected]>

Author: jviotti

src/core/json/include/sourcemeta/core/json_object.h

@@ -1,7 +1,6 @@
 #ifndef SOURCEMETA_CORE_JSON_OBJECT_H_
 #define SOURCEMETA_CORE_JSON_OBJECT_H_
 
-#include <algorithm>        // std::swap
 #include <cassert>          // assert
 #include <cstddef>          // std::size_t
 #include <initializer_list> // std::initializer_list
@@ -294,18 +293,18 @@ template <typename Key, typename Value, typename Hash> class FlatMap {
     const auto current_size{this->size()};
 
     if (this->hasher.is_perfect(key_hash)) {
-      for (auto &entry : this->data) {
-        if (entry.hash == key_hash) {
-          std::swap(entry, this->data.back());
-          this->data.pop_back();
+      for (auto iterator = this->data.begin(); iterator != this->data.end();
+           ++iterator) {
+        if (iterator->hash == key_hash) {
+          this->data.erase(iterator);
           return current_size - 1;
         }
       }
     } else {
-      for (auto &entry : this->data) {
-        if (entry.hash == key_hash && entry.first == key) {
-          std::swap(entry, this->data.back());
-          this->data.pop_back();
+      for (auto iterator = this->data.begin(); iterator != this->data.end();
+           ++iterator) {
+        if (iterator->hash == key_hash && iterator->first == key) {
+          this->data.erase(iterator);
           return current_size - 1;
         }
       }

test/json/json_object_test.cc

@@ -940,3 +940,23 @@ TEST(JSON_object, try_assign_before_empty) {
   EXPECT_EQ(properties.at(0), "baz");
   EXPECT_EQ(document.at("baz").to_integer(), 99);
 }
+
+TEST(JSON_object, erase_must_not_affect_ordering) {
+  sourcemeta::core::JSON document{{"x", sourcemeta::core::JSON{1}},
+                                  {"y", sourcemeta::core::JSON{2}},
+                                  {"z", sourcemeta::core::JSON{2}}};
+
+  auto before_iterator = document.as_object().begin();
+  EXPECT_EQ(before_iterator->first, "x");
+  std::advance(before_iterator, 1);
+  EXPECT_EQ(before_iterator->first, "y");
+  std::advance(before_iterator, 1);
+  EXPECT_EQ(before_iterator->first, "z");
+
+  document.erase("x");
+
+  auto after_iterator = document.as_object().begin();
+  EXPECT_EQ(after_iterator->first, "y");
+  std::advance(after_iterator, 1);
+  EXPECT_EQ(after_iterator->first, "z");
+}