From ce7498b54c3b05b2a8f414f7a61937d4344c2160 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sat, 31 Oct 2026 08:43:27 +0100 Subject: [PATCH] CBOR reader: address review of the scalar-run reader - parse_cbor_scalar_run: set the size explicitly in every case that reads a value, instead of relying on a default that every other case overwrites. - Remove cbor_scalar_follows(): parse_cbor_scalar_run already returns on the same conditions, and without the pre-check SAX canada needs another 8.5 % fewer instructions with GCC 13 (0.915 against the previous version), with no row more than 0.5 % worse (Mac and GCC). - Restore the comment on why parse_cbor_internal copies the container frame. Signed-off-by: Niels Lohmann --- .../nlohmann/detail/input/binary_reader.hpp | 31 ++++++------------- single_include/nlohmann/json.hpp | 31 ++++++------------- 2 files changed, 20 insertions(+), 42 deletions(-) diff --git a/include/nlohmann/detail/input/binary_reader.hpp b/include/nlohmann/detail/input/binary_reader.hpp index 5b105974e..5c7f9e10e 100644 --- a/include/nlohmann/detail/input/binary_reader.hpp +++ b/include/nlohmann/detail/input/binary_reader.hpp @@ -1711,7 +1711,7 @@ class binary_reader { const auto* const data = reinterpret_cast(ia.bulk_data()); const unsigned char head = data[0]; - std::size_t size = 1; + std::size_t size = 0; bool accepted = true; switch (head) { @@ -1748,12 +1748,15 @@ class binary_reader accepted = emit_cbor_negative_integer(load_big_endian(data + 1)); break; case 0xF4: + size = 1; accepted = sax->boolean(false); break; case 0xF5: + size = 1; accepted = sax->boolean(true); break; case 0xF6: + size = 1; accepted = sax->null(); break; case 0xFA: @@ -1767,10 +1770,12 @@ class binary_reader default: if (head <= 0x17) { + size = 1; accepted = sax->number_unsigned(static_cast(head)); } else if (head >= 0x20 && head <= 0x37) { + size = 1; accepted = sax->number_integer(static_cast(0x20 - 1 - head)); } else @@ -1815,7 +1820,7 @@ class binary_reader JSON_HEDLEY_NEVER_INLINE bool parse_cbor_array_start(std::true_type /*bulk*/) { - return !cbor_scalar_follows() || parse_cbor_scalar_run(container_stack.back().remaining); + return parse_cbor_scalar_run(container_stack.back().remaining); } /// input that is not contiguous: every value is read by parse_cbor_value @@ -1824,25 +1829,6 @@ class binary_reader return true; } - /*! - @brief whether @ref parse_cbor_scalar_run can read the next value: one of - the heads it handles, with at least 9 bytes of input left - */ - bool cbor_scalar_follows() const - { - if (ia.bulk_remaining() < 9) - { - return false; - } - const auto head = static_cast(*reinterpret_cast(ia.bulk_data())); - // major types 0 and 1 (unsigned and negative integers) up to an - // 8-byte argument, or false, true, null, float32, float64 (0xF4, - // 0xF5, 0xF6, 0xFA, 0xFB: bits 0, 1, 2, 6, 7 of 0xC7) - return ((head & 0xC0u) == 0 && (head & 0x1Fu) <= 0x1Bu) - || (head >= 0xF4u && ((0xC7u >> (head - 0xF4u)) & 1u) != 0); - } - - /*! @brief read a CBOR value and everything nested inside it @@ -1868,6 +1854,9 @@ class binary_reader { if (!container_stack.empty()) { + // a copy, not a reference: it must stay valid across the + // pop_back() below, which destroys the container_stack element + // it would otherwise alias const container_frame top = container_stack.back(); const bool is_object = top.is_object; bool at_end = false; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index b7ab5b7d9..b018dcdc6 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -19487,7 +19487,7 @@ class binary_reader { const auto* const data = reinterpret_cast(ia.bulk_data()); const unsigned char head = data[0]; - std::size_t size = 1; + std::size_t size = 0; bool accepted = true; switch (head) { @@ -19524,12 +19524,15 @@ class binary_reader accepted = emit_cbor_negative_integer(load_big_endian(data + 1)); break; case 0xF4: + size = 1; accepted = sax->boolean(false); break; case 0xF5: + size = 1; accepted = sax->boolean(true); break; case 0xF6: + size = 1; accepted = sax->null(); break; case 0xFA: @@ -19543,10 +19546,12 @@ class binary_reader default: if (head <= 0x17) { + size = 1; accepted = sax->number_unsigned(static_cast(head)); } else if (head >= 0x20 && head <= 0x37) { + size = 1; accepted = sax->number_integer(static_cast(0x20 - 1 - head)); } else @@ -19591,7 +19596,7 @@ class binary_reader JSON_HEDLEY_NEVER_INLINE bool parse_cbor_array_start(std::true_type /*bulk*/) { - return !cbor_scalar_follows() || parse_cbor_scalar_run(container_stack.back().remaining); + return parse_cbor_scalar_run(container_stack.back().remaining); } /// input that is not contiguous: every value is read by parse_cbor_value @@ -19600,25 +19605,6 @@ class binary_reader return true; } - /*! - @brief whether @ref parse_cbor_scalar_run can read the next value: one of - the heads it handles, with at least 9 bytes of input left - */ - bool cbor_scalar_follows() const - { - if (ia.bulk_remaining() < 9) - { - return false; - } - const auto head = static_cast(*reinterpret_cast(ia.bulk_data())); - // major types 0 and 1 (unsigned and negative integers) up to an - // 8-byte argument, or false, true, null, float32, float64 (0xF4, - // 0xF5, 0xF6, 0xFA, 0xFB: bits 0, 1, 2, 6, 7 of 0xC7) - return ((head & 0xC0u) == 0 && (head & 0x1Fu) <= 0x1Bu) - || (head >= 0xF4u && ((0xC7u >> (head - 0xF4u)) & 1u) != 0); - } - - /*! @brief read a CBOR value and everything nested inside it @@ -19644,6 +19630,9 @@ class binary_reader { if (!container_stack.empty()) { + // a copy, not a reference: it must stay valid across the + // pop_back() below, which destroys the container_stack element + // it would otherwise alias const container_frame top = container_stack.back(); const bool is_object = top.is_object; bool at_end = false;