Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress Clang-Tidy warning #4311

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
1 change: 1 addition & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Checks: '*,
-modernize-concat-nested-namespaces,
-modernize-type-traits,
-modernize-use-constraints,
-modernize-use-designated-initializers,
-modernize-use-nodiscard,
-modernize-use-std-numbers,
-modernize-use-trailing-return-type,
Expand Down
25 changes: 22 additions & 3 deletions include/nlohmann/detail/conversions/from_json.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,14 @@ template < typename BasicJsonType, typename T, std::size_t... Idx >
std::array<T, sizeof...(Idx)> from_json_inplace_array_impl(BasicJsonType&& j,
identity_tag<std::array<T, sizeof...(Idx)>> /*unused*/, index_sequence<Idx...> /*unused*/)
{
return { { std::forward<BasicJsonType>(j).at(Idx).template get<T>()... } };
return { { std::forward<BasicJsonType>(j).at(Idx).template get < T&& > ()... } };
}

template < typename BasicJsonType, typename T, std::size_t... Idx >
std::array<T, sizeof...(Idx)> from_json_inplace_array_impl(const BasicJsonType& j,
identity_tag<std::array<T, sizeof...(Idx)>> /*unused*/, index_sequence<Idx...> /*unused*/)
{
return { { j.at(Idx).template get<T>()... } };
}

template < typename BasicJsonType, typename T, std::size_t N >
Expand Down Expand Up @@ -373,6 +380,12 @@ inline void from_json(const BasicJsonType& j, ArithmeticType& val)
}
}

template<typename BasicJsonType, typename... Args, std::size_t... Idx>
std::tuple<Args...> from_json_tuple_impl_base(const BasicJsonType& j, index_sequence<Idx...> /*unused*/)
{
return std::make_tuple(j.at(Idx).template get<Args>()...);
}

