amorfo77/json
1
0
Fork 0
mirror of https://github.com/nlohmann/json.git synced 2026-10-01 04:00:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

Re-enable portability-template-virtual-member-function; remove redundant forwards

Browse Source 
.clang-tidy disabled three checks "to get the CI going" (#4489,
2024-11-13): portability-template-virtual-member-function,
bugprone-use-after-move and its alias hicpp-invalid-access-moved.

portability-template-virtual-member-function only flagged
output_stream_adapter::write_character/write_characters; annotate
both with NOLINT and re-enable the check.

bugprone-use-after-move flagged several double forwards that have no
effect at runtime:
- from_json.hpp calls std::forward<BasicJsonType>(j).at(Idx) inside
  pack expansions; at() has no ref-qualified overloads and always
  returns an lvalue reference, so the forward is a no-op. Replace with
  plain j.at(Idx) in all four places.
- the move constructor forwards the whole object to its base class
  and then reads other's members. That is item 9 of #5724 (together
  with its cppcheck suppressions) and is left to that change.
- input_adapters.hpp forwards the container twice on purpose, so the
  begin/end iterator types match adapter_type; annotate with NOLINT
  and a comment instead of changing behavior.

The check still flags the move constructor (see above) and two sites
in at(KeyType&&) (both overloads, json.hpp, in the throw's
string_t(std::forward<KeyType>(key)) after
find(std::forward<KeyType>(key))). Open PR #5689 rewrites that hunk,
so bugprone-use-after-move (and hicpp-invalid-access-moved)
stay disabled for now, with a comment explaining why; re-enable them
once #5689 and the #5724 move-constructor change have landed.

Also resolve the portability-avoid-pragma-once TODO: single_include
never has #pragma once (amalgamate.py strips it) and every supported
compiler accepts it in include/, so keep it disabled with an
explanatory comment instead of a TODO. Fix the stale "json.hpp,
around line 1265" comment in unit-class_parser.cpp, which now points
at the move constructor's actual line.

Behavior, the public API and the ABI do not change. Verified with
clang-tidy 22.1.8 that portability-template-virtual-member-function
now reports nothing, that bugprone-use-after-move/
hicpp-invalid-access-moved report only the known at(KeyType&&) and
move-constructor sites, and that unit-custom-base-class, unit-constructor1,
unit-conversions, unit-element_access2, unit-class_parser and
unit-diagnostic-positions (JSON_DIAGNOSTIC_POSITIONS=1) compile
under ASan/UBSan and pass with the same assertion counts as before.
Ran make amalgamate.

