From ce39330ff8529265a995d1aa155259508259ff18 Mon Sep 17 00:00:00 2001 From: HenryLee Date: Tue, 30 May 2017 20:23:25 +1000 Subject: [PATCH 1/7] 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/7] 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/7] 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/7] 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/7] 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/7] 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/7] 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()); } } }