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 <mail@nlohmann.me>

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<node*> moved{};                    ///< element sequences of moved arrays/objects (header node first)
-        std::vector<std::size_t> moved_cap{};          ///< capacity in nodes of a growable block; 0: a fixed sequence (a new value)
-        std::vector<std::unique_ptr<node[]>> chunks{}; ///< storage of new values and blocks; never moved
-        std::map<const node*, node*, std::less<const node*>> regions{}; ///< new arrays/objects: root -> container that uses it as its element sequence (nullptr: linked from a block)
+        std::vector<node*> moved{};                    ///< element sequences of moved arrays/objects (header node first) // NOLINT(readability-redundant-member-init)
+        std::vector<std::size_t> moved_cap{};          ///< capacity in nodes of a growable block; 0: a fixed sequence (a new value) // NOLINT(readability-redundant-member-init)
+        std::vector<std::unique_ptr<node[]>> 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<const node*, node*, std::less<const node*>> 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<std::unique_ptr<char[]>> texts{}; ///< edit arena, the current buffer last; earlier ones stay alive for string views
+        std::vector<std::unique_ptr<char[]>> 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<edit_state> edits{}; ///< created by the first edit
+    std::unique_ptr<edit_state> 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)

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<bool... Conditions>
+struct first_true : std::integral_constant<int, 0> {};
+
+template<bool... Conditions>
+struct first_true<false, Conditions...> : std::integral_constant < int, 1 + first_true<Conditions...>::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<V>::type;
-        return encode_impl(std::forward<V>(v), encode_tag < is_view<D>::value ? 0
-                           : std::is_same<D, BasicJsonType>::value ? 1
-                           : std::is_same<D, std::nullptr_t>::value ? 2
-                           : std::is_same<D, bool>::value ? 3
-                           : std::is_arithmetic<D>::value ? 4
-                           : std::is_convertible<const D&, string_view_t>::value ? 5 : 6 > {});
+        return encode_impl(std::forward<V>(v), encode_tag<first_true<is_view<D>::value,
+                           std::is_same<D, BasicJsonType>::value,
+                           std::is_same<D, std::nullptr_t>::value,
+                           std::is_same<D, bool>::value,
+                           std::is_arithmetic<D>::value,
+                           std::is_convertible<const D&, string_view_t>::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<T>::value ? 0 : (std::is_signed<T>::value ? 1 : 2) > {});
+        r.scalar = number_node(x, std::integral_constant<int, first_true<std::is_floating_point<T>::value, std::is_signed<T>::value>::value> {});
         return r;
     }
 

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<std::size_t>(e.chunk_end - e.chunk_cur) < k))
     {
         const std::size_t count = (std::max)(k, e.chunk_next);
-        std::unique_ptr<node[]> fresh(new node[count]());
+        std::unique_ptr<node[]> 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<char[]> fresh(new char[cap]);
+        std::unique_ptr<char[]> 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);

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<const unsigned char*>(&n) + 8, sizeof(t)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
+    std::memcpy(static_cast<void*>(&t), reinterpret_cast<const unsigned char*>(&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<unsigned char*>(&n) + 8, &target, sizeof(target)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
+    std::memcpy(reinterpret_cast<unsigned char*>(&n) + 8, static_cast<const void*>(&target), sizeof(const node*)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
 }
 
 /// the converted value of an integer node (stored in len/next)

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<const unsigned char*>(&n) + 8, sizeof(t)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
+    std::memcpy(static_cast<void*>(&t), reinterpret_cast<const unsigned char*>(&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<unsigned char*>(&n) + 8, &target, sizeof(target)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
+    std::memcpy(reinterpret_cast<unsigned char*>(&n) + 8, static_cast<const void*>(&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<node*> moved{};                    ///< element sequences of moved arrays/objects (header node first)
-        std::vector<std::size_t> moved_cap{};          ///< capacity in nodes of a growable block; 0: a fixed sequence (a new value)
-        std::vector<std::unique_ptr<node[]>> chunks{}; ///< storage of new values and blocks; never moved
-        std::map<const node*, node*, std::less<const node*>> regions{}; ///< new arrays/objects: root -> container that uses it as its element sequence (nullptr: linked from a block)
+        std::vector<node*> moved{};                    ///< element sequences of moved arrays/objects (header node first) // NOLINT(readability-redundant-member-init)
+        std::vector<std::size_t> moved_cap{};          ///< capacity in nodes of a growable block; 0: a fixed sequence (a new value) // NOLINT(readability-redundant-member-init)
+        std::vector<std::unique_ptr<node[]>> 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<const node*, node*, std::less<const node*>> 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<std::unique_ptr<char[]>> texts{}; ///< edit arena, the current buffer last; earlier ones stay alive for string views
+        std::vector<std::unique_ptr<char[]>> 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<edit_state> edits{}; ///< created by the first edit
+    std::unique_ptr<edit_state> 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<std::size_t>(e.chunk_end - e.chunk_cur) < k))
     {
         const std::size_t count = (std::max)(k, e.chunk_next);
-        std::unique_ptr<node[]> fresh(new node[count]());
+        std::unique_ptr<node[]> 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<char[]> fresh(new char[cap]);
+        std::unique_ptr<char[]> 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<bool... Conditions>
+struct first_true : std::integral_constant<int, 0> {};
+
+template<bool... Conditions>
+struct first_true<false, Conditions...> : std::integral_constant < int, 1 + first_true<Conditions...>::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<V>::type;
-        return encode_impl(std::forward<V>(v), encode_tag < is_view<D>::value ? 0
-                           : std::is_same<D, BasicJsonType>::value ? 1
-                           : std::is_same<D, std::nullptr_t>::value ? 2
-                           : std::is_same<D, bool>::value ? 3
-                           : std::is_arithmetic<D>::value ? 4
-                           : std::is_convertible<const D&, string_view_t>::value ? 5 : 6 > {});
+        return encode_impl(std::forward<V>(v), encode_tag<first_true<is_view<D>::value,
+                           std::is_same<D, BasicJsonType>::value,
+                           std::is_same<D, std::nullptr_t>::value,
+                           std::is_same<D, bool>::value,
+                           std::is_arithmetic<D>::value,
+                           std::is_convertible<const D&, string_view_t>::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<T>::value ? 0 : (std::is_signed<T>::value ? 1 : 2) > {});
+        r.scalar = number_node(x, std::integral_constant<int, first_true<std::is_floating_point<T>::value, std::is_signed<T>::value>::value> {});
         return r;
     }
 

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 <array>
 #include <cmath>
 #include <cstring>
 #include <cstdint>
@@ -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<const char*, 15> 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<std::size_t>(r(15))];
     }
     return s;
 }
@@ -66,7 +74,7 @@ ordered_json random_scalar()
         case 2:
             return static_cast<std::int64_t>(rng()) - 2147483648LL;
         case 3:
-            return static_cast<std::uint64_t>(rng()) * 4294967296ULL + rng();
+            return (static_cast<std::uint64_t>(rng()) * 4294967296ULL) + rng();
         case 4:
             return static_cast<double>(static_cast<std::int32_t>(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<double>();
-        const double b = j.get<double>();
-        CHECK(std::memcmp(&a, &b, sizeof(double)) == 0);
+        CHECK(bits(v.get<double>()) == bits(j.get<double>()));
     }
     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<std::size_t>(r(static_cast<int>(paths.size())))];
+                    const ptr_t& q = paths[static_cast<std::size_t>(r(static_cast<int>(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<std::size_t>(r(static_cast<int>(target.size())));
+                    const auto i = static_cast<std::size_t>(r(static_cast<int>(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");