mirror of
https://github.com/nlohmann/json.git
synced 2026-10-31 09:55:42 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
 Claude
|
675f914725 |
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?
|
||||
```
|
||||
@@ -2088,8 +2088,7 @@ class binary_writer
|
||||
/// arrays: the first index with one more digit; only maintained by
|
||||
/// @ref calc_bson_sizes
|
||||
std::uint64_t index_boundary = 10;
|
||||
/// @ref calc_bson_sizes: where its size goes in the table; @ref
|
||||
/// write_bson_document_single_pass: the position of its size field
|
||||
/// @ref calc_bson_sizes only: where its size goes in the table
|
||||
std::size_t size_slot;
|
||||
/// @ref calc_bson_sizes only: the size of its entries seen so far
|
||||
std::size_t entries_size = 0;
|
||||
@@ -2340,197 +2339,6 @@ class binary_writer
|
||||
}
|
||||
}
|
||||
|
||||
template<typename SinkType>
|
||||
using sink_written_t = decltype(std::declval<const SinkType&>().written());
|
||||
|
||||
/*!
|
||||
@brief whether BSON documents can be written in a single pass: the sink
|
||||
can overwrite what it has written (output_vector_sink), and every
|
||||
integer fits the 64 bits BSON has, so that writing an array of
|
||||
numbers needs no range check
|
||||
*/
|
||||
using bson_single_pass_sink = std::integral_constant < bool,
|
||||
is_detected<sink_written_t, OutputSinkType>::value
|
||||
&& std::numeric_limits<typename BasicJsonType::number_integer_t>::is_specialized
|
||||
&& std::numeric_limits<typename BasicJsonType::number_integer_t>::digits <= std::numeric_limits<std::int64_t>::digits
|
||||
&& std::numeric_limits<typename BasicJsonType::number_unsigned_t>::is_specialized
|
||||
&& std::numeric_limits<typename BasicJsonType::number_unsigned_t>::digits <= std::numeric_limits<std::uint64_t>::digits >;
|
||||
|
||||
/// write a placeholder for the size of an object or array whose entries
|
||||
/// follow; @return its position, for @ref patch_bson_size
|
||||
std::size_t write_bson_size_placeholder()
|
||||
{
|
||||
const std::size_t pos = oa.written();
|
||||
write_number<std::int32_t>(0, true);
|
||||
return pos;
|
||||
}
|
||||
|
||||
/// end the object or array whose size is at @a pos and write its size
|
||||
/// there: everything from that field to the terminating null byte
|
||||
/// @return false (and the size is not written) if it does not fit
|
||||
bool close_bson_container(const std::size_t pos)
|
||||
{
|
||||
oa.write_character(to_char_type(0x00));
|
||||
const std::size_t size = oa.written() - pos;
|
||||
if (JSON_HEDLEY_UNLIKELY(!value_in_range_of<std::int32_t>(size)))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
std::array<CharType, sizeof(std::int32_t)> bytes{};
|
||||
store_little_endian<std::int32_t>(bytes.data(), static_cast<std::int32_t>(size));
|
||||
oa.overwrite(pos, bytes.data(), bytes.size());
|
||||
return true;
|
||||
}
|
||||
|
||||
/*!
|
||||
@brief check the value @a j of a BSON entry as @ref calc_bson_value_size
|
||||
does, for @ref write_bson_document_single_pass
|
||||
@return at least the size of the value
|
||||
A number, boolean, or null needs no check there (its integers have at most
|
||||
64 bits) and has at most 8 characters; a discarded value is rejected with
|
||||
the same exception when it is written.
|
||||
*/
|
||||
std::size_t check_bson_value(const BasicJsonType& j)
|
||||
{
|
||||
return (j.is_string() || j.is_binary()) ? calc_bson_value_size(j) : 8;
|
||||
}
|
||||
|
||||
/// whether an entry of @a entry_size characters keeps the document that
|
||||
/// begins at @a start, closed by @a open_containers null bytes, within
|
||||
/// the 32-bit size of BSON
|
||||
bool bson_entry_fits(const std::size_t start, const std::size_t entry_size, const std::size_t open_containers) const noexcept
|
||||
{
|
||||
const std::size_t limit = static_cast<std::size_t>((std::numeric_limits<std::int32_t>::max)());
|
||||
const std::size_t written = oa.written() - start;
|
||||
return written <= limit && entry_size <= limit - written && open_containers <= limit - written - entry_size;
|
||||
}
|
||||
|
||||
/*!
|
||||
@brief Serializes the JSON object @a document as a BSON document in a
|
||||
single pass, writing the size of each object and array once it is
|
||||
complete
|
||||
|
||||
Checks every key and value as @ref calc_bson_sizes does, in the same
|
||||
order, so the same exception is thrown; the output written up to then is
|
||||
discarded with the vector it was written to.
|
||||
|
||||
@return false as soon as the document or a string, binary value, object,
|
||||
or array in it would be larger than the 32-bit BSON sizes allow;
|
||||
nothing large has been written then, and the caller writes the
|
||||
document in two passes, which reports that error
|
||||
*/
|
||||
template < typename SinkType = OutputSinkType,
|
||||
typename std::enable_if < is_detected<sink_written_t, SinkType>::value, int >::type = 0 >
|
||||
bool write_bson_document_single_pass(const BasicJsonType& document)
|
||||
{
|
||||
const std::size_t start = oa.written();
|
||||
// the object or array whose entries are being written (with the
|
||||
// position of its size), and the ones it is in
|
||||
bson_frame current(&document, write_bson_size_placeholder());
|
||||
std::vector<bson_frame> parents;
|
||||
|
||||
while (true)
|
||||
{
|
||||
const BasicJsonType* nested = nullptr;
|
||||
std::size_t nested_size_pos = 0;
|
||||
if (current.value->is_object())
|
||||
{
|
||||
const auto& object = *current.value->m_data.m_value.object;
|
||||
while (nested == nullptr && current.member != object.cend())
|
||||
{
|
||||
const auto& el = *current.member;
|
||||
++current.member;
|
||||
const std::size_t header_size = calc_bson_entry_header_size(el.first, el.second);
|
||||
const std::size_t value_size = el.second.is_structured() ? 0 : check_bson_value(el.second);
|
||||
if (JSON_HEDLEY_UNLIKELY(!bson_entry_fits(start, header_size + value_size, parents.size() + 1)))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
if (el.second.is_structured())
|
||||
{
|
||||
write_bson_entry_header(el.first, el.second.is_object() ? 0x03 : 0x04);
|
||||
nested_size_pos = write_bson_size_placeholder();
|
||||
nested = &el.second;
|
||||
}
|
||||
else
|
||||
{
|
||||
write_bson_value(el.first, el.second);
|
||||
}
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
const auto& array = *current.value->m_data.m_value.array;
|
||||
while (nested == nullptr && current.index < array.size())
|
||||
{
|
||||
if (is_bson_fixed_size(array[current.index]))
|
||||
{
|
||||
write_bson_array_fixed_entries(current);
|
||||
if (current.index == array.size())
|
||||
{
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// a string, binary value, object or array
|
||||
const BasicJsonType& el = array[current.index];
|
||||
const string_t index_name(current.index_name.data(), current.index_name_size);
|
||||
current.next_index();
|
||||
const std::size_t value_size = el.is_structured() ? 0 : check_bson_value(el);
|
||||
if (JSON_HEDLEY_UNLIKELY(!bson_entry_fits(start, 2 + index_name.size() + value_size, parents.size() + 1)))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
if (el.is_structured())
|
||||
{
|
||||
write_bson_entry_header(index_name, el.is_object() ? 0x03 : 0x04);
|
||||
nested_size_pos = write_bson_size_placeholder();
|
||||
nested = ⪙
|
||||
}
|
||||
else
|
||||
{
|
||||
write_bson_value(index_name, el);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (nested != nullptr)
|
||||
{
|
||||
if (nested->is_array())
|
||||
{
|
||||
// an array of booleans, numbers and null is written without a frame on the stack
|
||||
bson_frame leaf(nested, nested_size_pos);
|
||||
write_bson_array_fixed_entries(leaf);
|
||||
if (leaf.index == nested->m_data.m_value.array->size())
|
||||
{
|
||||
if (JSON_HEDLEY_UNLIKELY(!close_bson_container(nested_size_pos)))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
parents.push_back(std::move(current));
|
||||
current = std::move(leaf);
|
||||
continue;
|
||||
}
|
||||
parents.push_back(std::move(current));
|
||||
current = bson_frame(nested, nested_size_pos);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (JSON_HEDLEY_UNLIKELY(!close_bson_container(current.size_slot)))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
if (parents.empty())
|
||||
{
|
||||
return true;
|
||||
}
|
||||
current = std::move(parents.back());
|
||||
parents.pop_back();
|
||||
}
|
||||
}
|
||||
|
||||
/*!
|
||||
@brief Serializes the JSON object @a document as a BSON document
|
||||
|
||||
@@ -2542,39 +2350,6 @@ class binary_writer
|
||||
@pre document.type() == value_t::object
|
||||
*/
|
||||
void write_bson_document(const BasicJsonType& document)
|
||||
{
|
||||
write_bson_document(document, bson_single_pass_sink());
|
||||
}
|
||||
|
||||
/// the sink can overwrite: one pass, unless the error handler replaces or
|
||||
/// drops ill-formed UTF-8, which changes the sizes the checks compute
|
||||
void write_bson_document(const BasicJsonType& document, std::true_type /*single_pass*/)
|
||||
{
|
||||
const std::size_t start = oa.written();
|
||||
if ((error_handler == error_handler_t::keep || error_handler == error_handler_t::strict)
|
||||
&& write_bson_document_single_pass(document))
|
||||
{
|
||||
return;
|
||||
}
|
||||
// a length does not fit: start over, so that it is reported as the
|
||||
// two passes report it, before anything is written
|
||||
oa.rewind(start);
|
||||
write_bson_document_two_pass(document);
|
||||
}
|
||||
|
||||
void write_bson_document(const BasicJsonType& document, std::false_type /*single_pass*/)
|
||||
{
|
||||
write_bson_document_two_pass(document);
|
||||
}
|
||||
|
||||
/*!
|
||||
@brief Serializes the JSON object @a document as a BSON document: the
|
||||
sizes of all objects and arrays first, then the entries
|
||||
|
||||
@param[in] document the JSON object to serialize
|
||||
@pre document.type() == value_t::object
|
||||
*/
|
||||
void write_bson_document_two_pass(const BasicJsonType& document)
|
||||
{
|
||||
std::vector<std::size_t> nested_sizes;
|
||||
const std::size_t document_size = calc_bson_sizes(document, nested_sizes);
|
||||
|
||||
@@ -227,29 +227,6 @@ class output_vector_sink
|
||||
void flush() noexcept
|
||||
{}
|
||||
|
||||
/// the number of characters written so far, the position of the next one
|
||||
std::size_t written() const noexcept
|
||||
{
|
||||
return position();
|
||||
}
|
||||
|
||||
/// @brief replace @a length characters written before, from position @a pos
|
||||
///
|
||||
/// Lets a writer put a length in front of data it has written since, such
|
||||
/// as the size of a BSON document, without computing it in advance.
|
||||
void overwrite(const std::size_t pos, const CharType* s, const std::size_t length) noexcept
|
||||
{
|
||||
JSON_ASSERT(pos + length <= position());
|
||||
std::memcpy(v->data() + pos, s, length * sizeof(CharType));
|
||||
}
|
||||
|
||||
/// drop the characters written from position @a pos on
|
||||
void rewind(const std::size_t pos) noexcept
|
||||
{
|
||||
JSON_ASSERT(pos <= position());
|
||||
cursor = v->data() + pos;
|
||||
}
|
||||
|
||||
private:
|
||||
/// number of characters of *v written so far
|
||||
std::size_t position() const noexcept
|
||||
|
||||
Generated
+1
-249
@@ -26870,29 +26870,6 @@ class output_vector_sink
|
||||
void flush() noexcept
|
||||
{}
|
||||
|
||||
/// the number of characters written so far, the position of the next one
|
||||
std::size_t written() const noexcept
|
||||
{
|
||||
return position();
|
||||
}
|
||||
|
||||
/// @brief replace @a length characters written before, from position @a pos
|
||||
///
|
||||
/// Lets a writer put a length in front of data it has written since, such
|
||||
/// as the size of a BSON document, without computing it in advance.
|
||||
void overwrite(const std::size_t pos, const CharType* s, const std::size_t length) noexcept
|
||||
{
|
||||
JSON_ASSERT(pos + length <= position());
|
||||
std::memcpy(v->data() + pos, s, length * sizeof(CharType));
|
||||
}
|
||||
|
||||
/// drop the characters written from position @a pos on
|
||||
void rewind(const std::size_t pos) noexcept
|
||||
{
|
||||
JSON_ASSERT(pos <= position());
|
||||
cursor = v->data() + pos;
|
||||
}
|
||||
|
||||
private:
|
||||
/// number of characters of *v written so far
|
||||
std::size_t position() const noexcept
|
||||
@@ -29337,8 +29314,7 @@ class binary_writer
|
||||
/// arrays: the first index with one more digit; only maintained by
|
||||
/// @ref calc_bson_sizes
|
||||
std::uint64_t index_boundary = 10;
|
||||
/// @ref calc_bson_sizes: where its size goes in the table; @ref
|
||||
/// write_bson_document_single_pass: the position of its size field
|
||||
/// @ref calc_bson_sizes only: where its size goes in the table
|
||||
std::size_t size_slot;
|
||||
/// @ref calc_bson_sizes only: the size of its entries seen so far
|
||||
std::size_t entries_size = 0;
|
||||
@@ -29589,197 +29565,6 @@ class binary_writer
|
||||
}
|
||||
}
|
||||
|
||||
template<typename SinkType>
|
||||
using sink_written_t = decltype(std::declval<const SinkType&>().written());
|
||||
|
||||
/*!
|
||||
@brief whether BSON documents can be written in a single pass: the sink
|
||||
can overwrite what it has written (output_vector_sink), and every
|
||||
integer fits the 64 bits BSON has, so that writing an array of
|
||||
numbers needs no range check
|
||||
*/
|
||||
using bson_single_pass_sink = std::integral_constant < bool,
|
||||
is_detected<sink_written_t, OutputSinkType>::value
|
||||
&& std::numeric_limits<typename BasicJsonType::number_integer_t>::is_specialized
|
||||
&& std::numeric_limits<typename BasicJsonType::number_integer_t>::digits <= std::numeric_limits<std::int64_t>::digits
|
||||
&& std::numeric_limits<typename BasicJsonType::number_unsigned_t>::is_specialized
|
||||
&& std::numeric_limits<typename BasicJsonType::number_unsigned_t>::digits <= std::numeric_limits<std::uint64_t>::digits >;
|
||||
|
||||
/// write a placeholder for the size of an object or array whose entries
|
||||
/// follow; @return its position, for @ref patch_bson_size
|
||||
std::size_t write_bson_size_placeholder()
|
||||
{
|
||||
const std::size_t pos = oa.written();
|
||||
write_number<std::int32_t>(0, true);
|
||||
return pos;
|
||||
}
|
||||
|
||||
/// end the object or array whose size is at @a pos and write its size
|
||||
/// there: everything from that field to the terminating null byte
|
||||
/// @return false (and the size is not written) if it does not fit
|
||||
bool close_bson_container(const std::size_t pos)
|
||||
{
|
||||
oa.write_character(to_char_type(0x00));
|
||||
const std::size_t size = oa.written() - pos;
|
||||
if (JSON_HEDLEY_UNLIKELY(!value_in_range_of<std::int32_t>(size)))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
std::array<CharType, sizeof(std::int32_t)> bytes{};
|
||||
store_little_endian<std::int32_t>(bytes.data(), static_cast<std::int32_t>(size));
|
||||
oa.overwrite(pos, bytes.data(), bytes.size());
|
||||
return true;
|
||||
}
|
||||
|
||||
/*!
|
||||
@brief check the value @a j of a BSON entry as @ref calc_bson_value_size
|
||||
does, for @ref write_bson_document_single_pass
|
||||
@return at least the size of the value
|
||||
A number, boolean, or null needs no check there (its integers have at most
|
||||
64 bits) and has at most 8 characters; a discarded value is rejected with
|
||||
the same exception when it is written.
|
||||
*/
|
||||
std::size_t check_bson_value(const BasicJsonType& j)
|
||||
{
|
||||
return (j.is_string() || j.is_binary()) ? calc_bson_value_size(j) : 8;
|
||||
}
|
||||
|
||||
/// whether an entry of @a entry_size characters keeps the document that
|
||||
/// begins at @a start, closed by @a open_containers null bytes, within
|
||||
/// the 32-bit size of BSON
|
||||
bool bson_entry_fits(const std::size_t start, const std::size_t entry_size, const std::size_t open_containers) const noexcept
|
||||
{
|
||||
const std::size_t limit = static_cast<std::size_t>((std::numeric_limits<std::int32_t>::max)());
|
||||
const std::size_t written = oa.written() - start;
|
||||
return written <= limit && entry_size <= limit - written && open_containers <= limit - written - entry_size;
|
||||
}
|
||||
|
||||
/*!
|
||||
@brief Serializes the JSON object @a document as a BSON document in a
|
||||
single pass, writing the size of each object and array once it is
|
||||
complete
|
||||
|
||||
Checks every key and value as @ref calc_bson_sizes does, in the same
|
||||
order, so the same exception is thrown; the output written up to then is
|
||||
discarded with the vector it was written to.
|
||||
|
||||
@return false as soon as the document or a string, binary value, object,
|
||||
or array in it would be larger than the 32-bit BSON sizes allow;
|
||||
nothing large has been written then, and the caller writes the
|
||||
document in two passes, which reports that error
|
||||
*/
|
||||
template < typename SinkType = OutputSinkType,
|
||||
typename std::enable_if < is_detected<sink_written_t, SinkType>::value, int >::type = 0 >
|
||||
bool write_bson_document_single_pass(const BasicJsonType& document)
|
||||
{
|
||||
const std::size_t start = oa.written();
|
||||
// the object or array whose entries are being written (with the
|
||||
// position of its size), and the ones it is in
|
||||
bson_frame current(&document, write_bson_size_placeholder());
|
||||
std::vector<bson_frame> parents;
|
||||
|
||||
while (true)
|
||||
{
|
||||
const BasicJsonType* nested = nullptr;
|
||||
std::size_t nested_size_pos = 0;
|
||||
if (current.value->is_object())
|
||||
{
|
||||
const auto& object = *current.value->m_data.m_value.object;
|
||||
while (nested == nullptr && current.member != object.cend())
|
||||
{
|
||||
const auto& el = *current.member;
|
||||
++current.member;
|
||||
const std::size_t header_size = calc_bson_entry_header_size(el.first, el.second);
|
||||
const std::size_t value_size = el.second.is_structured() ? 0 : check_bson_value(el.second);
|
||||
if (JSON_HEDLEY_UNLIKELY(!bson_entry_fits(start, header_size + value_size, parents.size() + 1)))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
if (el.second.is_structured())
|
||||
{
|
||||
write_bson_entry_header(el.first, el.second.is_object() ? 0x03 : 0x04);
|
||||
nested_size_pos = write_bson_size_placeholder();
|
||||
nested = &el.second;
|
||||
}
|
||||
else
|
||||
{
|
||||
write_bson_value(el.first, el.second);
|
||||
}
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
const auto& array = *current.value->m_data.m_value.array;
|
||||
while (nested == nullptr && current.index < array.size())
|
||||
{
|
||||
if (is_bson_fixed_size(array[current.index]))
|
||||
{
|
||||
write_bson_array_fixed_entries(current);
|
||||
if (current.index == array.size())
|
||||
{
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// a string, binary value, object or array
|
||||
const BasicJsonType& el = array[current.index];
|
||||
const string_t index_name(current.index_name.data(), current.index_name_size);
|
||||
current.next_index();
|
||||
const std::size_t value_size = el.is_structured() ? 0 : check_bson_value(el);
|
||||
if (JSON_HEDLEY_UNLIKELY(!bson_entry_fits(start, 2 + index_name.size() + value_size, parents.size() + 1)))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
if (el.is_structured())
|
||||
{
|
||||
write_bson_entry_header(index_name, el.is_object() ? 0x03 : 0x04);
|
||||
nested_size_pos = write_bson_size_placeholder();
|
||||
nested = ⪙
|
||||
}
|
||||
else
|
||||
{
|
||||
write_bson_value(index_name, el);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (nested != nullptr)
|
||||
{
|
||||
if (nested->is_array())
|
||||
{
|
||||
// an array of booleans, numbers and null is written without a frame on the stack
|
||||
bson_frame leaf(nested, nested_size_pos);
|
||||
write_bson_array_fixed_entries(leaf);
|
||||
if (leaf.index == nested->m_data.m_value.array->size())
|
||||
{
|
||||
if (JSON_HEDLEY_UNLIKELY(!close_bson_container(nested_size_pos)))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
parents.push_back(std::move(current));
|
||||
current = std::move(leaf);
|
||||
continue;
|
||||
}
|
||||
parents.push_back(std::move(current));
|
||||
current = bson_frame(nested, nested_size_pos);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (JSON_HEDLEY_UNLIKELY(!close_bson_container(current.size_slot)))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
if (parents.empty())
|
||||
{
|
||||
return true;
|
||||
}
|
||||
current = std::move(parents.back());
|
||||
parents.pop_back();
|
||||
}
|
||||
}
|
||||
|
||||
/*!
|
||||
@brief Serializes the JSON object @a document as a BSON document
|
||||
|
||||
@@ -29791,39 +29576,6 @@ class binary_writer
|
||||
@pre document.type() == value_t::object
|
||||
*/
|
||||
void write_bson_document(const BasicJsonType& document)
|
||||
{
|
||||
write_bson_document(document, bson_single_pass_sink());
|
||||
}
|
||||
|
||||
/// the sink can overwrite: one pass, unless the error handler replaces or
|
||||
/// drops ill-formed UTF-8, which changes the sizes the checks compute
|
||||
void write_bson_document(const BasicJsonType& document, std::true_type /*single_pass*/)
|
||||
{
|
||||
const std::size_t start = oa.written();
|
||||
if ((error_handler == error_handler_t::keep || error_handler == error_handler_t::strict)
|
||||
&& write_bson_document_single_pass(document))
|
||||
{
|
||||
return;
|
||||
}
|
||||
// a length does not fit: start over, so that it is reported as the
|
||||
// two passes report it, before anything is written
|
||||
oa.rewind(start);
|
||||
write_bson_document_two_pass(document);
|
||||
}
|
||||
|
||||
void write_bson_document(const BasicJsonType& document, std::false_type /*single_pass*/)
|
||||
{
|
||||
write_bson_document_two_pass(document);
|
||||
}
|
||||
|
||||
/*!
|
||||
@brief Serializes the JSON object @a document as a BSON document: the
|
||||
sizes of all objects and arrays first, then the entries
|
||||
|
||||
@param[in] document the JSON object to serialize
|
||||
@pre document.type() == value_t::object
|
||||
*/
|
||||
void write_bson_document_two_pass(const BasicJsonType& document)
|
||||
{
|
||||
std::vector<std::size_t> nested_sizes;
|
||||
const std::size_t document_size = calc_bson_sizes(document, nested_sizes);
|
||||
|
||||
@@ -2267,132 +2267,3 @@ TEST_CASE("BSON large strings and binaries (chunked reader)")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
TEST_CASE("BSON written in one pass and in two passes")
|
||||
{
|
||||
// to_bson() into a returned vector writes each size once the object or
|
||||
// array is complete; through an output adapter, all sizes are computed
|
||||
// first. Both must produce the same bytes and throw the same exceptions.
|
||||
const auto two_pass = [](const json & j, const json::error_handler_t handler)
|
||||
{
|
||||
std::vector<std::uint8_t> result;
|
||||
json::to_bson(j, result, handler);
|
||||
return result;
|
||||
};
|
||||
|
||||
const auto check_same = [&](const json & j, const json::error_handler_t handler)
|
||||
{
|
||||
CAPTURE(j.dump(-1, ' ', true, json::error_handler_t::replace))
|
||||
CAPTURE(static_cast<int>(handler))
|
||||
#if !defined(JSON_NOEXCEPTION)
|
||||
std::string one_pass_error;
|
||||
std::string two_pass_error;
|
||||
std::vector<std::uint8_t> one;
|
||||
std::vector<std::uint8_t> two;
|
||||
try
|
||||
{
|
||||
one = json::to_bson(j, handler);
|
||||
}
|
||||
catch (const json::exception& e)
|
||||
{
|
||||
one_pass_error = e.what();
|
||||
}
|
||||
try
|
||||
{
|
||||
two = two_pass(j, handler);
|
||||
}
|
||||
catch (const json::exception& e)
|
||||
{
|
||||
two_pass_error = e.what();
|
||||
}
|
||||
CHECK(one_pass_error == two_pass_error);
|
||||
CHECK(one == two);
|
||||
#else
|
||||
CHECK(json::to_bson(j, handler) == two_pass(j, handler));
|
||||
#endif
|
||||
};
|
||||
|
||||
const json::error_handler_t handlers[] = {json::error_handler_t::keep, json::error_handler_t::strict,
|
||||
json::error_handler_t::replace, json::error_handler_t::ignore
|
||||
};
|
||||
|
||||
SECTION("corpus")
|
||||
{
|
||||
for (const auto& value : utils::round_trip_corpus::values())
|
||||
{
|
||||
const json document = value.is_object() ? value : json{{"value", value}};
|
||||
for (const auto handler : handlers)
|
||||
{
|
||||
check_same(document, handler);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
SECTION("nesting and arrays of every length class")
|
||||
{
|
||||
json document = json::object();
|
||||
document["empty object"] = json::object();
|
||||
document["empty array"] = json::array();
|
||||
for (const std::size_t n :
|
||||
{
|
||||
std::size_t(1), std::size_t(9), std::size_t(10), std::size_t(11), std::size_t(99), std::size_t(100), std::size_t(101), std::size_t(1000)
|
||||
})
|
||||
{
|
||||
json numbers = json::array();
|
||||
json mixed = json::array();
|
||||
for (std::size_t i = 0; i < n; ++i)
|
||||
{
|
||||
numbers.push_back(i % 3 == 0 ? json(static_cast<double>(i) / 3) : json(static_cast<std::int64_t>(i) * 1000003));
|
||||
mixed.push_back(i % 4 == 0 ? json("s" + std::to_string(i)) : i % 4 == 1 ? json::array({i, json::object({{"k", i}})}) : i % 4 == 2 ? json(nullptr) : json(i % 2 == 0));
|
||||
}
|
||||
document["numbers " + std::to_string(n)] = numbers;
|
||||
document["mixed " + std::to_string(n)] = mixed;
|
||||
document["nested " + std::to_string(n)] = json::object({{"a", numbers}, {"b", json::array({mixed, json::object()})}});
|
||||
}
|
||||
document["binary"] = json::binary({1, 2, 3}, 0x80);
|
||||
document["binary old"] = json::binary({1, 2, 3}, 0x02);
|
||||
for (const auto handler : handlers)
|
||||
{
|
||||
check_same(document, handler);
|
||||
}
|
||||
}
|
||||
|
||||
SECTION("errors at every position")
|
||||
{
|
||||
// an entry that cannot be written, at each place among valid ones
|
||||
const std::string bad_utf8 = "\xC3\x28";
|
||||
const json bad_entries[] =
|
||||
{
|
||||
json::object({{std::string("a\0b", 3), 1}}),
|
||||
json::object({{"s", bad_utf8}}),
|
||||
json::object({{bad_utf8, 1}}),
|
||||
json::array({1, 2, bad_utf8}),
|
||||
json::object({{"b", json::binary({1}, 0x100)}}),
|
||||
json::object({{"d", json(json::value_t::discarded)}}),
|
||||
json::array({json(json::value_t::discarded)}),
|
||||
};
|
||||
for (const auto& bad : bad_entries)
|
||||
{
|
||||
for (std::size_t pos = 0; pos < 4; ++pos)
|
||||
{
|
||||
json document = json::object();
|
||||
json list = json::array();
|
||||
for (std::size_t i = 0; i < 4; ++i)
|
||||
{
|
||||
if (i == pos)
|
||||
{
|
||||
document["x" + std::to_string(i)] = bad;
|
||||
list.push_back(bad);
|
||||
}
|
||||
document["n" + std::to_string(i)] = json::array({i, "str", json::object({{"k", i}})});
|
||||
list.push_back(i);
|
||||
}
|
||||
document["list"] = list;
|
||||
for (const auto handler : handlers)
|
||||
{
|
||||
check_same(document, handler);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user