Part of #5725

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This commit is contained in:
Niels Lohmann
2026-09-30 10:33:04 +02:00
parent 27db8bd678
commit b640bf728d
6 changed files with 28 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.clang-tidy
+9 -3
View File
@@ -1,9 +1,15 @@
# TODO: The first three checks are only removed to get the CI going. They have to be addressed at some point.
# TODO: portability-avoid-pragma-once: should be fixed eventually
# bugprone-use-after-move (hicpp-invalid-access-moved is its alias) still flags
# the basic_json move constructor, which forwards the whole object to its base
# class (#5724), and two forwards in the error-message construction of
# at(KeyType&&) (json.hpp, both overloads: find(std::forward<KeyType>(key))
# followed by string_t(std::forward<KeyType>(key)) in the throw), which #5689
# rewrites. Re-enable both checks once those changes have landed.
# portability-avoid-pragma-once: kept disabled on purpose. #pragma once is accepted
# by every supported compiler, and tools/amalgamate/amalgamate.py strips it from
# single_include, so there is nothing left to fix here.
Checks: '*,
-portability-template-virtual-member-function,
-bugprone-use-after-move,
-hicpp-invalid-access-moved,
include/nlohmann/detail/conversions/from_json.hpp
+4 -4
View File
@@ -353,7 +353,7 @@ 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 { { j.at(Idx).template get<T>()... } };
}
template < typename BasicJsonType, typename T, std::size_t N >
@@ -502,7 +502,7 @@ using tuple_type = std::tuple < decltype(from_json_tuple_get_impl(std::declval<B
template<std::size_t PTagValue, typename... Args, typename BasicJsonType, std::size_t... Idx>
tuple_type<PTagValue, BasicJsonType, Args...> from_json_tuple_impl_base(BasicJsonType&& j, index_sequence<Idx...> /*unused*/)
{
return tuple_type<PTagValue, BasicJsonType, Args...>(from_json_tuple_get_impl(std::forward<BasicJsonType>(j).at(Idx), detail::identity_tag<Args> {}, detail::priority_tag<PTagValue> {})...);
return tuple_type<PTagValue, BasicJsonType, Args...>(from_json_tuple_get_impl(j.at(Idx), detail::identity_tag<Args> {}, detail::priority_tag<PTagValue> {})...);
}
template<std::size_t PTagValue, typename BasicJsonType>
@@ -514,8 +514,8 @@ std::tuple<> from_json_tuple_impl_base(BasicJsonType& /*unused*/, 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 {j.at(0).template get<A1>(),
j.at(1).template get<A2>()};
}
template<typename BasicJsonType, typename A1, typename A2>
include/nlohmann/detail/input/input_adapters.hpp
+3
View File
@@ -763,6 +763,9 @@ struct container_input_adapter_factory< ContainerType,
static adapter_type create(ContainerType&& container)
{
// container is forwarded twice on purpose: the resulting begin/end
// iterator types must match adapter_type, computed the same way
// NOLINTNEXTLINE(bugprone-use-after-move)
return input_adapter(begin(std::forward<ContainerType>(container)), end(std::forward<ContainerType>(container)));
}
};
include/nlohmann/detail/output/output_adapters.hpp
+2
View File
@@ -117,12 +117,14 @@ class output_stream_adapter : public output_adapter_protocol<CharType>
: stream(s)
{}
// NOLINTNEXTLINE(portability-template-virtual-member-function)
void write_character(CharType c) override
{
stream.put(c);
}
JSON_HEDLEY_NON_NULL(2)
// NOLINTNEXTLINE(portability-template-virtual-member-function)
void write_characters(const CharType* s, std::size_t length) override
{
stream.write(s, static_cast<std::streamsize>(length));
single_include/nlohmann/json.hpp
+9 -4
View File
@@ -5850,7 +5850,7 @@ 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 { { j.at(Idx).template get<T>()... } };
}
template < typename BasicJsonType, typename T, std::size_t N >
@@ -5999,7 +5999,7 @@ using tuple_type = std::tuple < decltype(from_json_tuple_get_impl(std::declval<B
template<std::size_t PTagValue, typename... Args, typename BasicJsonType, std::size_t... Idx>
tuple_type<PTagValue, BasicJsonType, Args...> from_json_tuple_impl_base(BasicJsonType&& j, index_sequence<Idx...> /*unused*/)
{
return tuple_type<PTagValue, BasicJsonType, Args...>(from_json_tuple_get_impl(std::forward<BasicJsonType>(j).at(Idx), detail::identity_tag<Args> {}, detail::priority_tag<PTagValue> {})...);
return tuple_type<PTagValue, BasicJsonType, Args...>(from_json_tuple_get_impl(j.at(Idx), detail::identity_tag<Args> {}, detail::priority_tag<PTagValue> {})...);
}
template<std::size_t PTagValue, typename BasicJsonType>
@@ -6011,8 +6011,8 @@ std::tuple<> from_json_tuple_impl_base(BasicJsonType& /*unused*/, 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 {j.at(0).template get<A1>(),
j.at(1).template get<A2>()};
}
template<typename BasicJsonType, typename A1, typename A2>
@@ -8307,6 +8307,9 @@ struct container_input_adapter_factory< ContainerType,
static adapter_type create(ContainerType&& container)
{
// container is forwarded twice on purpose: the resulting begin/end
// iterator types must match adapter_type, computed the same way
// NOLINTNEXTLINE(bugprone-use-after-move)
return input_adapter(begin(std::forward<ContainerType>(container)), end(std::forward<ContainerType>(container)));
}
};
@@ -20256,12 +20259,14 @@ class output_stream_adapter : public output_adapter_protocol<CharType>
: stream(s)
{}
// NOLINTNEXTLINE(portability-template-virtual-member-function)
void write_character(CharType c) override
{
stream.put(c);
}
JSON_HEDLEY_NON_NULL(2)
// NOLINTNEXTLINE(portability-template-virtual-member-function)
void write_characters(const CharType* s, std::size_t length) override
{
stream.write(s, static_cast<std::streamsize>(length));
tests/src/unit-class_parser.cpp
+1 -1
View File
@@ -2525,7 +2525,7 @@ TEST_CASE("diagnostic positions: value lifetime, input adapters, and SAX")
SECTION("move constructor resets the moved-from value to npos")
{
// basic_json(basic_json&&) (json.hpp, around line 1265) copies
// basic_json(basic_json&&) (json.hpp, around line 1951) copies
// other's start_position/end_position into *this and then resets
// other's to npos (see the cppcheck-suppress[accessForwarded]
// annotation there, which flags this reset as worth a second