Do not compare an edited NaN equal to itself in json_view (#6085)

Two views compared equal when the tokens of two floats were equal, and
the same node was equal to itself. An edit writes a NaN as the token
"nan", so a view of it (or of an array holding it) was equal to itself
and to another NaN, while json(NaN) == json(NaN) and view == json are
false. The token shortcut now excludes the edited "nan" token (found by
the missing digit layout), dispatched on the document mode by a tag: a
read-only document cannot hold a NaN, and its walk is unchanged
(instruction count and cycles on canada, citm and twitter within
noise). For editable documents the check costs about 2 % instructions
on canada (cycles within noise). The same-node shortcut is kept for
read-only documents and for editable ones that were never edited; a
document with edits is walked, so comparing it with itself is linear
instead of constant.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

docs/mkdocs/docs/api/basic_json_view/operator_eq.md

@@ -57,7 +57,9 @@ compared. Two arrays of different [`size()`](size.md) are rejected without visit
 Only a single number, boolean, or `#!cpp null` value is ever materialized into a `BasicJsonType`, to reuse its
 `operator==` — for numbers, so that values written differently in the source text but equal in value (e.g. an
 integer and a floating-point literal) still compare equal, following the same rules `BasicJsonType` does for special
-values such as `#!cpp NaN`. Constructing one of these scalars never allocates. Strings are compared directly, without
+values such as `#!cpp NaN`: a `#!cpp NaN` that an [edit](../basic_json_document/set.md) wrote is not equal to itself, so
+a view of it is not equal to itself either, nor is a view of an array or object that contains one, as for
+`BasicJsonType`. Constructing one of these scalars never allocates. Strings are compared directly, without
 allocating, either from the source text on both sides or, for overload 2, against `BasicJsonType`'s own string.
 Arrays and objects are never materialized at all; only their elements or members are visited, one pair at a time.
 

include/nlohmann/detail/view/compare.hpp

@@ -79,6 +79,14 @@ NLOHMANN_VIEW_ALWAYS_INLINE bool bytes_equal(const char* x, const char* y, std::
     return true;
 }
 
+/// whether a float node holds the text nan, which an edit writes for a NaN (and records without a digit layout, so
+/// that a parsed float is told apart by one comparison of what the walk has loaded anyway): equal tokens are an
+/// equal value, except for this one
+NLOHMANN_VIEW_ALWAYS_INLINE bool float_token_is_nan(const node& n, const char* s) noexcept
+{
+    return NLOHMANN_VIEW_UNLIKELY(n.extra == 0xFFFFu) && n.len == 3 && s[0] == 'n';
+}
+
 /// whether the n1 bytes at x are before the n2 bytes at y (memcmp() order, then the shorter first);
 /// the first difference is found eight bytes at a time
 NLOHMANN_VIEW_ALWAYS_INLINE bool bytes_less(const char* x, std::size_t n1, const char* y, std::size_t n2) noexcept
@@ -688,7 +696,7 @@ class lockstep
             case value_t::string:
                 return a->len == b->len && bytes_equal(m_a.str(*a), m_b.str(*b), a->len);
             case value_t::number_float: // (the token, and the digit layout that the value is read with: they only differ in a damaged image)
-                return a->extra == b->extra && a->len == b->len && bytes_equal(m_a.str(*a), m_b.str(*b), a->len);
+                return same_float(a, b, std::integral_constant<bool, EA> {});
             case value_t::number_integer:
             case value_t::number_unsigned:
                 return integer_bits(*a) == integer_bits(*b);
@@ -783,6 +791,17 @@ class lockstep
         return true;
     }
 
