From baf4ccfda8c293f077cb401f0a0b2db5c2ad8202 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Tue, 29 Sep 2026 01:02:27 +0200 Subject: [PATCH] Address the clang-tidy findings of the editable documents Pick the overloads of encode() with a first_true trait instead of nested conditionals, name the pointer type in the copies of links, mark the owning pointers of the edit storage, and compare doubles by their bits in the tests. Signed-off-by: Niels Lohmann --- .../nlohmann/detail/view/document_data.hpp | 12 +++--- include/nlohmann/detail/view/edit.hpp | 21 ++++++--- include/nlohmann/detail/view/edit_storage.hpp | 6 +-- include/nlohmann/detail/view/node.hpp | 4 +- single_include/nlohmann/json_view.hpp | 43 +++++++++++-------- tests/src/unit-json_view_edit.cpp | 26 ++++++----- 6 files changed, 66 insertions(+), 46 deletions(-) diff --git a/include/nlohmann/detail/view/document_data.hpp b/include/nlohmann/detail/view/document_data.hpp index 4b43953a6..dc1a8adf9 100644 --- a/include/nlohmann/detail/view/document_data.hpp +++ b/include/nlohmann/detail/view/document_data.hpp @@ -63,19 +63,19 @@ struct document_data /// entries), whose entries link to the values. struct edit_state { - std::vector moved{}; ///< element sequences of moved arrays/objects (header node first) - std::vector moved_cap{}; ///< capacity in nodes of a growable block; 0: a fixed sequence (a new value) - std::vector> chunks{}; ///< storage of new values and blocks; never moved - std::map> regions{}; ///< new arrays/objects: root -> container that uses it as its element sequence (nullptr: linked from a block) + std::vector moved{}; ///< element sequences of moved arrays/objects (header node first) // NOLINT(readability-redundant-member-init) + std::vector moved_cap{}; ///< capacity in nodes of a growable block; 0: a fixed sequence (a new value) // NOLINT(readability-redundant-member-init) + std::vector> chunks{}; ///< storage of new values and blocks; never moved // NOLINT(readability-redundant-member-init,cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) + std::map> regions{}; ///< new arrays/objects: root -> container that uses it as its element sequence (nullptr: linked from a block) // NOLINT(readability-redundant-member-init) node* chunk_cur = nullptr; node* chunk_end = nullptr; std::size_t chunk_next = 64; - std::vector> texts{}; ///< edit arena, the current buffer last; earlier ones stay alive for string views + std::vector> texts{}; ///< edit arena, the current buffer last; earlier ones stay alive for string views // NOLINT(readability-redundant-member-init,cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) std::size_t text_used = 0; std::size_t text_cap = 0; std::size_t bytes = 0; ///< memory held by edits }; - std::unique_ptr edits{}; ///< created by the first edit + std::unique_ptr edits{}; ///< created by the first edit // NOLINT(readability-redundant-member-init) /// one allocation for the header and room for `nodes` nodes; large /// documents get a separate node array instead (so it can be trimmed) diff --git a/include/nlohmann/detail/view/edit.hpp b/include/nlohmann/detail/view/edit.hpp index 1c0e272e4..6b22d986d 100644 --- a/include/nlohmann/detail/view/edit.hpp +++ b/include/nlohmann/detail/view/edit.hpp @@ -36,6 +36,13 @@ namespace detail namespace view { +/// the index of the first true condition (the number of conditions if none is) +template +struct first_true : std::integral_constant {}; + +template +struct first_true : std::integral_constant < int, 1 + first_true::value > {}; + /// Checks a string the way basic_json's serializer does when it writes it /// (type_error.316 with the same message), so that an editable document /// only holds valid UTF-8: the error is at the first byte that no @@ -354,12 +361,12 @@ class editor encoded encode(V&& v) { using D = typename std::decay::type; - return encode_impl(std::forward(v), encode_tag < is_view::value ? 0 - : std::is_same::value ? 1 - : std::is_same::value ? 2 - : std::is_same::value ? 3 - : std::is_arithmetic::value ? 4 - : std::is_convertible::value ? 5 : 6 > {}); + return encode_impl(std::forward(v), encode_tag::value, + std::is_same::value, + std::is_same::value, + std::is_same::value, + std::is_arithmetic::value, + std::is_convertible::value>::value> {}); } /// a view of any document (copied; nothing is shared with it) @@ -415,7 +422,7 @@ class editor encoded encode_impl(T x, encode_tag<4> /*number*/) { encoded r; - r.scalar = number_node(x, std::integral_constant < int, std::is_floating_point::value ? 0 : (std::is_signed::value ? 1 : 2) > {}); + r.scalar = number_node(x, std::integral_constant::value, std::is_signed::value>::value> {}); return r; } diff --git a/include/nlohmann/detail/view/edit_storage.hpp b/include/nlohmann/detail/view/edit_storage.hpp index f166980ee..8e0ff8e37 100644 --- a/include/nlohmann/detail/view/edit_storage.hpp +++ b/include/nlohmann/detail/view/edit_storage.hpp @@ -39,7 +39,7 @@ inline document_data::edit_state& edit_state_of(document_data& d) { if (!d.edits) { - d.edits.reset(new document_data::edit_state()); + d.edits.reset(new document_data::edit_state()); // NOLINT(cppcoreguidelines-owning-memory): owned by the unique_ptr } return *d.edits; } @@ -51,7 +51,7 @@ inline node* alloc_nodes(document_data& d, std::size_t k) if (NLOHMANN_VIEW_UNLIKELY(static_cast(e.chunk_end - e.chunk_cur) < k)) { const std::size_t count = (std::max)(k, e.chunk_next); - std::unique_ptr fresh(new node[count]()); + std::unique_ptr fresh(new node[count]()); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) e.chunks.push_back(std::move(fresh)); e.chunk_cur = e.chunks.back().get(); e.chunk_end = e.chunk_cur + count; @@ -75,7 +75,7 @@ inline std::uint32_t append_text(document_data& d, const char* s, std::size_t n) { throw_out_of_range(416, "edits of 4 GiB or more are not supported by json_document"); } - std::unique_ptr fresh(new char[cap]); + std::unique_ptr fresh(new char[cap]); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) if (e.text_used != 0) { std::memcpy(fresh.get(), e.texts.back().get(), e.text_used); diff --git a/include/nlohmann/detail/view/node.hpp b/include/nlohmann/detail/view/node.hpp index 33be4f001..82774dac8 100644 --- a/include/nlohmann/detail/view/node.hpp +++ b/include/nlohmann/detail/view/node.hpp @@ -68,7 +68,7 @@ NLOHMANN_VIEW_ALWAYS_INLINE bool is_container(const node& n) noexcept NLOHMANN_VIEW_ALWAYS_INLINE const node* link_target(const node& n) noexcept { const node* t = nullptr; - std::memcpy(&t, reinterpret_cast(&n) + 8, sizeof(t)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + std::memcpy(static_cast(&t), reinterpret_cast(&n) + 8, sizeof(const node*)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) return t; } @@ -76,7 +76,7 @@ inline void make_link(node& n, const node* target) noexcept { n = node{}; n.kind = kind_link; - std::memcpy(reinterpret_cast(&n) + 8, &target, sizeof(target)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + std::memcpy(reinterpret_cast(&n) + 8, static_cast(&target), sizeof(const node*)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) } /// the converted value of an integer node (stored in len/next) diff --git a/single_include/nlohmann/json_view.hpp b/single_include/nlohmann/json_view.hpp index d58356189..53e15aac4 100644 --- a/single_include/nlohmann/json_view.hpp +++ b/single_include/nlohmann/json_view.hpp @@ -237,7 +237,7 @@ NLOHMANN_VIEW_ALWAYS_INLINE bool is_container(const node& n) noexcept NLOHMANN_VIEW_ALWAYS_INLINE const node* link_target(const node& n) noexcept { const node* t = nullptr; - std::memcpy(&t, reinterpret_cast(&n) + 8, sizeof(t)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + std::memcpy(static_cast(&t), reinterpret_cast(&n) + 8, sizeof(const node*)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) return t; } @@ -245,7 +245,7 @@ inline void make_link(node& n, const node* target) noexcept { n = node{}; n.kind = kind_link; - std::memcpy(reinterpret_cast(&n) + 8, &target, sizeof(target)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + std::memcpy(reinterpret_cast(&n) + 8, static_cast(&target), sizeof(const node*)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) } /// the converted value of an integer node (stored in len/next) @@ -328,19 +328,19 @@ struct document_data /// entries), whose entries link to the values. struct edit_state { - std::vector moved{}; ///< element sequences of moved arrays/objects (header node first) - std::vector moved_cap{}; ///< capacity in nodes of a growable block; 0: a fixed sequence (a new value) - std::vector> chunks{}; ///< storage of new values and blocks; never moved - std::map> regions{}; ///< new arrays/objects: root -> container that uses it as its element sequence (nullptr: linked from a block) + std::vector moved{}; ///< element sequences of moved arrays/objects (header node first) // NOLINT(readability-redundant-member-init) + std::vector moved_cap{}; ///< capacity in nodes of a growable block; 0: a fixed sequence (a new value) // NOLINT(readability-redundant-member-init) + std::vector> chunks{}; ///< storage of new values and blocks; never moved // NOLINT(readability-redundant-member-init,cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) + std::map> regions{}; ///< new arrays/objects: root -> container that uses it as its element sequence (nullptr: linked from a block) // NOLINT(readability-redundant-member-init) node* chunk_cur = nullptr; node* chunk_end = nullptr; std::size_t chunk_next = 64; - std::vector> texts{}; ///< edit arena, the current buffer last; earlier ones stay alive for string views + std::vector> texts{}; ///< edit arena, the current buffer last; earlier ones stay alive for string views // NOLINT(readability-redundant-member-init,cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) std::size_t text_used = 0; std::size_t text_cap = 0; std::size_t bytes = 0; ///< memory held by edits }; - std::unique_ptr edits{}; ///< created by the first edit + std::unique_ptr edits{}; ///< created by the first edit // NOLINT(readability-redundant-member-init) /// one allocation for the header and room for `nodes` nodes; large /// documents get a separate node array instead (so it can be trimmed) @@ -2512,7 +2512,7 @@ inline document_data::edit_state& edit_state_of(document_data& d) { if (!d.edits) { - d.edits.reset(new document_data::edit_state()); + d.edits.reset(new document_data::edit_state()); // NOLINT(cppcoreguidelines-owning-memory): owned by the unique_ptr } return *d.edits; } @@ -2524,7 +2524,7 @@ inline node* alloc_nodes(document_data& d, std::size_t k) if (NLOHMANN_VIEW_UNLIKELY(static_cast(e.chunk_end - e.chunk_cur) < k)) { const std::size_t count = (std::max)(k, e.chunk_next); - std::unique_ptr fresh(new node[count]()); + std::unique_ptr fresh(new node[count]()); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) e.chunks.push_back(std::move(fresh)); e.chunk_cur = e.chunks.back().get(); e.chunk_end = e.chunk_cur + count; @@ -2548,7 +2548,7 @@ inline std::uint32_t append_text(document_data& d, const char* s, std::size_t n) { throw_out_of_range(416, "edits of 4 GiB or more are not supported by json_document"); } - std::unique_ptr fresh(new char[cap]); + std::unique_ptr fresh(new char[cap]); // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) if (e.text_used != 0) { std::memcpy(fresh.get(), e.texts.back().get(), e.text_used); @@ -3029,6 +3029,13 @@ namespace detail namespace view { +/// the index of the first true condition (the number of conditions if none is) +template +struct first_true : std::integral_constant {}; + +template +struct first_true : std::integral_constant < int, 1 + first_true::value > {}; + /// Checks a string the way basic_json's serializer does when it writes it /// (type_error.316 with the same message), so that an editable document /// only holds valid UTF-8: the error is at the first byte that no @@ -3347,12 +3354,12 @@ class editor encoded encode(V&& v) { using D = typename std::decay::type; - return encode_impl(std::forward(v), encode_tag < is_view::value ? 0 - : std::is_same::value ? 1 - : std::is_same::value ? 2 - : std::is_same::value ? 3 - : std::is_arithmetic::value ? 4 - : std::is_convertible::value ? 5 : 6 > {}); + return encode_impl(std::forward(v), encode_tag::value, + std::is_same::value, + std::is_same::value, + std::is_same::value, + std::is_arithmetic::value, + std::is_convertible::value>::value> {}); } /// a view of any document (copied; nothing is shared with it) @@ -3408,7 +3415,7 @@ class editor encoded encode_impl(T x, encode_tag<4> /*number*/) { encoded r; - r.scalar = number_node(x, std::integral_constant < int, std::is_floating_point::value ? 0 : (std::is_signed::value ? 1 : 2) > {}); + r.scalar = number_node(x, std::integral_constant::value, std::is_signed::value>::value> {}); return r; } diff --git a/tests/src/unit-json_view_edit.cpp b/tests/src/unit-json_view_edit.cpp index 3a59e3ec9..6d89d937c 100644 --- a/tests/src/unit-json_view_edit.cpp +++ b/tests/src/unit-json_view_edit.cpp @@ -19,6 +19,7 @@ using nlohmann::ordered_json_editable_document; using nlohmann::ordered_json_editable_view; using ptr_t = ordered_json::json_pointer; +#include #include #include #include @@ -33,7 +34,7 @@ namespace { std::uint32_t rng() { - static std::mt19937 generator(5295); + static std::mt19937 generator(5295); // NOLINT(cert-msc32-c,cert-msc51-cpp,bugprone-random-generator-seed): reproducible return generator(); } @@ -44,13 +45,20 @@ int r(int n) int counter = 0; +std::uint64_t bits(double x) +{ + std::uint64_t b = 0; + std::memcpy(&b, &x, sizeof(b)); + return b; +} + std::string random_string() { - static const char* const pieces[] = {"a", "Z", " ", "~", "\n", "\"", "\\", "/", "\xc3\xa9", "\xe3\x81\x82", "\xf0\x9f\x98\x80", "\x7f", "\x1f", "0", "key"}; + static const std::array pieces = {{"a", "Z", " ", "~", "\n", "\"", "\\", "/", "\xc3\xa9", "\xe3\x81\x82", "\xf0\x9f\x98\x80", "\x7f", "\x1f", "0", "key"}}; std::string s; for (int i = r(3) == 0 ? r(30) : r(6); i > 0; --i) { - s += pieces[r(15)]; + s += pieces[static_cast(r(15))]; } return s; } @@ -66,7 +74,7 @@ ordered_json random_scalar() case 2: return static_cast(rng()) - 2147483648LL; case 3: - return static_cast(rng()) * 4294967296ULL + rng(); + return (static_cast(rng()) * 4294967296ULL) + rng(); case 4: return static_cast(static_cast(rng())) / (1 + r(1000)); case 5: @@ -161,9 +169,7 @@ void compare(const ordered_json_editable_view& v, const ordered_json& j) } else if (j.is_number_float()) { - const double a = v.get(); - const double b = j.get(); - CHECK(std::memcmp(&a, &b, sizeof(double)) == 0); + CHECK(bits(v.get()) == bits(j.get())); } else if (j.is_number_integer()) { @@ -249,7 +255,7 @@ TEST_CASE("json_view edits: differential") } else if (op == 2) // copy a value of the same document { - const ptr_t q = paths[static_cast(r(static_cast(paths.size())))]; + const ptr_t& q = paths[static_cast(r(static_cast(paths.size())))]; const ordered_json v = j[q]; d.set(tv, d.root().at(q)); j[p] = v; @@ -278,7 +284,7 @@ TEST_CASE("json_view edits: differential") } else if (op == 10 && target.is_array() && !target.empty()) // assign an element { - const std::size_t i = static_cast(r(static_cast(target.size()))); + const auto i = static_cast(r(static_cast(target.size()))); const ordered_json v = random_value(2); d.set(tv, i, v); j[p][i] = v; @@ -436,7 +442,7 @@ TEST_CASE("json_view edits: views and values") { text += (i != 0 ? ",\"k" : "\"k") + std::to_string(i) + "\":" + std::to_string(i); } - text += "}"; + text += '}'; json_editable_document d = json_editable_document::parse(text); d.set(d.root(), "k7", "seven"); // assigned in place: the index stays in use CHECK(d.root()["k7"].get_string() == "seven");