luo980/json
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make iterator operator++/--(int) equality-preserving (#3332)

Browse Source 
Commit f28fc22 introduced const qualifiers on post-(inc-/dec-)rement
operators of iterators. These qualifiers prevent the use of basic_json
in place of std::ranges::range, which requires the post-increment
operator to be equality-preserving.

These changes appear to be the result of ICC compiler suggestions, and
no further explanation is discernible from the PR discussion (#858).
Further testing revealed, that clang-tidy also suggests adding const to
prevent "accidental mutation of a temporary object".

As an alternative, this commit partially reverts f28fc22, removing all
added const qualifiers from return types and adds lvalue reference
qualifiers to the operator member functions instead.

Unit tests ensure the operators remain equality-preserving and
accidental mutation of temporaries following post-(inc-/dec-)rement is
prohibited.

Fixes #3331.
This commit is contained in:
Florian Albrechtskirchinger 2022-03-08 10:10:50 +01:00 committed by GitHub
parent f208a9c19b
commit 700b95f447
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 103 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
include/nlohmann/detail/iterators/iter_impl.hpp
View File

@ -352,7 +352,7 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci
@brief post-increment (it++)
@pre The iterator is initialized; i.e. `m_object != nullptr`.
*/
iter_impl const operator++(int) // NOLINT(readability-const-return-type)
iter_impl operator++(int)& // NOLINT(cert-dcl21-cpp)
{
auto result = *this;
++(*this);
@ -403,7 +403,7 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci
@brief post-decrement (it--)
@pre The iterator is initialized; i.e. `m_object != nullptr`.
*/
iter_impl const operator--(int) // NOLINT(readability-const-return-type)
iter_impl operator--(int)& // NOLINT(cert-dcl21-cpp)
{
auto result = *this;
--(*this);

4
include/nlohmann/detail/iterators/json_reverse_iterator.hpp
View File

@ -48,7 +48,7 @@ class json_reverse_iterator : public std::reverse_iterator<Base>
explicit json_reverse_iterator(const base_iterator& it) noexcept : base_iterator(it) {}
/// post-increment (it++)
json_reverse_iterator const operator++(int) // NOLINT(readability-const-return-type)
json_reverse_iterator operator++(int)& // NOLINT(cert-dcl21-cpp)
{
return static_cast<json_reverse_iterator>(base_iterator::operator++(1));
}
@ -60,7 +60,7 @@ class json_reverse_iterator : public std::reverse_iterator<Base>
}
/// post-decrement (it--)
json_reverse_iterator const operator--(int) // NOLINT(readability-const-return-type)
json_reverse_iterator operator--(int)& // NOLINT(cert-dcl21-cpp)
{
return static_cast<json_reverse_iterator>(base_iterator::operator--(1));
}

4
include/nlohmann/detail/iterators/primitive_iterator.hpp
View File

@ -87,7 +87,7 @@ class primitive_iterator_t
return *this;
}
primitive_iterator_t const operator++(int) noexcept // NOLINT(readability-const-return-type)
primitive_iterator_t operator++(int)& noexcept // NOLINT(cert-dcl21-cpp)
{
auto result = *this;
++m_it;
@ -100,7 +100,7 @@ class primitive_iterator_t
return *this;
}
primitive_iterator_t const operator--(int) noexcept // NOLINT(readability-const-return-type)
primitive_iterator_t operator--(int)& noexcept // NOLINT(cert-dcl21-cpp)
{
auto result = *this;
--m_it;

12
single_include/nlohmann/json.hpp
View File

@ -11308,7 +11308,7 @@ class primitive_iterator_t
return *this;
}
primitive_iterator_t const operator++(int) noexcept // NOLINT(readability-const-return-type)
primitive_iterator_t operator++(int)& noexcept // NOLINT(cert-dcl21-cpp)
{
auto result = *this;
++m_it;
@ -11321,7 +11321,7 @@ class primitive_iterator_t
return *this;
}
primitive_iterator_t const operator--(int) noexcept // NOLINT(readability-const-return-type)
primitive_iterator_t operator--(int)& noexcept // NOLINT(cert-dcl21-cpp)
{
auto result = *this;
--m_it;
@ -11728,7 +11728,7 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci
@brief post-increment (it++)
@pre The iterator is initialized; i.e. `m_object != nullptr`.
*/
iter_impl const operator++(int) // NOLINT(readability-const-return-type)
iter_impl operator++(int)& // NOLINT(cert-dcl21-cpp)
{
auto result = *this;
++(*this);
@ -11779,7 +11779,7 @@ class iter_impl // NOLINT(cppcoreguidelines-special-member-functions,hicpp-speci
@brief post-decrement (it--)
@pre The iterator is initialized; i.e. `m_object != nullptr`.
*/
iter_impl const operator--(int) // NOLINT(readability-const-return-type)
iter_impl operator--(int)& // NOLINT(cert-dcl21-cpp)
{
auto result = *this;
--(*this);
@ -12167,7 +12167,7 @@ class json_reverse_iterator : public std::reverse_iterator<Base>
explicit json_reverse_iterator(const base_iterator& it) noexcept : base_iterator(it) {}
/// post-increment (it++)
json_reverse_iterator const operator++(int) // NOLINT(readability-const-return-type)
json_reverse_iterator operator++(int)& // NOLINT(cert-dcl21-cpp)
{
return static_cast<json_reverse_iterator>(base_iterator::operator++(1));
}
@ -12179,7 +12179,7 @@ class json_reverse_iterator : public std::reverse_iterator<Base>
}
/// post-decrement (it--)
json_reverse_iterator const operator--(int) // NOLINT(readability-const-return-type)
json_reverse_iterator operator--(int)& // NOLINT(cert-dcl21-cpp)
{
return static_cast<json_reverse_iterator>(base_iterator::operator--(1));
}

91
test/src/unit-class_iterator.cpp
View File

@ -33,6 +33,12 @@ SOFTWARE.
#include <nlohmann/json.hpp>
using nlohmann::json;
template<typename Iter>
using can_post_increment_temporary = decltype((std::declval<Iter>()++)++);
template<typename Iter>
using can_post_decrement_temporary = decltype((std::declval<Iter>()--)--);
TEST_CASE("iterator class")
{
SECTION("construction")
@ -399,4 +405,89 @@ TEST_CASE("iterator class")
}
}
}
SECTION("equality-preserving")
{
SECTION("post-increment")
{
SECTION("primitive_iterator_t")
{
using Iter = nlohmann::detail::primitive_iterator_t;
CHECK(std::is_same < decltype(std::declval<Iter&>()++), Iter >::value);
}
SECTION("iter_impl")
{
using Iter = nlohmann::detail::iter_impl<json>;
CHECK(std::is_same < decltype(std::declval<Iter&>()++), Iter >::value);
}
SECTION("json_reverse_iterator")
{
using Base = nlohmann::detail::iter_impl<json>;
using Iter = nlohmann::detail::json_reverse_iterator<Base>;
CHECK(std::is_same < decltype(std::declval<Iter&>()++), Iter >::value);
}
}
SECTION("post-decrement")
{
SECTION("primitive_iterator_t")
{
using Iter = nlohmann::detail::primitive_iterator_t;
CHECK(std::is_same < decltype(std::declval<Iter&>()--), Iter >::value);
}
SECTION("iter_impl")
{
using Iter = nlohmann::detail::iter_impl<json>;
CHECK(std::is_same < decltype(std::declval<Iter&>()--), Iter >::value );
}
SECTION("json_reverse_iterator")
{
using Base = nlohmann::detail::iter_impl<json>;
using Iter = nlohmann::detail::json_reverse_iterator<Base>;
CHECK(std::is_same < decltype(std::declval<Iter&>()--), Iter >::value );
}
}
}
// prevent "accidental mutation of a temporary object"
SECTION("cert-dcl21-cpp")
{
using nlohmann::detail::is_detected;
SECTION("post-increment")
{
SECTION("primitive_iterator_t")
{
using Iter = nlohmann::detail::primitive_iterator_t;
CHECK_FALSE(is_detected<can_post_increment_temporary, Iter&>::value);
}
SECTION("iter_impl")
{
using Iter = nlohmann::detail::iter_impl<json>;
CHECK_FALSE(is_detected<can_post_increment_temporary, Iter&>::value);
}
SECTION("json_reverse_iterator")
{
using Base = nlohmann::detail::iter_impl<json>;
using Iter = nlohmann::detail::json_reverse_iterator<Base>;
CHECK_FALSE(is_detected<can_post_increment_temporary, Iter&>::value);
}
}
SECTION("post-decrement")
{
SECTION("primitive_iterator_t")
{
using Iter = nlohmann::detail::primitive_iterator_t;
CHECK_FALSE(is_detected<can_post_decrement_temporary, Iter&>::value);
}
SECTION("iter_impl")
{
using Iter = nlohmann::detail::iter_impl<json>;
CHECK_FALSE(is_detected<can_post_decrement_temporary, Iter&>::value);
}
SECTION("json_reverse_iterator")
{
using Base = nlohmann::detail::iter_impl<json>;
using Iter = nlohmann::detail::json_reverse_iterator<Base>;
CHECK_FALSE(is_detected<can_post_decrement_temporary, Iter&>::value);
}
}
}
}