template<typename BasicJsonType, typename... Args, std::size_t... Idx>
std::tuple<Args...> from_json_tuple_impl_base(BasicJsonType&& j, index_sequence<Idx...> /*unused*/)
{
Expand All @@ -382,8 +395,14 @@ std::tuple<Args...> from_json_tuple_impl_base(BasicJsonType&& j, index_sequence<
template < typename BasicJsonType, class A1, class A2 >
std::pair<A1, A2> from_json_tuple_impl(BasicJsonType&& j, identity_tag<std::pair<A1, A2>> /*unused*/, priority_tag<0> /*unused*/)
{
return {std::forward<BasicJsonType>(j).at(0).template get<A1>(),
std::forward<BasicJsonType>(j).at(1).template get<A2>()};
return {std::forward<BasicJsonType>(j).at(0).template get < A1&& > (),
std::forward<BasicJsonType>(j).at(1).template get < A2&& > ()};
}

template < typename BasicJsonType, class A1, class A2 >
std::pair<A1, A2> from_json_tuple_impl(const BasicJsonType& j, identity_tag<std::pair<A1, A2>> /*unused*/, priority_tag<0> /*unused*/)
{
return {j.at(0).template get<A1>(), j.at(1).template get<A2>()};
}

template<typename BasicJsonType, typename A1, typename A2>
Expand Down
2 changes: 1 addition & 1 deletion include/nlohmann/detail/meta/cpp_future.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ struct static_const
#endif

template<typename T, typename... Args>
inline constexpr std::array<T, sizeof...(Args)> make_array(Args&& ... args)
constexpr std::array<T, sizeof...(Args)> make_array(Args&& ... args)
{
return std::array<T, sizeof...(Args)> {{static_cast<T>(std::forward<Args>(args))...}};
}
Expand Down
6 changes: 3 additions & 3 deletions include/nlohmann/detail/meta/type_traits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ struct value_in_range_of_impl1<OfType, T, true>
};

template<typename OfType, typename T>
inline constexpr bool value_in_range_of(T val)
constexpr bool value_in_range_of(T val)
{
return value_in_range_of_impl1<OfType, T>::test(val);
}
Expand All @@ -750,7 +750,7 @@ namespace impl
{

template<typename T>
inline constexpr bool is_c_string()
constexpr bool is_c_string()
{
using TUnExt = typename std::remove_extent<T>::type;
using TUnCVExt = typename std::remove_cv<TUnExt>::type;
Expand Down Expand Up @@ -778,7 +778,7 @@ namespace impl
{

template<typename T>
inline constexpr bool is_transparent()
constexpr bool is_transparent()
{
return is_detected<detect_is_transparent, T>::value;
}
Expand Down
4 changes: 2 additions & 2 deletions include/nlohmann/detail/output/serializer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ class serializer
@param[in] x unsigned integer number to count its digits
@return number of decimal digits
*/
inline unsigned int count_digits(number_unsigned_t x) noexcept
unsigned int count_digits(number_unsigned_t x) noexcept
{
unsigned int n_digits = 1;
for (;;)
Expand Down Expand Up @@ -952,7 +952,7 @@ class serializer
* absolute values of INT_MIN and INT_MAX are usually not the same. See
* #1708 for details.
*/
inline number_unsigned_t remove_sign(number_integer_t x) noexcept
number_unsigned_t remove_sign(number_integer_t x) noexcept
{
JSON_ASSERT(x < 0 && x < (std::numeric_limits<number_integer_t>::max)()); // NOLINT(misc-redundant-expression)
return static_cast<number_unsigned_t>(-(x + 1)) + 1;
Expand Down
16 changes: 7 additions & 9 deletions include/nlohmann/json.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1212,15 +1212,13 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
/// @brief move constructor
/// @sa https://json.nlohmann.me/api/basic_json/basic_json/
basic_json(basic_json&& other) noexcept
: json_base_class_t(std::forward<json_base_class_t>(other)),
m_data(std::move(other.m_data))
// check that passed value is valid (has to be done before forwarding)
: json_base_class_t((other.assert_invariant(false), std::forward<json_base_class_t>(other))),
Copy link
Contributor

@gregmarr gregmarr Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just do assert_invariant(false); in the body (assert it in this after moving instead of asserting in other) to avoid this mess?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another possibility for avoiding all these issues is to do std::swap() on m_data and other.m_data in the body.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not at all happy with this. I also thought about swapping...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for asking a question for something that is probably obvious, but which constructor of json_base_class_t is this call valid for? I suppose it is one of the basic_json constructors being used here?

FWIW, it should be completely fine to check the invariants after the initializer list has been evaluated. Is there a possibility to consider rewriting the data move constructor so these assignments don't need to be done at this level?

m_data(std::move(other.m_data))// NOLINT(bugprone-use-after-move,hicpp-invalid-access-moved)
{
// check that passed value is valid
other.assert_invariant(false);

// invalidate payload
other.m_data.m_type = value_t::null;
other.m_data.m_value = {};
other.m_data.m_type = value_t::null; // NOLINT(bugprone-use-after-move,hicpp-invalid-access-moved)
other.m_data.m_value = {};// NOLINT(bugprone-use-after-move,hicpp-invalid-access-moved)

set_parents();
assert_invariant();
Expand Down Expand Up @@ -2012,7 +2010,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
auto it = m_data.m_value.object->find(std::forward<KeyType>(key));
if (it == m_data.m_value.object->end())
{
JSON_THROW(out_of_range::create(403, detail::concat("key '", string_t(std::forward<KeyType>(key)), "' not found"), this));
JSON_THROW(out_of_range::create(403, "key not found (key is an rvalue and cannot be shown)", this));
}
return set_parent(it->second);
}
Expand Down Expand Up @@ -2050,7 +2048,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
auto it = m_data.m_value.object->find(std::forward<KeyType>(key));
if (it == m_data.m_value.object->end())
{
JSON_THROW(out_of_range::create(403, detail::concat("key '", string_t(std::forward<KeyType>(key)), "' not found"), this));
JSON_THROW(out_of_range::create(403, "key not found (key is an rvalue and cannot be shown)", this));
}
return it->second;
}
Expand Down
53 changes: 35 additions & 18 deletions single_include/nlohmann/json.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3219,7 +3219,7 @@ struct static_const
#endif

template<typename T, typename... Args>
inline constexpr std::array<T, sizeof...(Args)> make_array(Args&& ... args)
constexpr std::array<T, sizeof...(Args)> make_array(Args&& ... args)
{
return std::array<T, sizeof...(Args)> {{static_cast<T>(std::forward<Args>(args))...}};
}
Expand Down Expand Up @@ -4147,7 +4147,7 @@ struct value_in_range_of_impl1<OfType, T, true>
};

template<typename OfType, typename T>
inline constexpr bool value_in_range_of(T val)
constexpr bool value_in_range_of(T val)
{
return value_in_range_of_impl1<OfType, T>::test(val);
}
Expand All @@ -4163,7 +4163,7 @@ namespace impl
{

template<typename T>
inline constexpr bool is_c_string()
constexpr bool is_c_string()
{
using TUnExt = typename std::remove_extent<T>::type;
using TUnCVExt = typename std::remove_cv<TUnExt>::type;
Expand Down Expand Up @@ -4191,7 +4191,7 @@ namespace impl
{

template<typename T>
inline constexpr bool is_transparent()
constexpr bool is_transparent()
{
return is_detected<detect_is_transparent, T>::value;
}
Expand Down Expand Up @@ -4904,7 +4904,14 @@ template < typename BasicJsonType, typename T, std::size_t... Idx >
std::array<T, sizeof...(Idx)> from_json_inplace_array_impl(BasicJsonType&& j,
identity_tag<std::array<T, sizeof...(Idx)>> /*unused*/, index_sequence<Idx...> /*unused*/)
{
return { { std::forward<BasicJsonType>(j).at(Idx).template get<T>()... } };
return { { std::forward<BasicJsonType>(j).at(Idx).template get < T&& > ()... } };
}

template < typename BasicJsonType, typename T, std::size_t... Idx >
std::array<T, sizeof...(Idx)> from_json_inplace_array_impl(const BasicJsonType& j,
identity_tag<std::array<T, sizeof...(Idx)>> /*unused*/, index_sequence<Idx...> /*unused*/)
{
return { { j.at(Idx).template get<T>()... } };
}

template < typename BasicJsonType, typename T, std::size_t N >
Expand Down Expand Up @@ -5000,6 +5007,12 @@ inline void from_json(const BasicJsonType& j, ArithmeticType& val)
}
}

template<typename BasicJsonType, typename... Args, std::size_t... Idx>
std::tuple<Args...> from_json_tuple_impl_base(const BasicJsonType& j, index_sequence<Idx...> /*unused*/)
{
return std::make_tuple(j.at(Idx).template get<Args>()...);
}

template<typename BasicJsonType, typename... Args, std::size_t... Idx>
std::tuple<Args...> from_json_tuple_impl_base(BasicJsonType&& j, index_sequence<Idx...> /*unused*/)
{
Expand All @@ -5009,8 +5022,14 @@ std::tuple<Args...> from_json_tuple_impl_base(BasicJsonType&& j, index_sequence<
template < typename BasicJsonType, class A1, class A2 >
std::pair<A1, A2> from_json_tuple_impl(BasicJsonType&& j, identity_tag<std::pair<A1, A2>> /*unused*/, priority_tag<0> /*unused*/)
{
return {std::forward<BasicJsonType>(j).at(0).template get<A1>(),
std::forward<BasicJsonType>(j).at(1).template get<A2>()};
return {std::forward<BasicJsonType>(j).at(0).template get < A1&& > (),
std::forward<BasicJsonType>(j).at(1).template get < A2&& > ()};
}

template < typename BasicJsonType, class A1, class A2 >
std::pair<A1, A2> from_json_tuple_impl(const BasicJsonType& j, identity_tag<std::pair<A1, A2>> /*unused*/, priority_tag<0> /*unused*/)
{
return {j.at(0).template get<A1>(), j.at(1).template get<A2>()};
}

template<typename BasicJsonType, typename A1, typename A2>
Expand Down Expand Up @@ -18652,7 +18671,7 @@ class serializer
@param[in] x unsigned integer number to count its digits
@return number of decimal digits
*/
inline unsigned int count_digits(number_unsigned_t x) noexcept
unsigned int count_digits(number_unsigned_t x) noexcept
{
unsigned int n_digits = 1;
for (;;)
Expand Down Expand Up @@ -18961,7 +18980,7 @@ class serializer
* absolute values of INT_MIN and INT_MAX are usually not the same. See
* #1708 for details.
*/
inline number_unsigned_t remove_sign(number_integer_t x) noexcept
number_unsigned_t remove_sign(number_integer_t x) noexcept
{
JSON_ASSERT(x < 0 && x < (std::numeric_limits<number_integer_t>::max)()); // NOLINT(misc-redundant-expression)
return static_cast<number_unsigned_t>(-(x + 1)) + 1;
Expand Down Expand Up @@ -20515,15 +20534,13 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
/// @brief move constructor
/// @sa https://json.nlohmann.me/api/basic_json/basic_json/
basic_json(basic_json&& other) noexcept
: json_base_class_t(std::forward<json_base_class_t>(other)),
m_data(std::move(other.m_data))
// check that passed value is valid (has to be done before forwarding)
: json_base_class_t((other.assert_invariant(false), std::forward<json_base_class_t>(other))),
m_data(std::move(other.m_data))// NOLINT(bugprone-use-after-move,hicpp-invalid-access-moved)
{
// check that passed value is valid
other.assert_invariant(false);

// invalidate payload
other.m_data.m_type = value_t::null;
other.m_data.m_value = {};
other.m_data.m_type = value_t::null; // NOLINT(bugprone-use-after-move,hicpp-invalid-access-moved)
other.m_data.m_value = {};// NOLINT(bugprone-use-after-move,hicpp-invalid-access-moved)

set_parents();
assert_invariant();
Expand Down Expand Up @@ -21315,7 +21332,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
auto it = m_data.m_value.object->find(std::forward<KeyType>(key));
if (it == m_data.m_value.object->end())
{
JSON_THROW(out_of_range::create(403, detail::concat("key '", string_t(std::forward<KeyType>(key)), "' not found"), this));
JSON_THROW(out_of_range::create(403, "key not found (key is an rvalue and cannot be shown)", this));
}
return set_parent(it->second);
}
Expand Down Expand Up @@ -21353,7 +21370,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
auto it = m_data.m_value.object->find(std::forward<KeyType>(key));
if (it == m_data.m_value.object->end())
{
JSON_THROW(out_of_range::create(403, detail::concat("key '", string_t(std::forward<KeyType>(key)), "' not found"), this));
JSON_THROW(out_of_range::create(403, "key not found (key is an rvalue and cannot be shown)", this));
}
return it->second;
}
Expand Down
8 changes: 4 additions & 4 deletions tests/src/unit-conversions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1164,7 +1164,7 @@ TEST_CASE("value conversion")

SECTION("non-const")
{
const json j_const = j;
const json j_const = j; // NOLINT(performance-unnecessary-copy-initialization)
const auto& b = j_const.get_binary();
CHECK(*json(b).m_data.m_value.binary == *j.m_data.m_value.binary);
}
Expand Down Expand Up @@ -1515,9 +1515,9 @@ NLOHMANN_JSON_SERIALIZE_ENUM(cards,

enum TaskState
{
TS_STOPPED,
TS_RUNNING,
TS_COMPLETED,
TS_STOPPED = 0,
TS_RUNNING = 1,
TS_COMPLETED = 2,
TS_INVALID = -1,
};

Expand Down
5 changes: 5 additions & 0 deletions tests/src/unit-diagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ TEST_CASE("Better diagnostics")
{
json j;
j["object"]["object"] = true;

#if defined(JSON_HAS_CPP_17)
CHECK_THROWS_WITH_AS(j["object"].at("not_found"), "[json.exception.out_of_range.403] (/object) key not found (key is an rvalue and cannot be shown)", json::out_of_range);
#else
CHECK_THROWS_WITH_AS(j["object"].at("not_found"), "[json.exception.out_of_range.403] (/object) key 'not_found' not found", json::out_of_range);
#endif
}

SECTION("array index out of range")
Expand Down
9 changes: 7 additions & 2 deletions tests/src/unit-element_access2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,17 @@ TEST_CASE_TEMPLATE("element access 2", Json, nlohmann::json, nlohmann::ordered_j

SECTION("access outside bounds")
{
#if defined(JSON_HAS_CPP_17)
CHECK_THROWS_WITH_AS(j.at("foo"), "[json.exception.out_of_range.403] key not found (key is an rvalue and cannot be shown)", typename Json::out_of_range&);
CHECK_THROWS_WITH_AS(j_const.at("foo"), "[json.exception.out_of_range.403] key not found (key is an rvalue and cannot be shown)", typename Json::out_of_range&);
#else
CHECK_THROWS_WITH_AS(j.at("foo"), "[json.exception.out_of_range.403] key 'foo' not found", typename Json::out_of_range&);
CHECK_THROWS_WITH_AS(j_const.at("foo"), "[json.exception.out_of_range.403] key 'foo' not found", typename Json::out_of_range&);
#endif

#ifdef JSON_HAS_CPP_17
CHECK_THROWS_WITH_AS(j.at(std::string_view("foo")), "[json.exception.out_of_range.403] key 'foo' not found", typename Json::out_of_range&);
CHECK_THROWS_WITH_AS(j_const.at(std::string_view("foo")), "[json.exception.out_of_range.403] key 'foo' not found", typename Json::out_of_range&);
CHECK_THROWS_WITH_AS(j.at(std::string_view("foo")), "[json.exception.out_of_range.403] key not found (key is an rvalue and cannot be shown)", typename Json::out_of_range&);
CHECK_THROWS_WITH_AS(j_const.at(std::string_view("foo")), "[json.exception.out_of_range.403] key not found (key is an rvalue and cannot be shown)", typename Json::out_of_range&);
#endif
}

Expand Down
4 changes: 4 additions & 0 deletions tests/src/unit-udt_macro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,11 @@ TEST_CASE_TEMPLATE("Serialization/deserialization via NLOHMANN_DEFINE_TYPE_INTRU
// check exception in case of missing field
json j = json(p1);
j.erase("age");
#if defined(JSON_HAS_CPP_17)
CHECK_THROWS_WITH_AS(j.get<T>(), "[json.exception.out_of_range.403] key not found (key is an rvalue and cannot be shown)", json::out_of_range);
#else
CHECK_THROWS_WITH_AS(j.get<T>(), "[json.exception.out_of_range.403] key 'age' not found", json::out_of_range);
#endif
}
}

Expand Down
Loading