+    /// (a read-only document holds no NaN)
+    bool same_float(const node* a, const node* b, std::false_type /*editable*/) const noexcept
+    {
+        return a->extra == b->extra && a->len == b->len && bytes_equal(m_a.str(*a), m_b.str(*b), a->len);
+    }
+
+    bool same_float(const node* a, const node* b, std::true_type /*editable*/) const noexcept
+    {
+        return a->extra == b->extra && a->len == b->len && bytes_equal(m_a.str(*a), m_b.str(*b), a->len) && !float_token_is_nan(*a, m_a.str(*a));
+    }
+
     bool elements(const node* a, const node* b, unsigned budget, lockstep_path::entry& stop) noexcept
     {
         const node* pa = nav_a::first(m_a, a);
@@ -839,9 +858,9 @@ bool certainly_equal(const view_side<BasicJsonType, ViewA>& a, const view_side<B
     {
         return false;
     }
-    if (na == nb)
+    if (na == nb && (!view_side<BasicJsonType, ViewA>::nav::editable() || a.document().edits == nullptr))
     {
-        return true; // (the same node of the same document: documents have distinct nodes)
+        return true; // (the same node of the same document: documents have distinct nodes; a document with edits is walked, as an edit can have written a NaN, which is not equal to itself)
     }
     path.resume = nullptr;
     if (path.next < path.length && path.nodes[path.next].a == na && path.nodes[path.next].b == nb)
@@ -886,7 +905,7 @@ void resume_array(const lockstep_path& /*path*/, const A& /*a*/, const B& /*b*/,
 template<typename BasicJsonType, typename ViewA, typename ViewB>
 bool floats_equal(const view_side<BasicJsonType, ViewA>& a, const view_side<BasicJsonType, ViewB>& b)
 {
-    return (a.value_node()->extra == b.value_node()->extra && a.float_token() == b.float_token()) || std::equal_to<typename BasicJsonType::number_float_t>()(a.floating(), b.floating());
+    return (a.value_node()->extra == b.value_node()->extra && a.float_token() == b.float_token() && !(view_side<BasicJsonType, ViewA>::nav::editable() && float_token_is_nan(*a.value_node(), a.float_token().data()))) || std::equal_to<typename BasicJsonType::number_float_t>()(a.floating(), b.floating());
 }
 
 template<typename A, typename B>

single_include/nlohmann/json_view.hpp

@@ -3262,6 +3262,14 @@ NLOHMANN_VIEW_ALWAYS_INLINE bool bytes_equal(const char* x, const char* y, std::
     return true;
 }
 
+/// whether a float node holds the text nan, which an edit writes for a NaN (and records without a digit layout, so
+/// that a parsed float is told apart by one comparison of what the walk has loaded anyway): equal tokens are an
+/// equal value, except for this one
+NLOHMANN_VIEW_ALWAYS_INLINE bool float_token_is_nan(const node& n, const char* s) noexcept
+{
+    return NLOHMANN_VIEW_UNLIKELY(n.extra == 0xFFFFu) && n.len == 3 && s[0] == 'n';
+}
+
 /// whether the n1 bytes at x are before the n2 bytes at y (memcmp() order, then the shorter first);
 /// the first difference is found eight bytes at a time
 NLOHMANN_VIEW_ALWAYS_INLINE bool bytes_less(const char* x, std::size_t n1, const char* y, std::size_t n2) noexcept
@@ -3871,7 +3879,7 @@ class lockstep
             case value_t::string:
                 return a->len == b->len && bytes_equal(m_a.str(*a), m_b.str(*b), a->len);
             case value_t::number_float: // (the token, and the digit layout that the value is read with: they only differ in a damaged image)
-                return a->extra == b->extra && a->len == b->len && bytes_equal(m_a.str(*a), m_b.str(*b), a->len);
+                return same_float(a, b, std::integral_constant<bool, EA> {});
             case value_t::number_integer:
             case value_t::number_unsigned:
                 return integer_bits(*a) == integer_bits(*b);
@@ -3966,6 +3974,17 @@ class lockstep
         return true;
     }
 
+    /// (a read-only document holds no NaN)
+    bool same_float(const node* a, const node* b, std::false_type /*editable*/) const noexcept
+    {
+        return a->extra == b->extra && a->len == b->len && bytes_equal(m_a.str(*a), m_b.str(*b), a->len);
+    }
+
+    bool same_float(const node* a, const node* b, std::true_type /*editable*/) const noexcept
+    {
+        return a->extra == b->extra && a->len == b->len && bytes_equal(m_a.str(*a), m_b.str(*b), a->len) && !float_token_is_nan(*a, m_a.str(*a));
+    }
+
     bool elements(const node* a, const node* b, unsigned budget, lockstep_path::entry& stop) noexcept
     {
         const node* pa = nav_a::first(m_a, a);
@@ -4022,9 +4041,9 @@ bool certainly_equal(const view_side<BasicJsonType, ViewA>& a, const view_side<B
     {
         return false;
     }
-    if (na == nb)
+    if (na == nb && (!view_side<BasicJsonType, ViewA>::nav::editable() || a.document().edits == nullptr))
     {
-        return true; // (the same node of the same document: documents have distinct nodes)
+        return true; // (the same node of the same document: documents have distinct nodes; a document with edits is walked, as an edit can have written a NaN, which is not equal to itself)
     }
     path.resume = nullptr;
     if (path.next < path.length && path.nodes[path.next].a == na && path.nodes[path.next].b == nb)
@@ -4069,7 +4088,7 @@ void resume_array(const lockstep_path& /*path*/, const A& /*a*/, const B& /*b*/,
 template<typename BasicJsonType, typename ViewA, typename ViewB>
 bool floats_equal(const view_side<BasicJsonType, ViewA>& a, const view_side<BasicJsonType, ViewB>& b)
 {
-    return (a.value_node()->extra == b.value_node()->extra && a.float_token() == b.float_token()) || std::equal_to<typename BasicJsonType::number_float_t>()(a.floating(), b.floating());
+    return (a.value_node()->extra == b.value_node()->extra && a.float_token() == b.float_token() && !(view_side<BasicJsonType, ViewA>::nav::editable() && float_token_is_nan(*a.value_node(), a.float_token().data()))) || std::equal_to<typename BasicJsonType::number_float_t>()(a.floating(), b.floating());
 }
 
 template<typename A, typename B>

tests/src/unit-json_view_edit_values.cpp

@@ -206,6 +206,44 @@ TEST_CASE("json_view edits: views and values")
         CHECK(d.root().materialize().dump() == json::parse(R"([1.5, 100.0, 0.1, null, null, 18446744073709551615, -9223372036854775808])").dump());
     }
 
+    SECTION("a NaN written by an edit is not equal to itself")
+    {
+        // as for basic_json: NaN != NaN, so a view of it, of the array that holds it, and the same view twice agree
+        json_editable_document d = json_editable_document::parse("[0, 0, 1.5]");
+        d.set(d.root(), 0, std::numeric_limits<double>::quiet_NaN());
+        d.set(d.root(), 1, std::numeric_limits<double>::quiet_NaN());
+        json_editable_document other = json_editable_document::parse("[0, 0, 1.5]");
+        other.set(other.root(), 0, std::numeric_limits<double>::quiet_NaN());
+        other.set(other.root(), 1, std::numeric_limits<double>::quiet_NaN());
+        const json value = d.root().materialize();
+
+        CHECK_FALSE(value == value);
+        CHECK_FALSE(d.root()[0] == d.root()[0]);
+        CHECK_FALSE(d.root()[0] == d.root()[1]);
+        CHECK_FALSE(d.root() == d.root());
+        CHECK_FALSE(d.root() == other.root());
+        CHECK_FALSE(d.root()[0] == other.root()[0]);
+        CHECK(d.root() != d.root());
+        CHECK(d.root()[0] != d.root()[0]);
+        CHECK_FALSE(d.root() == value);
+        CHECK_FALSE(d.root()[0] == value[0]);
+        // the other elements are equal as before
+        CHECK(d.root()[2] == d.root()[2]);
+        CHECK(d.root()[2] == other.root()[2]);
+        // finite and infinite floats written by an edit stay equal to themselves
+        d.set(d.root(), 0, 0.1);
+        d.set(d.root(), 1, std::numeric_limits<double>::infinity());
+        other.set(other.root(), 0, 0.1);
+        other.set(other.root(), 1, std::numeric_limits<double>::infinity());
+        CHECK(d.root() == d.root());
+        CHECK(d.root() == other.root());
+        CHECK(d.root()[1] == other.root()[1]);
+        // a document that was never edited
+        const json_editable_document untouched = json_editable_document::parse("[0.5, [1, 2]]");
+        CHECK(untouched.root() == untouched.root());
+        CHECK(untouched.root()[1] == untouched.root()[1]);
+    }
+
     SECTION("numbers of other float types")
     {
         // doubles have their own path to the output; other float types are