From 09d41a894b4eee46e49c0ab2711efed63d535187 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 30 Sep 2026 18:40:36 +0200 Subject: [PATCH] Re-enable bugprone-use-after-move/hicpp-invalid-access-moved These two checks (and portability-template-virtual-member-function) were disabled in #4489 (November 2024) "only removed to get the CI going". portability-template-virtual-member-function is a separate, still-open cleanup (#5725 item 3 on its own branch) and stays disabled here; this commit only re-enables the move/forward checks and cleans up what they flag on this branch. The move constructor (json.hpp) already casts to the base type instead of forwarding the whole object (#5724 item 9), so it no longer trips either check. The at(KeyType&&) double-forward this check used to flag was reduced to a single forward with the now-unforwarded reuse annotated by a NOLINTNEXTLINE in #5724 item 3's object_at() helper (clang-tidy 22 still flags that reuse even after a single forward; see that commit's message). What is left here: - from_json_inplace_array_impl(), from_json_tuple_impl_base() and the std::pair overload of from_json_tuple_impl() forwarded j into every j.at(...) call in a pack expansion or a pair of calls. at() has no ref-qualified overloads, so the forward was a no-op; call j.at(...) directly. - container_input_adapter_factory::create() forwards container twice on purpose, into begin() and end(), so both see the same value category and produce matching iterator types. Annotate it with NOLINTNEXTLINE and a comment instead of changing it. - unit-class_parser.cpp's "move constructor resets the moved-from value to npos" test still pointed at the pre-static_cast move constructor by line number and mentioned the cppcheck-suppress annotation that #5724 item 9 already removed; update the comment. No behavior change anywhere in include/. Verified with clang-tidy 22.1.8 (Docker silkeh/clang:22, --platform linux/amd64) against a TU including json.hpp with the repo's .clang-tidy: bugprone-use-after-move and hicpp-invalid-access-moved report nothing unsuppressed. Overlaps #5737 (open PR for the rest of #5725 item 3: the from_json.hpp/input_adapters.hpp cleanup above, and portability-template-virtual-member-function), which currently keeps both checks disabled pending this move-constructor change; whichever of this commit and that PR lands second will need a small rebase of .clang-tidy. Signed-off-by: Niels Lohmann --- .clang-tidy | 4 +--- include/nlohmann/detail/conversions/from_json.hpp | 8 ++++---- include/nlohmann/detail/input/input_adapters.hpp | 1 + single_include/nlohmann/json.hpp | 9 +++++---- tests/src/unit-class_parser.cpp | 8 +++----- 5 files changed, 14 insertions(+), 16 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 29b3e9302..7823f77ca 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,11 +1,9 @@ -# TODO: The first three checks are only removed to get the CI going. They have to be addressed at some point. +# TODO: portability-template-virtual-member-function is only removed to get the CI going. It has to be addressed at some point. # TODO: portability-avoid-pragma-once: should be fixed eventually Checks: '*, -portability-template-virtual-member-function, - -bugprone-use-after-move, - -hicpp-invalid-access-moved, -altera-id-dependent-backward-branch, -altera-struct-pack-align, diff --git a/include/nlohmann/detail/conversions/from_json.hpp b/include/nlohmann/detail/conversions/from_json.hpp index 11e40f5f4..fdbd9df18 100644 --- a/include/nlohmann/detail/conversions/from_json.hpp +++ b/include/nlohmann/detail/conversions/from_json.hpp @@ -353,7 +353,7 @@ template < typename BasicJsonType, typename T, std::size_t... Idx > std::array from_json_inplace_array_impl(BasicJsonType&& j, identity_tag> /*unused*/, index_sequence /*unused*/) { - return { { std::forward(j).at(Idx).template get()... } }; + return { { j.at(Idx).template get()... } }; } 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 tuple_type from_json_tuple_impl_base(BasicJsonType&& j, index_sequence /*unused*/) { - return tuple_type(from_json_tuple_get_impl(std::forward(j).at(Idx), detail::identity_tag {}, detail::priority_tag {})...); + return tuple_type(from_json_tuple_get_impl(j.at(Idx), detail::identity_tag {}, detail::priority_tag {})...); } template @@ -514,8 +514,8 @@ std::tuple<> from_json_tuple_impl_base(BasicJsonType& /*unused*/, index_sequence template < typename BasicJsonType, class A1, class A2 > std::pair from_json_tuple_impl(BasicJsonType&& j, identity_tag> /*unused*/, priority_tag<0> /*unused*/) { - return {std::forward(j).at(0).template get(), - std::forward(j).at(1).template get()}; + return {j.at(0).template get(), + j.at(1).template get()}; } template diff --git a/include/nlohmann/detail/input/input_adapters.hpp b/include/nlohmann/detail/input/input_adapters.hpp index e174775c5..0005d6ed9 100644 --- a/include/nlohmann/detail/input/input_adapters.hpp +++ b/include/nlohmann/detail/input/input_adapters.hpp @@ -763,6 +763,7 @@ struct container_input_adapter_factory< ContainerType, static adapter_type create(ContainerType&& container) { + // NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) forwarded twice on purpose, so begin() and end() see the same value category and yield matching iterator types return input_adapter(begin(std::forward(container)), end(std::forward(container))); } }; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index f358de29a..288163777 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -5849,7 +5849,7 @@ template < typename BasicJsonType, typename T, std::size_t... Idx > std::array from_json_inplace_array_impl(BasicJsonType&& j, identity_tag> /*unused*/, index_sequence /*unused*/) { - return { { std::forward(j).at(Idx).template get()... } }; + return { { j.at(Idx).template get()... } }; } template < typename BasicJsonType, typename T, std::size_t N > @@ -5998,7 +5998,7 @@ using tuple_type = std::tuple < decltype(from_json_tuple_get_impl(std::declval tuple_type from_json_tuple_impl_base(BasicJsonType&& j, index_sequence /*unused*/) { - return tuple_type(from_json_tuple_get_impl(std::forward(j).at(Idx), detail::identity_tag {}, detail::priority_tag {})...); + return tuple_type(from_json_tuple_get_impl(j.at(Idx), detail::identity_tag {}, detail::priority_tag {})...); } template @@ -6010,8 +6010,8 @@ std::tuple<> from_json_tuple_impl_base(BasicJsonType& /*unused*/, index_sequence template < typename BasicJsonType, class A1, class A2 > std::pair from_json_tuple_impl(BasicJsonType&& j, identity_tag> /*unused*/, priority_tag<0> /*unused*/) { - return {std::forward(j).at(0).template get(), - std::forward(j).at(1).template get()}; + return {j.at(0).template get(), + j.at(1).template get()}; } template @@ -8306,6 +8306,7 @@ struct container_input_adapter_factory< ContainerType, static adapter_type create(ContainerType&& container) { + // NOLINTNEXTLINE(bugprone-use-after-move,hicpp-invalid-access-moved) forwarded twice on purpose, so begin() and end() see the same value category and yield matching iterator types return input_adapter(begin(std::forward(container)), end(std::forward(container))); } }; diff --git a/tests/src/unit-class_parser.cpp b/tests/src/unit-class_parser.cpp index 3d825162e..e498df0ae 100644 --- a/tests/src/unit-class_parser.cpp +++ b/tests/src/unit-class_parser.cpp @@ -2525,12 +2525,10 @@ 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 1944) 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 - // look). Only the top-level moved-from value is affected; its - // (moved-away) children are gone along with it. + // other's to npos. Only the top-level moved-from value is + // affected; its (moved-away) children are gone along with it. const std::string s = R"({"a":1,"b":[1,2,3]})"; json a = json::parse(s); const auto a_start = a.start_pos();