From ce39330ff8529265a995d1aa155259508259ff18 Mon Sep 17 00:00:00 2001 From: HenryLee Date: Tue, 30 May 2017 20:23:25 +1000 Subject: [PATCH 1/8] Add converting constructors for iterator --- src/json.hpp | 64 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/src/json.hpp b/src/json.hpp index 486b9c6c1..c66b45f0f 100644 --- a/src/json.hpp +++ b/src/json.hpp @@ -8053,42 +8053,58 @@ class basic_json } } - /* - Use operator `const_iterator` instead of `const_iterator(const iterator& - other) noexcept` to avoid two class definitions for @ref iterator and - @ref const_iterator. - - This function is only called if this class is an @ref iterator. If this - class is a @ref const_iterator this function is not called. + /*! + @note The conventional copy constructor is not defined. It is replaced + by either of the following two converting constructors. + They support copy from iterator to iterator, + copy from const iterator to const iterator, + and conversion from iterator to const iterator. + However conversion from const iterator to iterator is not defined. */ - operator const_iterator() const - { - const_iterator ret; - - if (m_object) - { - ret.m_object = m_object; - ret.m_it = m_it; - } - - return ret; - } /*! - @brief copy constructor - @param[in] other iterator to copy from + @brief converting constructor + @param[in] other non-const iterator to copy from @note It is not checked whether @a other is initialized. */ - iter_impl(const iter_impl& other) noexcept + iter_impl(const iter_impl& other) noexcept + : m_object(other.m_object), m_it(other.m_it) + {} + + /*! + @brief converting constructor + @param[in] other const iterator to copy from + @note It is not checked whether @a other is initialized. + */ + iter_impl(const iter_impl& other) noexcept : m_object(other.m_object), m_it(other.m_it) {} /*! @brief copy assignment - @param[in,out] other iterator to copy from + @param[in,out] other non-const iterator to copy from + @return const/non-const iterator @note It is not checked whether @a other is initialized. */ - iter_impl& operator=(iter_impl other) noexcept( + iter_impl& operator=(iter_impl other) noexcept( + std::is_nothrow_move_constructible::value and + std::is_nothrow_move_assignable::value and + std::is_nothrow_move_constructible::value and + std::is_nothrow_move_assignable::value + ) + { + std::swap(m_object, other.m_object); + std::swap(m_it, other.m_it); + return *this; + } + + /*! + @brief copy assignment + @param[in,out] other const iterator to copy from + @return const iterator + @note It is not checked whether @a other is initialized. + */ + iter_impl& operator=(iter_impl other) noexcept( std::is_nothrow_move_constructible::value and std::is_nothrow_move_assignable::value and std::is_nothrow_move_constructible::value and From 790e7ca9e9b075f9156f86045e26313c5d7825b1 Mon Sep 17 00:00:00 2001 From: HenryLee Date: Tue, 30 May 2017 20:27:05 +1000 Subject: [PATCH 2/8] Add struct keyword in front of internal_iterator --- src/json.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/json.hpp b/src/json.hpp index c66b45f0f..88123d586 100644 --- a/src/json.hpp +++ b/src/json.hpp @@ -8089,8 +8089,8 @@ class basic_json iter_impl& operator=(iter_impl other) noexcept( std::is_nothrow_move_constructible::value and std::is_nothrow_move_assignable::value and - std::is_nothrow_move_constructible::value and - std::is_nothrow_move_assignable::value + std::is_nothrow_move_constructible::value and + std::is_nothrow_move_assignable::value ) { std::swap(m_object, other.m_object); @@ -8107,8 +8107,8 @@ class basic_json iter_impl& operator=(iter_impl other) noexcept( std::is_nothrow_move_constructible::value and std::is_nothrow_move_assignable::value and - std::is_nothrow_move_constructible::value and - std::is_nothrow_move_assignable::value + std::is_nothrow_move_constructible::value and + std::is_nothrow_move_assignable::value ) { std::swap(m_object, other.m_object); @@ -8601,7 +8601,7 @@ class basic_json /// associated JSON instance pointer m_object = nullptr; /// the actual iterator of the associated instance - internal_iterator m_it = internal_iterator(); + struct internal_iterator m_it = internal_iterator(); }; /*! From 881cd3f42024394906a337372816ce6de0d2f241 Mon Sep 17 00:00:00 2001 From: HenryLee Date: Tue, 30 May 2017 20:57:22 +1000 Subject: [PATCH 3/8] Remove the iter_impl copy constructor and copy assignment --- src/json.hpp | 40 +++++++--------------------------------- 1 file changed, 7 insertions(+), 33 deletions(-) diff --git a/src/json.hpp b/src/json.hpp index 88123d586..588cf81b3 100644 --- a/src/json.hpp +++ b/src/json.hpp @@ -8054,11 +8054,12 @@ class basic_json } /*! - @note The conventional copy constructor is not defined. It is replaced - by either of the following two converting constructors. - They support copy from iterator to iterator, - copy from const iterator to const iterator, - and conversion from iterator to const iterator. + @note The conventional copy constructor and copy assignment are + implicitly defined. + Combined with the following converting constructor and assigment, + they support: copy from iterator to iterator, + copy from const iterator to const iterator, + and conversion from iterator to const iterator. However conversion from const iterator to iterator is not defined. */ @@ -8072,16 +8073,7 @@ class basic_json {} /*! - @brief converting constructor - @param[in] other const iterator to copy from - @note It is not checked whether @a other is initialized. - */ - iter_impl(const iter_impl& other) noexcept - : m_object(other.m_object), m_it(other.m_it) - {} - - /*! - @brief copy assignment + @brief converting assignment @param[in,out] other non-const iterator to copy from @return const/non-const iterator @note It is not checked whether @a other is initialized. @@ -8098,24 +8090,6 @@ class basic_json return *this; } - /*! - @brief copy assignment - @param[in,out] other const iterator to copy from - @return const iterator - @note It is not checked whether @a other is initialized. - */ - iter_impl& operator=(iter_impl other) noexcept( - std::is_nothrow_move_constructible::value and - std::is_nothrow_move_assignable::value and - std::is_nothrow_move_constructible::value and - std::is_nothrow_move_assignable::value - ) - { - std::swap(m_object, other.m_object); - std::swap(m_it, other.m_it); - return *this; - } - private: /*! @brief set the iterator to the first value From 0a51fb22eda45ae2aee7bec4b70cbaa450b216ad Mon Sep 17 00:00:00 2001 From: HenryLee Date: Tue, 30 May 2017 23:37:24 +1000 Subject: [PATCH 4/8] Fix indentation --- src/json.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/json.hpp b/src/json.hpp index 588cf81b3..e92580c20 100644 --- a/src/json.hpp +++ b/src/json.hpp @@ -8056,9 +8056,9 @@ class basic_json /*! @note The conventional copy constructor and copy assignment are implicitly defined. - Combined with the following converting constructor and assigment, - they support: copy from iterator to iterator, - copy from const iterator to const iterator, + Combined with the following converting constructor and assigment, + they support: copy from iterator to iterator, + copy from const iterator to const iterator, and conversion from iterator to const iterator. However conversion from const iterator to iterator is not defined. */ @@ -8075,7 +8075,7 @@ class basic_json /*! @brief converting assignment @param[in,out] other non-const iterator to copy from - @return const/non-const iterator + @return const/non-const iterator @note It is not checked whether @a other is initialized. */ iter_impl& operator=(iter_impl other) noexcept( From c09a4cbbd7e175062db101898d2b4bd2b26128f0 Mon Sep 17 00:00:00 2001 From: HenryLee Date: Wed, 31 May 2017 00:31:16 +1000 Subject: [PATCH 5/8] Add test cases for iterator to const iterator conversion --- test/src/unit-iterators1.cpp | 52 ++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/test/src/unit-iterators1.cpp b/test/src/unit-iterators1.cpp index 6605076ad..348c72495 100644 --- a/test/src/unit-iterators1.cpp +++ b/test/src/unit-iterators1.cpp @@ -1511,4 +1511,56 @@ TEST_CASE("iterators 1") } } } + + SECTION("conversion from iterator to const iterator") + { + SECTION("boolean") + { + json j = true; + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + } + SECTION("string") + { + json j = "hello world"; + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + } + SECTION("array") + { + json j = {1, 2, 3}; + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + } + SECTION("object") + { + json j = {{"A", 1}, {"B", 2}, {"C", 3}}; + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + } + SECTION("number (integer)") + { + json j = 23; + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + } + SECTION("number (unsigned)") + { + json j = 23u; + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + } + SECTION("number (float)") + { + json j = 23.42; + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + } + SECTION("null") + { + json j = nullptr; + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + } + } } From 2d5f0c05499e7a0c0f94e82b8c35b03b30a1f94d Mon Sep 17 00:00:00 2001 From: HenryLee Date: Wed, 31 May 2017 00:50:14 +1000 Subject: [PATCH 6/8] Redefine the converting assignment in iterator --- src/json.hpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/json.hpp b/src/json.hpp index e92580c20..1bd8addc8 100644 --- a/src/json.hpp +++ b/src/json.hpp @@ -8078,15 +8078,10 @@ class basic_json @return const/non-const iterator @note It is not checked whether @a other is initialized. */ - iter_impl& operator=(iter_impl other) noexcept( - std::is_nothrow_move_constructible::value and - std::is_nothrow_move_assignable::value and - std::is_nothrow_move_constructible::value and - std::is_nothrow_move_assignable::value - ) + iter_impl& operator=(const iter_impl& other) noexcept { - std::swap(m_object, other.m_object); - std::swap(m_it, other.m_it); + m_object = other.m_object; + m_it = other.m_it; return *this; } From f2e164303953d468eec74a238d9539f3f5a6ff4e Mon Sep 17 00:00:00 2001 From: HenryLee Date: Wed, 31 May 2017 00:50:40 +1000 Subject: [PATCH 7/8] Add test cases for iterator to const iterator assignment --- test/src/unit-iterators1.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/src/unit-iterators1.cpp b/test/src/unit-iterators1.cpp index 348c72495..2dc9aae84 100644 --- a/test/src/unit-iterators1.cpp +++ b/test/src/unit-iterators1.cpp @@ -1519,48 +1519,64 @@ TEST_CASE("iterators 1") json j = true; json::const_iterator it = j.begin(); CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); } SECTION("string") { json j = "hello world"; json::const_iterator it = j.begin(); CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); } SECTION("array") { json j = {1, 2, 3}; json::const_iterator it = j.begin(); CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); } SECTION("object") { json j = {{"A", 1}, {"B", 2}, {"C", 3}}; json::const_iterator it = j.begin(); CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); } SECTION("number (integer)") { json j = 23; json::const_iterator it = j.begin(); CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); } SECTION("number (unsigned)") { json j = 23u; json::const_iterator it = j.begin(); CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); } SECTION("number (float)") { json j = 23.42; json::const_iterator it = j.begin(); CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); } SECTION("null") { json j = nullptr; json::const_iterator it = j.begin(); CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); } } } From 92ef19696a18187112512b8361b822d817d01e95 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Tue, 6 Jun 2017 20:48:11 +0200 Subject: [PATCH 8/8] :pencil2: cleanup after #598 --- README.md | 1 + test/src/unit-iterators1.cpp | 100 +++++++++++++++++------------------ 2 files changed, 51 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 5d84c3f27..af4f8cbad 100644 --- a/README.md +++ b/README.md @@ -840,6 +840,7 @@ I deeply appreciate the help of the following people. - [tinloaf](https://github.com/tinloaf) made sure all pushed warnings are properly popped. - [Fytch](https://github.com/Fytch) found a bug in the documentation. - [Jay Sistar](https://github.com/Type1J) implemented a Meson build description +- [Henry Lee](https://github.com/HenryRLee) fixed a warning in ICC Thanks a lot for helping out! Please [let me know](mailto:mail@nlohmann.me) if I forgot someone. diff --git a/test/src/unit-iterators1.cpp b/test/src/unit-iterators1.cpp index 2dc9aae84..783bf850a 100644 --- a/test/src/unit-iterators1.cpp +++ b/test/src/unit-iterators1.cpp @@ -1513,70 +1513,70 @@ TEST_CASE("iterators 1") } SECTION("conversion from iterator to const iterator") - { + { SECTION("boolean") - { + { json j = true; - json::const_iterator it = j.begin(); - CHECK(it == j.cbegin()); - it = j.begin(); - CHECK(it == j.cbegin()); - } + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); + } SECTION("string") - { + { json j = "hello world"; - json::const_iterator it = j.begin(); - CHECK(it == j.cbegin()); - it = j.begin(); - CHECK(it == j.cbegin()); - } + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); + } SECTION("array") - { + { json j = {1, 2, 3}; - json::const_iterator it = j.begin(); - CHECK(it == j.cbegin()); - it = j.begin(); - CHECK(it == j.cbegin()); - } + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); + } SECTION("object") - { + { json j = {{"A", 1}, {"B", 2}, {"C", 3}}; - json::const_iterator it = j.begin(); - CHECK(it == j.cbegin()); - it = j.begin(); - CHECK(it == j.cbegin()); - } + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); + } SECTION("number (integer)") - { + { json j = 23; - json::const_iterator it = j.begin(); - CHECK(it == j.cbegin()); - it = j.begin(); - CHECK(it == j.cbegin()); - } + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); + } SECTION("number (unsigned)") - { + { json j = 23u; - json::const_iterator it = j.begin(); - CHECK(it == j.cbegin()); - it = j.begin(); - CHECK(it == j.cbegin()); - } + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); + } SECTION("number (float)") - { + { json j = 23.42; - json::const_iterator it = j.begin(); - CHECK(it == j.cbegin()); - it = j.begin(); - CHECK(it == j.cbegin()); - } + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); + } SECTION("null") - { + { json j = nullptr; - json::const_iterator it = j.begin(); - CHECK(it == j.cbegin()); - it = j.begin(); - CHECK(it == j.cbegin()); - } - } + json::const_iterator it = j.begin(); + CHECK(it == j.cbegin()); + it = j.begin(); + CHECK(it == j.cbegin()); + } + } }