From 02d57db31ebb10a9ea96ca06d61b1fc07eae00b2 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 11 Nov 2026 22:36:54 +0100 Subject: [PATCH] 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 --- .../docs/api/basic_json_view/operator_eq.md | 4 +- include/nlohmann/detail/view/compare.hpp | 27 +++++++++++-- single_include/nlohmann/json_view.hpp | 27 +++++++++++-- tests/src/unit-json_view_edit_values.cpp | 38 +++++++++++++++++++ 4 files changed, 87 insertions(+), 9 deletions(-) diff --git a/docs/mkdocs/docs/api/basic_json_view/operator_eq.md b/docs/mkdocs/docs/api/basic_json_view/operator_eq.md index b385aa0a9..2da60db9e 100644 --- a/docs/mkdocs/docs/api/basic_json_view/operator_eq.md +++ b/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. diff --git a/include/nlohmann/detail/view/compare.hpp b/include/nlohmann/detail/view/compare.hpp index 2d2c238b9..ab0e5bbef 100644 --- a/include/nlohmann/detail/view/compare.hpp +++ b/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 {}); 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& a, const view_side::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 bool floats_equal(const view_side& a, const view_side& b) { - return (a.value_node()->extra == b.value_node()->extra && a.float_token() == b.float_token()) || std::equal_to()(a.floating(), b.floating()); + return (a.value_node()->extra == b.value_node()->extra && a.float_token() == b.float_token() && !(view_side::nav::editable() && float_token_is_nan(*a.value_node(), a.float_token().data()))) || std::equal_to()(a.floating(), b.floating()); } template diff --git a/single_include/nlohmann/json_view.hpp b/single_include/nlohmann/json_view.hpp index f2a39d268..af959c06b 100644 --- a/single_include/nlohmann/json_view.hpp +++ 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 {}); 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& a, const view_side::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 bool floats_equal(const view_side& a, const view_side& b) { - return (a.value_node()->extra == b.value_node()->extra && a.float_token() == b.float_token()) || std::equal_to()(a.floating(), b.floating()); + return (a.value_node()->extra == b.value_node()->extra && a.float_token() == b.float_token() && !(view_side::nav::editable() && float_token_is_nan(*a.value_node(), a.float_token().data()))) || std::equal_to()(a.floating(), b.floating()); } template diff --git a/tests/src/unit-json_view_edit_values.cpp b/tests/src/unit-json_view_edit_values.cpp index 881d957f8..034b94e16 100644 --- a/tests/src/unit-json_view_edit_values.cpp +++ 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::quiet_NaN()); + d.set(d.root(), 1, std::numeric_limits::quiet_NaN()); + json_editable_document other = json_editable_document::parse("[0, 0, 1.5]"); + other.set(other.root(), 0, std::numeric_limits::quiet_NaN()); + other.set(other.root(), 1, std::numeric_limits::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::infinity()); + other.set(other.root(), 0, 0.1); + other.set(other.root(), 1, std::numeric_limits::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