Compare commits

..
Author SHA1 Message Date
Claude 675f914725 Add review-like-gregmarr Claude skill
Distilled from gregmarr's inline review and conversation comments on
nlohmann/json: checklist of recurring review topics and his review voice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N5NasBGrV1SGwx7n1eo75g
2026-10-30 23:26:28 +00:00
7 changed files with 473 additions and 1448 deletions

No files matched your search

@@ -0,0 +1,194 @@
---
name: review-like-gregmarr
description: Review a diff, branch, or PR of nlohmann/json the way long-time contributor gregmarr does — a line-by-line read focused on C++11 correctness, reference invalidation, backward/source compatibility, ODR/ABI, duplicated logic, hidden costs, repo conventions (amalgamation, astyle, macros) and precise docs, phrased as short courteous questions. Use when asked to "review like Greg", "gregmarr review", or for a careful maintainer-style review of changes to this library.
---
# Review like gregmarr
Distilled from ~1,400 inline review comments and ~1,300 issue/PR comments by
@gregmarr on nlohmann/json. He is a careful, courteous, line-by-line reviewer
who ("just a contributor") defers final decisions to @nlohmann.
## Workflow
1. Determine the target: a PR number (`gh pr diff <n>`), a branch
(`git diff $(git merge-base HEAD develop)...HEAD`), or the working tree
(`git diff`). Read the full diff, and open surrounding code in `include/nlohmann/`
whenever a hunk's context is not enough to judge it.
2. Walk the diff **file by file, hunk by hunk**, applying the checklist below.
Comment on specific lines; don't write a big essay.
3. Write the review in his voice (see "Voice"). Group nothing artificially —
emit one comment per location, in file/line order, with `path:line`.
4. If the same issue repeats, comment fully once, then "As above" /
"Same here (and N other places in this file)."
5. End with a 1–3 line summary only if useful (e.g. "Just this remaining and
then LGTM." or "@nlohmann thoughts on the naming?"). Don't post to GitHub
unless the user asks.
## Checklist (roughly in order of how often he raises them)
### Duplication and simplification (most frequent)
- Copy-pasted bodies, duplicated `#if/#elif` blocks, or "a copy of this function
that can get out of sync" → propose a shared helper or alias, with a code sketch.
- **Diverging limits/constants**: a new depth/recursion limit or magic number
where one already exists → "Should this reuse the existing recursion limit?
Different limits per operation invites them to diverge over time."
- Unneeded variables, parameters that can be deduced, values passed to a
function that the constructor already has, output parameter == return value.
- Verbose type spellings → `using` alias / alias template; `decltype()` where the
type is known.
- Disabling a lint warning instead of fixing the code ("why not just add the parens?").
### Lifetime, invalidation, UB
- A reference/pointer/iterator into a container (`stack.back()`,
`container_stack.back()`, `ordered_map` elements) held across `push_back`,
`pop_back`, `emplace`, or reallocation → "This invalidates `top`." Prefer a
`const` copy, or at least a comment warning future editors.
- Dangling refs from rvalue arguments stored in adapters; views outliving documents.
- Wrong logic operators (`&&` vs `||`), lost `!`, unreachable code, `continue`
that should be `break`, missing `return`, off-by-one in counters
(`chars_read` incremented twice).
- Uninitialized members → default member initializers; parameters that
"should not have a default so it's never left off by accident".
- Exception safety: leaks when `to_json` is called on a non-null value, partial
state, destructors that allocate, swallowed exceptions (`noexcept` that hides
throws — "The library shouldn't try to swallow the exception").
- Thread-unsafe calls (`setlocale`), specializations that redefine `std::` things
(ODR / UB).
- Narrowing/truncation: `static_cast<std::make_unsigned<...>>`, signed/unsigned
`char` cases, `n == 0` edge cases, buffer overruns guarded only by `JSON_ASSERT`.
### C++11 floor and portability
- The library requires only **C++11**: flag `if constexpr`, C++14 generic
lambdas/variable templates, `std::clamp`, C++17 library features, etc. unless
gated.
- Gate on the library's macros (`JSON_HAS_CPP_17`, `JSON_HAS_CPP_20`,
`JSON_HAS_THREE_WAY_COMPARISON`, `JSON_HAS_FILESYSTEM`) or feature-test macros
with `defined()` checks and exact values
(`#if defined(__cpp_lib_char8_t) && __cpp_lib_char8_t >= 201811`), not raw
`__cplusplus`. Don't silently disable a feature for an existing config.
- User-overridable macros: if externally defined, leave as-is — never clobber to
`1` (breaks `#define X 0`). `#if X` vs `#ifdef X` must match the convention.
- New macros start with `JSON_` / `NLOHMANN_JSON_` consistently with neighbors.
- `throw` must be `JSON_THROW` (and `try/catch` → `JSON_TRY/JSON_CATCH`) so
`JSON_NOEXCEPTION` builds work.
- `(std::min)(a, b)` / `(std::max)` for Windows macros; MSVC warnings (C4100
etc.); `string_t` not being `std::string`/`char`.
- Code relying on operations not required of user-supplied `array_t`/`object_t`
(range ctor, `emplace_hint`, allocator) → ask whether that's allowed.
- Does it work with `JSON_USE_IMPLICIT_CONVERSIONS=0`? With `JSON_DIAGNOSTICS`?
### Compatibility, ABI, API design
- Behavior changes to existing APIs/macros (e.g. `NLOHMANN_DEFINE_TYPE_*`,
released in 3.11.0) are breaking → needs opt-in macro or wait for 4.0; must be
prominently documented in release notes. Cite when the behavior shipped.
- Header-only: source compatibility matters, binary doesn't — adding a
**defaulted** parameter is fine; new overloads add ambiguity risk.
- Anything that changes layout or semantics per-config → should it be part of the
ABI tag (`json_abi_diag...`)? Mismatched config macros across TUs = ODR violation.
- Brand-new API shouldn't be added as deprecated — use `= delete` or omit.
- Naming consistency with existing options (`ignore_trailing_commas` vs
`ignore_comments`; singular "diagnostic"); names that read like functions;
single-letter or inconsistently `m_`-prefixed members.
- Prefer a type-safe template over passing a `value_t` that could be invalid.
- "Why would this belong in this library?" for very niche features.
### Hidden costs
- Extra copies (lost `std::move`, removed C++17 path), temporaries
(`push_back(std::string(...))` vs two `push_back`s), repeated `.back()` calls,
casting more than once, `reserve()` inside loops, quadratic recomputation for
deep nesting, dynamic allocation replacing stack allocation.
- `push_back` → `emplace_back`; `rbegin()`, `.data()`, `std::array`, `pop_back()`
to keep counters in sync.
- Struct member order/padding (put `bool` after `int`; "store value before type"),
`sizeof(basic_json)` growth, per-node pointer cost. "Don't pay for what you
don't use."
- Compile-time impact of new includes/templates.
- Ask for measurement instead of asserting a win.
### Style and repo conventions
- Formatting must match `make pretty` (astyle): spaced `template <`, `!` in traits
→ suggest `detail::negation<>`; 4-space indents; brace style; blank lines.
- Redundant keywords: `inline` inside class bodies or on templates (full
specializations do need it), `noexcept` on destructors, `public` in `struct`,
`{}` after defaulted init, C-style casts → `static_cast`.
- Sorted includes (cpplint: system before others), sorted lists in docs/CMake.
- **Amalgamation**: changes must be in `include/nlohmann/...` and
`single_include/` regenerated via `make amalgamate` — flag edits made only in
`single_include`, or amalgamated output that doesn't match / contains unrelated
changes.
- Merge debris, stray files at repo root, accidental changes to vendored files
(doctest.h), Makefile tweaks from local testing, missing trailing newline,
whitespace churn mixed with real changes ("separate commit or PR?").
- Comments: explain non-obvious code ("Comment about why this iterates
backwards?"); no change-log wording like "still" in live code; comment
blocks must stay attached to their function.
### Docs (`docs/mkdocs/docs/...`, README)
- Typos as `s/varry/vary/` or "typo: interal".
- Accuracy vs. actual behavior; state preconditions ("array is not empty"),
costs (one pointer per value with `JSON_DIAGNOSTICS`), defaults, which overloads
a sentence applies to, version numbers ("3.13.0").
- Consistency with sibling pages; unexplained claims ("Why is that a problem?
It's not obvious here."); markdown table column alignment.
- New macros/features need a docs page and an example.
### Tests
- Not his main focus, but: new macro/feature needs tests in `tests/src/unit-*.cpp`
that always build with the option (coverage build); test both with and
without the option; tests should use different values than the
initial ones; put tests in the existing file instead of duplicating a SAX class;
static_asserts where possible.
### Usually NOT commented on
Commit messages/PR titles, licensing, local variable naming bikeshedding,
demanding benchmarks, rejecting the overall feature idea. CI YAML gets
comments only for merge debris.
## Voice
- **Mostly questions**: "Is there a reason…?", "Should this…?", "Would it make
sense to…?", "Why not…?", "Any reason not to…?", "Is this needed?",
"Intentional?"
- **Flat statements only for clear bugs**, with the reason: "This `if` will
never be true. You want `||` not `&&`." "Remove the noexcept. The library
shouldn't try to swallow the exception."
- **Terse when trivial**: "`emplace_back`", "typo: alse", "astyle, negation",
"as above", "Lost a blank line here."
- **Concrete sketches** in plain fenced code blocks introduced with "Maybe like
this?" / "Something like this:" / "Untested:". Rarely ```suggestion blocks.
- **Hedge taste**: "minor nit:", "Personally I prefer…", "I'm on the fence…",
"not something for which I would hold up the PR", "(If this was just moved,
ignore this.)", "not the fault of this PR".
- Cite evidence: cppreference, godbolt, the standard, related issues/PRs by
number, existing code ("see `NLOHMANN_JSON_ABI_TAG_DIAGNOSTICS`").
- Light humor allowed sparingly (":)", "astyle strikes again", "Ugh").
- Praise is rare and short ("Good change.", "Looks reasonable.").
- Defer design calls: "@nlohmann has the final say here."
- Never invent facts about the code: if unsure, phrase it as a question rather than a claim.
## Example output
```
include/nlohmann/detail/input/parser.hpp:212
This invalidates `top`. Make it a `const` copy so it can't be read after the
`push_back` by a future change?
include/nlohmann/detail/input/parser.hpp:240
Should this reuse the existing recursion limit rather than adding a new one?
Different limits per operation invites them to diverge over time.
include/nlohmann/detail/macro_scope.hpp:88
If this has been externally defined, we should leave it as-is. This will break
if someone has `#define JSON_HAS_FOO 0`.
include/nlohmann/json.hpp:1534
`throw` → `JSON_THROW`, or this can't be used by people that disable exceptions.
single_include/nlohmann/json.hpp:9001
Looks like this was edited directly. Changes go in `include/` and then
`make amalgamate`.
docs/mkdocs/docs/api/basic_json/back.md:12
Should this list the precondition that the array is not empty?
```
+27 -453
View File
@@ -22,7 +22,6 @@
#include <bit> //byteswap
#endif
#include <nlohmann/detail/bit_ops.hpp>
#include <nlohmann/detail/exceptions.hpp>
#include <nlohmann/detail/input/input_adapters.hpp>
#include <nlohmann/detail/input/json_sax.hpp>
@@ -742,13 +741,11 @@ class binary_reader
bool get_cbor_negative_integer()
{
NumberType number{};
return get_number(number) && emit_cbor_negative_integer(number);
}
if (JSON_HEDLEY_UNLIKELY(!get_number(number)))
{
return false;
}
/// pass the CBOR negative integer -1 - @a number to the SAX parser
template<typename NumberType>
bool emit_cbor_negative_integer(const NumberType number)
{
// the value is -1 - number, which fits into number_integer_t
// whenever number does; the outer cast undoes the integral promotion
// for number_integer_t types narrower than int
@@ -973,7 +970,7 @@ class binary_reader
case 0x95:
case 0x96:
case 0x97:
return enter_cbor_array(conditional_static_cast<std::size_t>(static_cast<unsigned int>(current) & 0x1Fu));
return enter_array(conditional_static_cast<std::size_t>(static_cast<unsigned int>(current) & 0x1Fu));
case 0x98: // array (one-byte uint8_t for n follows)
case 0x99: // array (two-byte uint16_t for n follow)
@@ -982,7 +979,7 @@ class binary_reader
{
std::uint64_t len{};
std::size_t size{};
return get_cbor_argument(len) && get_cbor_container_size(len, size, "array") && enter_cbor_array(size);
return get_cbor_argument(len) && get_cbor_container_size(len, size, "array") && enter_array(size);
}
case 0x9F: // array (indefinite length)
@@ -1136,10 +1133,16 @@ class binary_reader
return get_half_float(false);
case 0xFA: // Single-Precision Float (four-byte IEEE 754)
return get_cbor_float<float>();
{
float number{};
return get_number(number) && emit_float(number);
}
case 0xFB: // Double-Precision Float (eight-byte IEEE 754)
return get_cbor_float<double>();
{
double number{};
return get_number(number) && emit_float(number);
}
default: // anything else (0xFF is handled inside the other types)
{
@@ -1148,16 +1151,6 @@ class binary_reader
}
}
/// read a single- or double-precision float (kept out of
/// parse_cbor_value, which is inlined into parse_cbor_internal)
template<typename NumberType>
JSON_HEDLEY_NEVER_INLINE
bool get_cbor_float()
{
NumberType number{};
return get_number(number) && emit_float(number);
}
/*!
@brief reports a nested indefinite-length CBOR string or byte array
@param[in] type_name name of the rejected string type
@@ -1683,157 +1676,6 @@ class binary_reader
return true;
}
/*!
@brief read a big-endian number of type @a NumberType at @a data
*/
template<typename NumberType>
NumberType load_big_endian(const unsigned char* data) const noexcept
{
NumberType result{};
std::memcpy(&result, data, sizeof(NumberType));
if (is_little_endian)
{
byte_swap(result);
}
return result;
}
/*!
@brief read the integers, floats, booleans, and nulls that follow in a
definite-length array, without the per-element bookkeeping of
@ref parse_cbor_internal
Stops at the end of the array, at any other kind of value, and when fewer
than 9 bytes (the longest of these values) are left in the input, leaving
those to @ref parse_cbor_value, which reads them the same way.
@param[in,out] remaining the number of elements of the array not yet read
@return whether the SAX parser accepted the values
*/
bool parse_cbor_scalar_run(std::size_t& remaining)
{
while (remaining != 0 && ia.bulk_remaining() >= 9)
{
const auto* const data = reinterpret_cast<const unsigned char*>(ia.bulk_data());
const unsigned char head = data[0];
std::size_t size = 0;
bool accepted = true;
switch (head)
{
case 0x18:
size = 2;
accepted = emit_unsigned(data[1]);
break;
case 0x19:
size = 3;
accepted = emit_unsigned(load_big_endian<std::uint16_t>(data + 1));
break;
case 0x1A:
size = 5;
accepted = emit_unsigned(load_big_endian<std::uint32_t>(data + 1));
break;
case 0x1B:
size = 9;
accepted = emit_unsigned(load_big_endian<std::uint64_t>(data + 1));
break;
case 0x38:
size = 2;
accepted = emit_cbor_negative_integer(data[1]);
break;
case 0x39:
size = 3;
accepted = emit_cbor_negative_integer(load_big_endian<std::uint16_t>(data + 1));
break;
case 0x3A:
size = 5;
accepted = emit_cbor_negative_integer(load_big_endian<std::uint32_t>(data + 1));
break;
case 0x3B:
size = 9;
accepted = emit_cbor_negative_integer(load_big_endian<std::uint64_t>(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:
size = 5;
accepted = emit_float(load_big_endian<float>(data + 1));
break;
case 0xFB:
size = 9;
accepted = emit_float(load_big_endian<double>(data + 1));
break;
default:
if (head <= 0x17)
{
size = 1;
accepted = sax->number_unsigned(static_cast<number_unsigned_t>(head));
}
else if (head >= 0x20 && head <= 0x37)
{
size = 1;
accepted = sax->number_integer(static_cast<std::int8_t>(0x20 - 1 - head));
}
else
{
// any other value is read by parse_cbor_value
return true;
}
break;
}
// as if read by get() and get_number(): current is the head byte
ia.bulk_skip(size);
chars_read += size;
current = head;
--remaining;
if (JSON_HEDLEY_UNLIKELY(!accepted))
{
return false;
}
}
return true;
}
/*!
@brief open a definite-length array and read the scalars it begins with
(see @ref parse_cbor_scalar_run)
Only the start of an array is tried, which covers arrays of numbers
(e.g. coordinates) but costs arrays of other values only one test, not one
per element.
@param[in] len the number of elements
@return whether the SAX parser accepted the array and the values read
*/
bool enter_cbor_array(const std::size_t len)
{
return enter_array(len) && (len == 0 || parse_cbor_array_start(std::integral_constant<bool, bulk_scan> {}));
}
/// read the scalars the array just opened begins with; out of line, so
/// that @ref parse_cbor_value stays small enough to be inlined into
/// @ref parse_cbor_internal
JSON_HEDLEY_NEVER_INLINE
bool parse_cbor_array_start(std::true_type /*bulk*/)
{
return parse_cbor_scalar_run(container_stack.back().remaining);
}
/// input that is not contiguous: every value is read by parse_cbor_value
bool parse_cbor_array_start(std::false_type /*bulk*/) const noexcept
{
return true;
}
/*!
@brief read a CBOR value and everything nested inside it
@@ -1863,7 +1705,6 @@ class binary_reader
// 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;
if (top.remaining != npos)
@@ -1875,7 +1716,7 @@ class binary_reader
{
// claim the element about to be read
--container_stack.back().remaining;
if (is_object)
if (top.is_object)
{
get();
}
@@ -1888,7 +1729,7 @@ class binary_reader
// Testing for it consumes a byte, which is the first byte
// of the next element when it is not one.
at_end = (get() == 0xFF);
fetch = is_object;
fetch = top.is_object;
}
if (at_end)
@@ -1905,7 +1746,7 @@ class binary_reader
continue;
}
if (is_object)
if (top.is_object)
{
if (JSON_HEDLEY_UNLIKELY(!skip_cbor_key_tags(tag_handler)))
{
@@ -2130,7 +1971,7 @@ class binary_reader
case 0x9D:
case 0x9E:
case 0x9F:
return enter_msgpack_array(conditional_static_cast<std::size_t>(static_cast<unsigned int>(current) & 0x0Fu));
return enter_array(conditional_static_cast<std::size_t>(static_cast<unsigned int>(current) & 0x0Fu));
// fixstr
case 0xA0:
@@ -2261,13 +2102,13 @@ class binary_reader
case 0xDC: // array 16
{
std::uint16_t len{};
return get_number(len) && enter_msgpack_array(static_cast<std::size_t>(len));
return get_number(len) && enter_array(static_cast<std::size_t>(len));
}
case 0xDD: // array 32
{
std::uint32_t len{};
return get_number(len) && enter_msgpack_array(conditional_static_cast<std::size_t>(len));
return get_number(len) && enter_array(conditional_static_cast<std::size_t>(len));
}
case 0xDE: // map 16
@@ -2609,138 +2450,6 @@ class binary_reader
}
}
/*!
@brief read the integers, floats, booleans, and nils that follow in a
MessagePack array, without the per-element bookkeeping of
@ref parse_msgpack_internal
Stops at the end of the array, at any other kind of value, and when fewer
than 9 bytes (the longest of these values) are left in the input, leaving
those to @ref parse_msgpack_value, which reads them the same way.
@param[in,out] remaining the number of elements of the array not yet read
@return whether the SAX parser accepted the values
*/
bool parse_msgpack_scalar_run(std::size_t& remaining)
{
while (remaining != 0 && ia.bulk_remaining() >= 9)
{
const auto* const data = reinterpret_cast<const unsigned char*>(ia.bulk_data());
const unsigned char head = data[0];
std::size_t size = 0;
bool accepted = true;
switch (head)
{
case 0xC0: // nil
size = 1;
accepted = sax->null();
break;
case 0xC2: // false
size = 1;
accepted = sax->boolean(false);
break;
case 0xC3: // true
size = 1;
accepted = sax->boolean(true);
break;
case 0xCA: // float 32
size = 5;
accepted = emit_float(load_big_endian<float>(data + 1));
break;
case 0xCB: // float 64
size = 9;
accepted = emit_float(load_big_endian<double>(data + 1));
break;
case 0xCC: // uint 8
size = 2;
accepted = emit_unsigned(data[1]);
break;
case 0xCD: // uint 16
size = 3;
accepted = emit_unsigned(load_big_endian<std::uint16_t>(data + 1));
break;
case 0xCE: // uint 32
size = 5;
accepted = emit_unsigned(load_big_endian<std::uint32_t>(data + 1));
break;
case 0xCF: // uint 64
size = 9;
accepted = emit_unsigned(load_big_endian<std::uint64_t>(data + 1));
break;
case 0xD0: // int 8
size = 2;
accepted = emit_signed(static_cast<std::int8_t>(data[1]));
break;
case 0xD1: // int 16
size = 3;
accepted = emit_signed(load_big_endian<std::int16_t>(data + 1));
break;
case 0xD2: // int 32
size = 5;
accepted = emit_signed(load_big_endian<std::int32_t>(data + 1));
break;
case 0xD3: // int 64
size = 9;
accepted = emit_signed(load_big_endian<std::int64_t>(data + 1));
break;
default:
if (head <= 0x7F)
{
// positive fixint
size = 1;
accepted = sax->number_unsigned(static_cast<number_unsigned_t>(head));
}
else if (head >= 0xE0)
{
// negative fixint
size = 1;
accepted = sax->number_integer(static_cast<std::int8_t>(head));
}
else
{
// any other value is read by parse_msgpack_value
return true;
}
break;
}
// as if read by get() and get_number(): current is the last byte
ia.bulk_skip(size);
chars_read += size;
current = data[size - 1];
--remaining;
if (JSON_HEDLEY_UNLIKELY(!accepted))
{
return false;
}
}
return true;
}
/*!
@brief open a MessagePack array and read the scalars it begins with (see
@ref parse_msgpack_scalar_run)
@param[in] len the number of elements
@return whether the SAX parser accepted the array and the values read
*/
bool enter_msgpack_array(const std::size_t len)
{
return enter_array(len) && (len == 0 || parse_msgpack_array_start(std::integral_constant<bool, bulk_scan> {}));
}
/// read the scalars the array just opened begins with; out of line, so
/// that @ref parse_msgpack_value stays small enough to be inlined
JSON_HEDLEY_NEVER_INLINE
bool parse_msgpack_array_start(std::true_type /*bulk*/)
{
return parse_msgpack_scalar_run(container_stack.back().remaining);
}
/// input that is not contiguous: every value is read by parse_msgpack_value
bool parse_msgpack_array_start(std::false_type /*bulk*/) const noexcept
{
return true;
}
/*!
@brief read a MessagePack value and everything nested inside it
@@ -3546,107 +3255,6 @@ class binary_reader
return unexpected_byte("invalid byte", "value");
}
/*!
@brief read the elements of an optimized array of numbers (type marker
@a type) that are in the input, in one loop, without the
per-element bookkeeping of @ref parse_ubjson_internal
Values are emitted as @ref get_ubjson_value emits them. Types other than
the fixed-size integers and floats are left to @ref get_ubjson_value, as
are the elements the input ends before.
@param[in,out] remaining the number of elements not yet read
@param[in] type the type marker of the elements
@return whether the SAX parser accepted the values
*/
JSON_HEDLEY_NEVER_INLINE
bool parse_ubjson_typed_run(std::size_t& remaining, const char_int_type type, std::true_type /*bulk*/)
{
const bool bjdata = input_format == input_format_t::bjdata;
switch (type)
{
case 'U':
return parse_ubjson_typed_run<std::uint8_t>(remaining);
case 'i':
return parse_ubjson_typed_run<std::int8_t>(remaining);
case 'I':
return parse_ubjson_typed_run<std::int16_t>(remaining);
case 'l':
return parse_ubjson_typed_run<std::int32_t>(remaining);
case 'L':
return parse_ubjson_typed_run<std::int64_t>(remaining);
case 'u':
return !bjdata || parse_ubjson_typed_run<std::uint16_t>(remaining);
case 'm':
return !bjdata || parse_ubjson_typed_run<std::uint32_t>(remaining);
case 'M':
return !bjdata || parse_ubjson_typed_run<std::uint64_t>(remaining);
case 'd':
return parse_ubjson_typed_run<float>(remaining);
case 'D':
return parse_ubjson_typed_run<double>(remaining);
default:
return true;
}
}
/// input that is not contiguous: every element is read by get_ubjson_value
bool parse_ubjson_typed_run(std::size_t& /*remaining*/, const char_int_type /*type*/, std::false_type /*bulk*/) const noexcept
{
return true;
}
template<typename NumberType>
bool parse_ubjson_typed_run(std::size_t& remaining)
{
constexpr std::size_t size = sizeof(NumberType);
const std::size_t count = (std::min)(remaining, ia.bulk_remaining() / size);
if (count == 0)
{
return true;
}
const auto* const data = reinterpret_cast<const unsigned char*>(ia.bulk_data());
// UBJSON stores numbers big-endian, BJData little-endian
const bool swap = is_little_endian != (input_format == input_format_t::bjdata);
std::size_t read = 0;
bool accepted = true;
while (read < count && accepted)
{
NumberType number{};
std::memcpy(&number, data + (read * size), size);
if (swap)
{
byte_swap(number);
}
++read;
accepted = emit_ubjson_number(number);
}
// as if read by get_number(): current is the last byte read
ia.bulk_skip(read * size);
chars_read += read * size;
current = data[(read * size) - 1];
remaining -= read;
return accepted;
}
template<typename NumberType, enable_if_t<std::is_floating_point<NumberType>::value, int> = 0>
bool emit_ubjson_number(const NumberType number)
{
return emit_float(number);
}
template < typename NumberType, enable_if_t < std::is_integral<NumberType>::value&& std::is_signed<NumberType>::value, int > = 0 >
bool emit_ubjson_number(const NumberType number)
{
return emit_signed(number);
}
template < typename NumberType, enable_if_t < std::is_integral<NumberType>::value&& !std::is_signed<NumberType>::value, int > = 0 >
bool emit_ubjson_number(const NumberType number)
{
return emit_unsigned(number);
}
/*!
@return whether array creation completed
*/
@@ -3682,15 +3290,7 @@ class binary_reader
return false;
}
// the numbers that are in the input, in one loop; the rest (if
// the input ends early) one by one below
std::size_t remaining = size_and_type.first;
if (!nd_is_char && JSON_HEDLEY_UNLIKELY(!parse_ubjson_typed_run(remaining, size_and_type.second, std::integral_constant<bool, bulk_scan> {})))
{
return false;
}
for (std::size_t i = size_and_type.first - remaining; i < size_and_type.first; ++i)
for (std::size_t i = 0; i < size_and_type.first; ++i)
{
if (nd_is_char)
{
@@ -3750,8 +3350,7 @@ class binary_reader
container_stack.back().remaining = 0;
}
return size_and_type.second == 0
|| parse_ubjson_typed_run(container_stack.back().remaining, size_and_type.second, std::integral_constant<bool, bulk_scan> {});
return true;
}
return enter_array(detail::unknown_size());
@@ -4092,13 +3691,13 @@ class binary_reader
return sax->null();
case 0xFB:
return sax->number_float(static_cast<number_float_t>(-1.0), no_source);
return sax->number_float(static_cast<number_float_t>(-1.0), "");
case 0xFC:
return sax->number_float(static_cast<number_float_t>(0.0), no_source);
return sax->number_float(static_cast<number_float_t>(0.0), "");
case 0xFD:
return sax->number_float(static_cast<number_float_t>(1.0), no_source);
return sax->number_float(static_cast<number_float_t>(1.0), "");
case 0xFF: // empty string
{
@@ -4467,27 +4066,6 @@ class binary_reader
return current;
}
/// reverse the byte order of a double with a single byteswap(): GCC does
/// not merge the byte-wise swap of the generic version into one
static void byte_swap(double& number) noexcept
{
static_assert(sizeof(double) == sizeof(std::uint64_t), "double must have 64 bits");
std::uint64_t bits{};
std::memcpy(&bits, &number, sizeof(bits));
bits = byteswap(bits);
std::memcpy(&number, &bits, sizeof(bits));
}
/// reverse the byte order of a float with a single byteswap()
static void byte_swap(float& number) noexcept
{
static_assert(sizeof(float) == sizeof(std::uint32_t), "float must have 32 bits");
std::uint32_t bits{};
std::memcpy(&bits, &number, sizeof(bits));
bits = byteswap(bits);
std::memcpy(&number, &bits, sizeof(bits));
}
template<class NumberType>
static void byte_swap(NumberType& number)
{
@@ -4623,7 +4201,7 @@ class binary_reader
{
return out_of_range_error(exception_id::number_overflow, "number overflow", "value");
}
return sax->number_float(result, no_source);
return sax->number_float(result, "");
}
/*!
@@ -4683,7 +4261,7 @@ class binary_reader
}();
return sax->number_float((half & 0x8000u) != 0
? static_cast<number_float_t>(-val)
: static_cast<number_float_t>(val), no_source);
: static_cast<number_float_t>(val), "");
}
/*!
@@ -5008,10 +4586,6 @@ class binary_reader
/// the SAX parser
json_sax_t* sax = nullptr;
/// the (empty) source text of every number_float event: binary formats
/// have none, and a string_t built for each float would cost more
const string_t no_source{};
/// the containers that have been opened and not closed yet; see @ref container_frame
std::vector<container_frame> container_stack{};
+252 -679
View File
File diff suppressed because it is too large. Load diff
-75
View File
@@ -26,14 +26,11 @@ using nlohmann::ordered_json;
#include <limits>
#include <iostream>
#include <fstream>
#include <list>
#include <set>
#include "make_test_data_available.hpp"
#include "test_utils.hpp"
#include "sax_countdown.hpp"
#include "sax_event_loggers.hpp"
using utils::SaxCountdown;
using utils::SaxEventLogger;
// trait_test_arg and the "value_in_range_of trait" TEST_CASE_TEMPLATE_DEFINE
@@ -4749,75 +4746,3 @@ TEST_CASE("BJData optimized container type marker is validated for count 0")
CHECK(json::from_bjdata(vb) == json::binary({}));
}
}
TEST_CASE("BJData optimized arrays from contiguous and non-contiguous input")
{
// contiguous input reads the numbers of an array with a type marker in
// one loop, other input number by number; both must give the same events,
// value, or error for the complete document and for every truncation of
// it, and stop at every event a SAX handler rejects
json document =
{
{"u8", {0, 200, 255}},
{"i8", {-5, 100, -128}},
{"i16", {-30000, 1000, 30000}},
{"i32", {-2000000000, 5, 70000}},
{"i64", {-9000000000000000000LL, 1, 5000000000LL}},
{"f64", {1.5, -0.1, 2.5}},
{"pairs", {{1.25, -3.75}, {1.25, -3.75}, json::array()}},
{"mixed", {1, "a", 2.5, nullptr}},
{"u16", {40000, 1, 65535}},
{"u32", {4000000000U, 1, 70000}},
{"u64", {18446744073709551615ULL, 1, 5000000000ULL}},
{"nd", {{"_ArrayType_", "double"}, {"_ArraySize_", {2, 2}}, {"_ArrayData_", {1.5, -2.5, 3.5, -4.5}}}},
{"nd8", {{"_ArrayType_", "uint8"}, {"_ArraySize_", {3}}, {"_ArrayData_", {1, 2, 3}}}},
};
const std::vector<std::uint8_t> encoded = json::to_bjdata(document, true, true);
CHECK(json::from_bjdata(encoded) == document);
for (std::size_t length = 0; length <= encoded.size(); ++length)
{
CAPTURE(length)
const std::vector<std::uint8_t> contiguous(encoded.begin(), encoded.begin() + static_cast<std::ptrdiff_t>(length));
const std::list<std::uint8_t> bytewise(contiguous.begin(), contiguous.end());
CHECK(json::from_bjdata(contiguous, true, false).dump() == json::from_bjdata(bytewise.begin(), bytewise.end(), true, false).dump());
#if !defined(JSON_NOEXCEPTION)
std::string contiguous_error;
std::string bytewise_error;
try
{
static_cast<void>(json::from_bjdata(contiguous));
}
catch (const json::exception& e)
{
contiguous_error = e.what();
}
try
{
static_cast<void>(json::from_bjdata(bytewise.begin(), bytewise.end()));
}
catch (const json::exception& e)
{
bytewise_error = e.what();
}
CHECK(contiguous_error == bytewise_error);
#endif
SaxEventLogger from_contiguous;
SaxEventLogger from_bytewise;
CHECK(json::sax_parse(contiguous, &from_contiguous, json::input_format_t::bjdata)
== json::sax_parse(bytewise.begin(), bytewise.end(), &from_bytewise, json::input_format_t::bjdata));
CHECK(from_contiguous.events == from_bytewise.events);
}
for (int events = 0; events <= 80; ++events)
{
CAPTURE(events)
const std::list<std::uint8_t> bytewise(encoded.begin(), encoded.end());
SaxCountdown from_contiguous(events);
SaxCountdown from_bytewise(events);
CHECK(json::sax_parse(encoded, &from_contiguous, json::input_format_t::bjdata)
== json::sax_parse(bytewise.begin(), bytewise.end(), &from_bytewise, json::input_format_t::bjdata));
}
}
-74
View File
@@ -29,9 +29,7 @@ using nlohmann::json;
#include "round_trip_corpus.hpp"
#include "test_utils.hpp"
#include "sax_countdown.hpp"
#include "sax_event_loggers.hpp"
using utils::SaxCountdown;
using utils::SaxEventLogger;
TEST_CASE("CBOR")
@@ -2448,78 +2446,6 @@ TEST_CASE("CBOR SAX parsing stops at every event")
CHECK(count_events(std::vector<std::uint8_t>({0xBF, 0x61, 'a', 0x9F, 0x01, 0xFF, 0xFF})) == 6);
}
TEST_CASE("CBOR arrays of scalars from contiguous and non-contiguous input")
{
// contiguous input reads the integers, floats, booleans, and nulls an
// array begins with in one loop, other input value by value; both must
// give the same events, value, or error for the complete document and for
// every truncation of it, and stop at every event a SAX handler rejects
const std::vector<std::uint8_t> cbor =
{
0xA3, // map(3)
0x61, 'a', // "a"
0x98, 0x16, // array(22)
0x00, 0x17, 0x18, 0xFF, 0x19, 0x01, 0x00, // 0, 23, 255, 256
0x1A, 0x00, 0x01, 0x00, 0x00, // 65536
0x1B, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 18446744073709551615
0x20, 0x37, 0x38, 0xFF, 0x39, 0x01, 0x00, // -1, -24, -256, -257
0x3A, 0x00, 0x01, 0x00, 0x00, // -65537
0x3B, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // -18446744073709551616 (a float)
0x3B, 0x7F, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // -9223372036854775808
0xF4, 0xF5, 0xF6, // false, true, null
0xFA, 0x3F, 0xC0, 0x00, 0x00, // 1.5 (float32)
0xFB, 0xBF, 0xB9, 0x99, 0x99, 0x99, 0x99, 0x99, 0x9A, // -0.1
0xF9, 0x3C, 0x00, // 1.0 (float16, ends the loop)
0x01, 0x61, 'a', // 1, "a"
0xFB, 0x40, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // 2.5
0x61, 'b', // "b"
0x83, // array(3)
0x82, 0xFB, 0x3F, 0xF4, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // [1.25,
0xFB, 0xC0, 0x0E, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // -3.75]
0x82, 0xFB, 0x3F, 0xF4, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // [1.25,
0xFB, 0xC0, 0x0E, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // -3.75]
0x80, // []
0x61, 'c', // "c"
0x84, 0xA0, 0x01, 0xC1, 0x02, 0x81, 0x03 // [{}, 1, tag(1) 2, [3]]
};
const auto tags = json::cbor_tag_handler_t::ignore;
const json j = json::from_cbor(cbor, true, true, tags);
CHECK(j["a"].size() == 22);
CHECK(j["a"][5] == 18446744073709551615u);
CHECK(j["a"][11].is_number_float());
CHECK(j["a"][11] == -18446744073709551616.0);
CHECK(j["a"][12] == (std::numeric_limits<std::int64_t>::min)());
CHECK(j["a"][17] == -0.1);
CHECK(j["b"] == json::parse("[[1.25, -3.75], [1.25, -3.75], []]"));
CHECK(j["c"] == json::parse("[{}, 1, 2, [3]]"));
for (std::size_t length = 0; length <= cbor.size(); ++length)
{
CAPTURE(length)
const std::vector<std::uint8_t> contiguous(cbor.begin(), cbor.begin() + static_cast<std::ptrdiff_t>(length));
const std::list<std::uint8_t> bytewise(contiguous.begin(), contiguous.end());
CHECK(json::from_cbor(contiguous, true, false, tags).dump() == json::from_cbor(bytewise.begin(), bytewise.end(), true, false, tags).dump());
SaxEventLogger from_contiguous;
SaxEventLogger from_bytewise;
CHECK(json::sax_parse(contiguous, &from_contiguous, json::input_format_t::cbor, true, false, false, tags)
== json::sax_parse(bytewise.begin(), bytewise.end(), &from_bytewise, json::input_format_t::cbor, true, false, false, tags));
CHECK(from_contiguous.events == from_bytewise.events);
}
for (int events = 0; events <= 60; ++events)
{
CAPTURE(events)
const std::list<std::uint8_t> bytewise(cbor.begin(), cbor.end());
SaxCountdown from_contiguous(events);
SaxCountdown from_bytewise(events);
CHECK(json::sax_parse(cbor, &from_contiguous, json::input_format_t::cbor, true, false, false, tags)
== json::sax_parse(bytewise.begin(), bytewise.end(), &from_bytewise, json::input_format_t::cbor, true, false, false, tags));
}
}
TEST_CASE("CBOR indefinite-length strings do not recurse per chunk")
{
// Reading an indefinite-length string or byte array used to call itself
-97
View File
@@ -27,15 +27,12 @@ using nlohmann::json;
#include <sstream>
#include <iomanip>
#include <limits>
#include <list>
#include <set>
#include "make_test_data_available.hpp"
#include "round_trip_corpus.hpp"
#include "test_utils.hpp"
#include "sax_countdown.hpp"
#include "sax_event_loggers.hpp"
using utils::SaxCountdown;
using utils::SaxEventLogger;
TEST_CASE("MessagePack")
@@ -2215,97 +2212,3 @@ TEST_CASE("MessagePack large strings and binaries (chunked reader)")
}
}
}
TEST_CASE("MessagePack arrays of scalars from contiguous and non-contiguous input")
{
// contiguous input reads the integers, floats, booleans, and nils an
// array begins with in one loop, other input value by value; both must
// give the same events, value, or error for the complete document and for
// every truncation of it, and stop at every event a SAX handler rejects
const std::vector<std::uint8_t> msgpack =
{
0x83, // map(3)
0xA1, 'a', // "a"
0xDC, 0x00, 0x16, // array 16 (22)
0x00, 0x7F, 0xCC, 0xFF, 0xCD, 0x01, 0x00, // 0, 127, 255, 256
0xCE, 0x00, 0x01, 0x00, 0x00, // 65536
0xCF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 18446744073709551615
0xFF, 0xE0, 0xD0, 0x80, 0xD1, 0xFF, 0x00, // -1, -32, -128, -256
0xD2, 0xFF, 0xFE, 0xFF, 0xFF, // -65537
0xD3, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // -9223372036854775808
0xC2, 0xC3, 0xC0, // false, true, nil
0xCA, 0x3F, 0xC0, 0x00, 0x00, // 1.5 (float 32)
0xCB, 0xBF, 0xB9, 0x99, 0x99, 0x99, 0x99, 0x99, 0x9A, // -0.1
0xA1, 'x', // "x" (ends the loop)
0x01, // 1
0xCB, 0x40, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // 2.5
0xD9, 0x01, 'y', // "y" (str 8)
0xC0, // nil
0xA1, 'b', // "b"
0x93, // fixarray(3)
0x92, 0xCB, 0x3F, 0xF4, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // [1.25,
0xCB, 0xC0, 0x0E, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // -3.75]
0x92, 0xCB, 0x3F, 0xF4, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // [1.25,
0xCB, 0xC0, 0x0E, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // -3.75]
0x90, // []
0xA1, 'c', // "c"
0x94, 0x80, 0x01, 0xC4, 0x01, 0x05, 0x91, 0x03 // [{}, 1, bin 8, [3]]
};
const json j = json::from_msgpack(msgpack);
CHECK(j["a"].size() == 22);
CHECK(j["a"][5] == 18446744073709551615u);
CHECK(j["a"][7] == -32);
CHECK(j["a"][11] == (std::numeric_limits<std::int64_t>::min)());
CHECK(j["a"][15] == 1.5);
CHECK(j["a"][16] == -0.1);
CHECK(j["a"][17] == "x");
CHECK(j["b"] == json::parse("[[1.25, -3.75], [1.25, -3.75], []]"));
CHECK(j["c"][2].is_binary());
for (std::size_t length = 0; length <= msgpack.size(); ++length)
{
CAPTURE(length)
const std::vector<std::uint8_t> contiguous(msgpack.begin(), msgpack.begin() + static_cast<std::ptrdiff_t>(length));
const std::list<std::uint8_t> bytewise(contiguous.begin(), contiguous.end());
CHECK(json::from_msgpack(contiguous, true, false).dump() == json::from_msgpack(bytewise.begin(), bytewise.end(), true, false).dump());
#if !defined(JSON_NOEXCEPTION)
std::string contiguous_error;
std::string bytewise_error;
try
{
static_cast<void>(json::from_msgpack(contiguous));
}
catch (const json::parse_error& e)
{
contiguous_error = e.what();
}
try
{
static_cast<void>(json::from_msgpack(bytewise.begin(), bytewise.end()));
}
catch (const json::parse_error& e)
{
bytewise_error = e.what();
}
CHECK(contiguous_error == bytewise_error);
#endif
SaxEventLogger from_contiguous;
SaxEventLogger from_bytewise;
CHECK(json::sax_parse(contiguous, &from_contiguous, json::input_format_t::msgpack)
== json::sax_parse(bytewise.begin(), bytewise.end(), &from_bytewise, json::input_format_t::msgpack));
CHECK(from_contiguous.events == from_bytewise.events);
}
for (int events = 0; events <= 60; ++events)
{
CAPTURE(events)
const std::list<std::uint8_t> bytewise(msgpack.begin(), msgpack.end());
SaxCountdown from_contiguous(events);
SaxCountdown from_bytewise(events);
CHECK(json::sax_parse(msgpack, &from_contiguous, json::input_format_t::msgpack)
== json::sax_parse(bytewise.begin(), bytewise.end(), &from_bytewise, json::input_format_t::msgpack));
}
}
-70
View File
@@ -21,15 +21,12 @@ using nlohmann::json;
#include <iostream>
#include <fstream>
#include <list>
#include <set>
#include "make_test_data_available.hpp"
#include "round_trip_corpus.hpp"
#include "test_utils.hpp"
#include "sax_countdown.hpp"
#include "sax_event_loggers.hpp"
using utils::SaxCountdown;
using utils::SaxEventLogger;
TEST_CASE("UBJSON")
@@ -3403,70 +3400,3 @@ TEST_CASE("UBJSON optimized container type marker is validated for count 0")
}
}
}
TEST_CASE("UBJSON optimized arrays from contiguous and non-contiguous input")
{
// contiguous input reads the numbers of an array with a type marker in
// one loop, other input number by number; both must give the same events,
// value, or error for the complete document and for every truncation of
// it, and stop at every event a SAX handler rejects
json document =
{
{"u8", {0, 200, 255}},
{"i8", {-5, 100, -128}},
{"i16", {-30000, 1000, 30000}},
{"i32", {-2000000000, 5, 70000}},
{"i64", {-9000000000000000000LL, 1, 5000000000LL}},
{"f64", {1.5, -0.1, 2.5}},
{"pairs", {{1.25, -3.75}, {1.25, -3.75}, json::array()}},
{"mixed", {1, "a", 2.5, nullptr}},
};
const std::vector<std::uint8_t> encoded = json::to_ubjson(document, true, true);
CHECK(json::from_ubjson(encoded) == document);
for (std::size_t length = 0; length <= encoded.size(); ++length)
{
CAPTURE(length)
const std::vector<std::uint8_t> contiguous(encoded.begin(), encoded.begin() + static_cast<std::ptrdiff_t>(length));
const std::list<std::uint8_t> bytewise(contiguous.begin(), contiguous.end());
CHECK(json::from_ubjson(contiguous, true, false).dump() == json::from_ubjson(bytewise.begin(), bytewise.end(), true, false).dump());
#if !defined(JSON_NOEXCEPTION)
std::string contiguous_error;
std::string bytewise_error;
try
{
static_cast<void>(json::from_ubjson(contiguous));
}
catch (const json::exception& e)
{
contiguous_error = e.what();
}
try
{
static_cast<void>(json::from_ubjson(bytewise.begin(), bytewise.end()));
}
catch (const json::exception& e)
{
bytewise_error = e.what();
}
CHECK(contiguous_error == bytewise_error);
#endif
SaxEventLogger from_contiguous;
SaxEventLogger from_bytewise;
CHECK(json::sax_parse(contiguous, &from_contiguous, json::input_format_t::ubjson)
== json::sax_parse(bytewise.begin(), bytewise.end(), &from_bytewise, json::input_format_t::ubjson));
CHECK(from_contiguous.events == from_bytewise.events);
}
for (int events = 0; events <= 80; ++events)
{
CAPTURE(events)
const std::list<std::uint8_t> bytewise(encoded.begin(), encoded.end());
SaxCountdown from_contiguous(events);
SaxCountdown from_bytewise(events);
CHECK(json::sax_parse(encoded, &from_contiguous, json::input_format_t::ubjson)
== json::sax_parse(bytewise.begin(), bytewise.end(), &from_bytewise, json::input_format_t::ubjson));
}
}