Skip to content

Add iterator+sentinel tests and docs for binary deserializers - #5265

Merged
nlohmann merged 5 commits into
developfrom
claude/todo-183-plan-267c0c
Jul 11, 2026
Merged

nlohmann merged 5 commits into
developfrom
claude/todo-183-plan-267c0c

Conversation

@nlohmann

Copy link
Copy Markdown
Owner

Summary

This PR extends C++20 ranges support (iterator+sentinel pairs) to the binary format deserializers, matching the JSON text parsers. Users can now read binary files directly via std::istreambuf_iterator<char> + a sentinel without pre-buffering to a vector.

  • Adds istreambuf_sentinel helper to test utilities for stream EOF detection
  • Implements tests for all 5 binary formats reading directly from test data files
  • Updates documentation for from_cbor, from_msgpack, from_ubjson, from_bjdata, and from_bson with overload (3) descriptions
  • All tests pass with existing test suite data

Test Plan

✅ CBOR: 9 test cases pass
✅ MessagePack: 4 test cases pass
✅ UBJSON: 5 test cases pass
✅ BJData: 25 test cases pass
✅ BSON: 8 test cases pass (includes temp file + cleanup test)

🤖 Generated with Claude Code

@nlohmann
nlohmann marked this pull request as draft July 10, 2026 17:43
Comment thread tests/src/unit-user_defined_input.cpp Fixed
@nlohmann
nlohmann force-pushed the claude/todo-183-plan-267c0c branch from 4bdcf92 to ef50d69 Compare July 10, 2026 17:46
This commit extends the C++20 ranges support (iterator+sentinel pairs) to the
binary format deserializers from_cbor, from_msgpack, from_ubjson, from_bjdata,
and from_bson, matching what was already done for parse(), accept(), and
sax_parse().

Changes:
- Add istreambuf_sentinel helper to test_utils.hpp for EOF detection in tests
- Add 5 new test cases that read binary files directly via
  std::istreambuf_iterator<char> + sentinel, without pre-buffering
- Update documentation for all 5 from_* functions to document overload (3)
  with SentinelType parameter
- All tests pass; verified against existing test suite data
- Fix potential buffer over-read warning in heterogeneous iterator test

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann
nlohmann force-pushed the claude/todo-183-plan-267c0c branch from ef50d69 to 2269656 Compare July 10, 2026 17:50
Comment thread include/nlohmann/detail/input/input_adapters.hpp
Comment thread tests/src/unit-bson.cpp Outdated
nlohmann added 3 commits July 10, 2026 23:15
Address PR review feedback and CI failures:

- Merge the separate same-type and sentinel-type iterator overloads of
  parse(), accept(), sax_parse(), and the five from_* binary deserializers
  into a single overload with SentinelType defaulted to IteratorType,
  as suggested in review. Applied the same simplification to the
  detail::input_adapter() free functions.
- Fix a latent ambiguity: some compilers (e.g. GCC 4.8) unreliably SFINAE
  the operator!= detection for std::nullptr_t against container/string
  types, making calls like parse(s, nullptr, ...) ambiguous with the
  compatible-input overload. can_compare_ne now explicitly excludes
  std::nullptr_t as a SentinelType.
- Use a named enable_if_t template parameter instead of an unnamed
  function parameter for the SFINAE guard, fixing a clang-tidy
  hicpp-named-parameter/readability-named-parameter failure.
- Update parse.md, accept.md, sax_parse.md, and the five from_*.md pages
  to document the merged overload instead of separate (2)/(3) overloads,
  also fixing an over-160-char line that broke the documentation
  style_check CI job.
- Rework the BSON iterator+sentinel test to parse a BSON file already
  present in the test suite instead of writing/deleting a temp file.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
CustomSentinel lives in an anonymous namespace (internal linkage), and
the library's parse loop only ever evaluates the iterator-first
direction (it != last), so the reversed-order friend operator!= was
never referenced. Clang's -Weverything flags such unused internal
declarations as an error. Drop the unused overload; the used direction
is enough to satisfy can_compare_ne's either-order detection.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
- Drop the unused reversed-order operator!= overload from
  utils::istreambuf_sentinel (only iterator != sentinel is ever
  evaluated) and name the remaining friend's sentinel parameter, fixing
  hicpp-named-parameter/readability-named-parameter.
- Mark the istreambuf_iterator first/last helper variable const in the
  five binary-format sentinel tests, fixing misc-const-correctness.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann
nlohmann marked this pull request as ready for review July 10, 2026 22:11
json_str is only read via .data()/.size() and never reassigned, so
clang-tidy correctly flags it as const-able. Verified against the exact
CI job (silkeh/clang:dev, ci_clang_tidy target) by running clang-tidy
directly on this file plus the five binary-format sentinel tests
touched by prior commits; all are now clean.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann nlohmann added this to the Release 3.13.0 milestone Jul 11, 2026
@nlohmann nlohmann added the 🚀 ready to merge Ready to merge - just waiting for CI to complete. label Jul 11, 2026
@nlohmann
nlohmann merged commit ca76c37 into develop Jul 11, 2026
155 of 156 checks passed
@nlohmann
nlohmann deleted the claude/todo-183-plan-267c0c branch July 11, 2026 13:09
Comment thread include/nlohmann/detail/input/input_adapters.hpp
SA-gitlab-to-github pushed a commit to DarkhiveAI/dh-fork-json that referenced this pull request Jul 12, 2026
Extend the memcpy fast path in iterator_input_adapter to sized
sentinels of a different type, not just same-type iterator pairs.
std::counted_iterator paired with std::default_sentinel_t already
satisfies std::contiguous_iterator and std::sized_sentinel_for, so
std::ranges::distance (C++20) lets that combination reach the fast
path too, instead of silently falling back to the byte-by-byte path.

- iterator_is_contiguous now also allows std::sized_sentinel_for<SentinelType,
  IteratorType> under C++20, gated the same way as the existing
  std::contiguous_iterator detection.
- get_elements_impl's fast path uses std::ranges::distance under C++20
  (works for both same-type and sized-sentinel pairs) and falls back to
  std::distance pre-C++20, where SentinelType is always IteratorType.
- Add a C++20-only test exercising json::parse/accept with
  std::counted_iterator + std::default_sentinel_t.
- Document std::default_sentinel_t + std::counted_iterator as a
  SentinelType example across parse.md, accept.md, sax_parse.md, and
  the five from_*.md pages, replacing an earlier ambiguously worded
  bullet.

Addresses review feedback: nlohmann#5265 (comment)

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
jwnimmer-tri pushed a commit to jwnimmer-tri/json that referenced this pull request Sep 21, 2026
…nn#5265)

* Add iterator+sentinel tests and docs for binary deserializers

This commit extends the C++20 ranges support (iterator+sentinel pairs) to the
binary format deserializers from_cbor, from_msgpack, from_ubjson, from_bjdata,
and from_bson, matching what was already done for parse(), accept(), and
sax_parse().

Changes:
- Add istreambuf_sentinel helper to test_utils.hpp for EOF detection in tests
- Add 5 new test cases that read binary files directly via
  std::istreambuf_iterator<char> + sentinel, without pre-buffering
- Update documentation for all 5 from_* functions to document overload (3)
  with SentinelType parameter
- All tests pass; verified against existing test suite data
- Fix potential buffer over-read warning in heterogeneous iterator test

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

* Merge iterator+sentinel overloads and fix ambiguity/CI issues

Address PR review feedback and CI failures:

- Merge the separate same-type and sentinel-type iterator overloads of
  parse(), accept(), sax_parse(), and the five from_* binary deserializers
  into a single overload with SentinelType defaulted to IteratorType,
  as suggested in review. Applied the same simplification to the
  detail::input_adapter() free functions.
- Fix a latent ambiguity: some compilers (e.g. GCC 4.8) unreliably SFINAE
  the operator!= detection for std::nullptr_t against container/string
  types, making calls like parse(s, nullptr, ...) ambiguous with the
  compatible-input overload. can_compare_ne now explicitly excludes
  std::nullptr_t as a SentinelType.
- Use a named enable_if_t template parameter instead of an unnamed
  function parameter for the SFINAE guard, fixing a clang-tidy
  hicpp-named-parameter/readability-named-parameter failure.
- Update parse.md, accept.md, sax_parse.md, and the five from_*.md pages
  to document the merged overload instead of separate (2)/(3) overloads,
  also fixing an over-160-char line that broke the documentation
  style_check CI job.
- Rework the BSON iterator+sentinel test to parse a BSON file already
  present in the test suite instead of writing/deleting a temp file.

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

* Fix -Wunneeded-internal-declaration for CustomSentinel in test

CustomSentinel lives in an anonymous namespace (internal linkage), and
the library's parse loop only ever evaluates the iterator-first
direction (it != last), so the reversed-order friend operator!= was
never referenced. Clang's -Weverything flags such unused internal
declarations as an error. Drop the unused overload; the used direction
is enough to satisfy can_compare_ne's either-order detection.

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

* Fix clang-tidy hicpp-named-parameter and misc-const-correctness

- Drop the unused reversed-order operator!= overload from
  utils::istreambuf_sentinel (only iterator != sentinel is ever
  evaluated) and name the remaining friend's sentinel parameter, fixing
  hicpp-named-parameter/readability-named-parameter.
- Mark the istreambuf_iterator first/last helper variable const in the
  five binary-format sentinel tests, fixing misc-const-correctness.

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

* Fix clang-tidy misc-const-correctness in heterogeneous sentinel test

json_str is only read via .data()/.size() and never reassigned, so
clang-tidy correctly flags it as const-able. Verified against the exact
CI job (silkeh/clang:dev, ci_clang_tidy target) by running clang-tidy
directly on this file plus the five binary-format sentinel tests
touched by prior commits; all are now clean.

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

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation L 🚀 ready to merge Ready to merge - just waiting for CI to complete. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants