diff --git a/include/nlohmann/ordered_map.hpp b/include/nlohmann/ordered_map.hpp index cf5f133e70..51b954524b 100644 --- a/include/nlohmann/ordered_map.hpp +++ b/include/nlohmann/ordered_map.hpp @@ -107,16 +107,55 @@ template , iterator erase(iterator pos) { - auto it = pos; + return erase(pos, std::next(pos)); + } - // Since we cannot move const Keys, re-construct them in place - for (auto next = it; ++next != this->end(); ++it) + iterator erase(iterator first, iterator last) + { + const auto elements_affected = std::distance(first, last); + const auto offset = std::distance(Container::begin(), first); + + // This is the start situation. We need to delete elements_affected + // elements (3 in this example: e, f, g), and need to return an + // iterator past the last deleted element (h in this example). + // Note that offset is the distance from the start of the vector + // to first. We will need this later. + + // [ a, b, c, d, e, f, g, h, i, j ] + // ^ ^ + // first last + + // Since we cannot move const Keys, we re-construct them in place. + // We start at first and re-construct (viz. copy) the elements from + // the back of the vector. Example for first iteration: + + // ,--------. + // v | destroy e and re-construct with h + // [ a, b, c, d, e, f, g, h, i, j ] + // ^ ^ + // it it + elements_affected + + for (auto it = first; std::next(it, elements_affected) != Container::end(); ++it) { - it->~value_type(); // Destroy but keep allocation - new (&*it) value_type{std::move(*next)}; + it->~value_type(); // destroy but keep allocation + new (&*it) value_type{std::move(*std::next(it, elements_affected))}; // "move" next element to it } - Container::pop_back(); - return pos; + + // [ a, b, c, d, h, i, j, h, i, j ] + // ^ ^ + // first last + + // remove the unneeded elements at the end of the vector + Container::resize(this->size() - static_cast(elements_affected)); + + // [ a, b, c, d, h, i, j ] + // ^ ^ + // first last + + // first is now pointing past the last deleted element, but we cannot + // use this iterator, because it may have been invalidated by the + // resize call. Instead, we can return begin() + offset. + return Container::begin() + offset; } size_type count(const Key& key) const diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 5e25c98e39..5800976306 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -17494,16 +17494,55 @@ template , iterator erase(iterator pos) { - auto it = pos; + return erase(pos, std::next(pos)); + } + + iterator erase(iterator first, iterator last) + { + const auto elements_affected = std::distance(first, last); + const auto offset = std::distance(Container::begin(), first); + + // This is the start situation. We need to delete elements_affected + // elements (3 in this example: e, f, g), and need to return an + // iterator past the last deleted element (h in this example). + // Note that offset is the distance from the start of the vector + // to first. We will need this later. + + // [ a, b, c, d, e, f, g, h, i, j ] + // ^ ^ + // first last + + // Since we cannot move const Keys, we re-construct them in place. + // We start at first and re-construct (viz. copy) the elements from + // the back of the vector. Example for first iteration: - // Since we cannot move const Keys, re-construct them in place - for (auto next = it; ++next != this->end(); ++it) + // ,--------. + // v | destroy e and re-construct with h + // [ a, b, c, d, e, f, g, h, i, j ] + // ^ ^ + // it it + elements_affected + + for (auto it = first; std::next(it, elements_affected) != Container::end(); ++it) { - it->~value_type(); // Destroy but keep allocation - new (&*it) value_type{std::move(*next)}; + it->~value_type(); // destroy but keep allocation + new (&*it) value_type{std::move(*std::next(it, elements_affected))}; // "move" next element to it } - Container::pop_back(); - return pos; + + // [ a, b, c, d, h, i, j, h, i, j ] + // ^ ^ + // first last + + // remove the unneeded elements at the end of the vector + Container::resize(this->size() - static_cast(elements_affected)); + + // [ a, b, c, d, h, i, j ] + // ^ ^ + // first last + + // first is now pointing past the last deleted element, but we cannot + // use this iterator, because it may have been invalidated by the + // resize call. Instead, we can return begin() + offset. + return Container::begin() + offset; } size_type count(const Key& key) const diff --git a/test/src/unit-ordered_map.cpp b/test/src/unit-ordered_map.cpp index 7a23e36fb3..a8f2850c8c 100644 --- a/test/src/unit-ordered_map.cpp +++ b/test/src/unit-ordered_map.cpp @@ -210,6 +210,45 @@ TEST_CASE("ordered_map") ++it2; CHECK(it2 == om.end()); } + + SECTION("with iterator pair") + { + SECTION("range in the middle") + { + // need more elements + om["vier"] = "four"; + om["fünf"] = "five"; + + // delete "zwei" and "drei" + auto it = om.erase(om.begin() + 1, om.begin() + 3); + CHECK(it->first == "vier"); + CHECK(om.size() == 3); + } + + SECTION("range at the beginning") + { + // need more elements + om["vier"] = "four"; + om["fünf"] = "five"; + + // delete "eins" and "zwei" + auto it = om.erase(om.begin(), om.begin() + 2); + CHECK(it->first == "drei"); + CHECK(om.size() == 3); + } + + SECTION("range at the end") + { + // need more elements + om["vier"] = "four"; + om["fünf"] = "five"; + + // delete "vier" and "fünf" + auto it = om.erase(om.begin() + 3, om.end()); + CHECK(it == om.end()); + CHECK(om.size() == 3); + } + } } SECTION("count") diff --git a/test/src/unit-regression2.cpp b/test/src/unit-regression2.cpp index f0c4ef4519..b2f2a7dc37 100644 --- a/test/src/unit-regression2.cpp +++ b/test/src/unit-regression2.cpp @@ -746,6 +746,23 @@ TEST_CASE("regression tests 2") std::vector foo; j.get_to(foo); } + + SECTION("issue #3108 - ordered_json doesn't support range based erase") + { + ordered_json j = {1, 2, 2, 4}; + + auto last = std::unique(j.begin(), j.end()); + j.erase(last, j.end()); + + CHECK(j.dump() == "[1,2,4]"); + + j.erase(std::remove_if(j.begin(), j.end(), [](const ordered_json & val) + { + return val == 2; + }), j.end()); + + CHECK(j.dump() == "[1,4]"); + } } DOCTEST_CLANG_SUPPRESS_WARNING_POP