JSON parser, lexer and SAX DOM parsers: duplicated position/EOF/UTF-8 encoding code and stale comments and includes #5712

These items came out of a technical-debt review of develop at 633de8e. Each item is independent and can be fixed in its own PR; please reference this issue from it. All items are behavior-preserving: items 1, 2 and 6 move duplicated detail code into one place, and the rest change only comments and standard-library includes. None of them changes the public API or the ABI. Every PR must regenerate single_include/ with make amalgamate.

Checklist

• 1. Deduplicate handle_diagnostic_positions_for_json_value() in the two DOM SAX parsers
• 2. Deduplicate the strict-EOF / release_lookahead() / error block in parser::parse() — overlaps Recover from parse errors when parse_error() returns true #5601
• 3. Fix wrong comments in parser.hpp ("recursive descent", skip_to_state_evaluation)
• 4. Update the discard_number_values comments that still describe strtoull()/strtoll() — overlaps Recover from parse errors when parse_error() returns true #5601
• 5. Replace the "single source of truth for UTF-8 validation" claim in string_utils.hpp
• 6. Share the code point to UTF-8 encoding between the wide-string input helpers and the lexer — overlaps Reject malformed UTF-16/UTF-32 units in wide-string input #5704
• 7. Fix stale include lists and doc comments in detail/input/ headers — overlaps Move the float conversion chain out of the lexer #5616, Add AST-based public API checker and fix documentation gaps it found #5267

Items

1. Deduplicate handle_diagnostic_positions_for_json_value() in the two DOM SAX parsers

With JSON_DIAGNOSTIC_POSITIONS, json_sax_dom_parser (json_sax.hpp L379-L447) and json_sax_dom_callback_parser (L834-L898) each have a private handle_diagnostic_positions_for_json_value(BasicJsonType& v). diff <(sed -n 379,447p json_sax.hpp) <(sed -n 834,898p json_sax.hpp) shows only comment and blank-line differences. Both need only m_lexer_ref. Any fix to position tracking has to be made twice. For example, the string case was recently changed to use get_token_start_position().

The annotations have already drifted:

• The first copy wraps case value_t::discarded in LCOV_EXCL_START/STOP (L418-L425) and calls it "esoteric". The callback parser does reach that branch from end_object()/end_array() (L661-L665, L781).
• Both copies (L443, L894) list -warnings-as-errors in NOLINT(...). That is not a check name, and no other JSON_ASSERT(false) in include/ has it (e.g. lexer.hpp L1144).

Proposed change: Keep one implementation in nlohmann::detail under #if JSON_DIAGNOSTIC_POSITIONS, e.g. template<typename BasicJsonType, typename LexerType> void set_diagnostic_positions(BasicJsonType& v, const LexerType* lexer);. Both parsers call it with m_lexer_ref. The body reads private basic_json members (start_position, end_position, m_data), which works today because both parsers are friends (json.hpp L149-L151). A static member of a small detail struct needs only one extra friend line. While moving the code, drop the LCOV markers and the "esoteric" comment, and remove -warnings-as-errors from the NOLINT. The duplicated start-position setup in start_object()/start_array() is out of scope until #5706 lands, because #5706 edits the callback versions.

Verification: ci_test_diagnostic_positions, ci_test_diagnostics, unit-diagnostic-positions and unit-class_parser. Coverage should show the discarded branch covered without an exclusion, and ci_clang_tidy checks the NOLINT.

2. Deduplicate the strict-EOF / release_lookahead() / error block in parser::parse()

parser::parse() (parser.hpp L96-L165) has a branch for json_sax_dom_callback_parser (L98-L134) and a branch for json_sax_dom_parser (L135-L162). Apart from the parser type and the callback-only if (result.is_discarded()) { result = nullptr; }, both run the same ~25 lines: sax_parse_internal(&sdp), the strict end-of-input check with parse_error::create(101, ...), m_lexer.release_lookahead() in non-strict mode, and result = value_t::discarded on sdp.is_errored(). The copies have started to drift: the call is wrapped differently, and the second release_lookahead comment just says "see above". Every change to EOF handling, lookahead release or error propagation has to be made twice.

Proposed change: Add a private member template:

template<typename DomSax>
bool parse_dom(DomSax& sdp, const bool strict)
{
    sax_parse_internal(&sdp);
    if (strict) { /* EOF check, sdp.parse_error(...) */ }
    else { m_lexer.release_lookahead(); }
    return !sdp.is_errored();
}

parse() then only builds the matching SAX parser, sets discarded on failure, and maps a discarded top-level value to nullptr in the callback branch. Leave sax_parse() (L179-L207) alone. It runs the EOF check and release_lookahead() only if (result), while parse() runs them unconditionally. Merging the two would change the stream position after a failed non-strict parse with allow_exceptions == false, which is observable with JSON_PRECISE_STREAM_POSITION.

Verification: unit-class_parser (strict and non-strict, callbacks, allow_exceptions = false), unit-deserialization (stream position, JSON_PRECISE_STREAM_POSITION), and unit-disabled_exceptions.

Sequencing: Draft #5601 edits the same lines: it turns both calls into sax_parse_internal<false>(&sdp) and reworks sax_parse(). Do this after #5601 is merged or closed.

3. Fix wrong comments in parser.hpp ("recursive descent", skip_to_state_evaluation)

• The class documentation (parser.hpp L54-L58) says "This class implements a recursive descent parser." sax_parse_internal() (L212-L222) does not recurse. It is a while (true) loop with an explicit std::vector<bool> states stack. binary_reader.hpp L204-L210 even cites it as the non-recursive model.
• At L441-L446 (end of array) and L501-L507 (end of object), the comment "By setting skip_to_state_evaluation to false, we are effectively jumping to the beginning of this if" sits directly above skip_to_state_evaluation = true;. The code sets the flag to true, so the next iteration skips the token switch and evaluates the enclosing state directly.

Proposed change: Change comments only. Replace the class sentence with "This class implements an iterative parser that keeps the open containers on an explicit stack and reports what it reads as SAX events." At both flag sites, write: "By setting skip_to_state_evaluation to true, the next iteration skips reading a token and evaluates the enclosing state directly."

Verification: The check-amalgamation job. No code changes.

#5601 (hunks from L447 and L508) and #5592 (L417, L456, L477, L520) edit sax_parse_internal() but not these lines. Whichever lands later may need a trivial rebase.

4. Update the discard_number_values comments that still describe strtoull()/strtoll()

#5484 added discard_number_values, which lets accept() skip integer conversion when the digit count shows that the value fits in 64 bits. A day later, #5283 replaced strtoull()/strtoll() with convert_integer() (lexer.hpp L1396-L1425), but the comments were not updated:

• The 38-line block in convert_number() (L1497-L1535) argues about avoiding strtoull()/strtoll() and their errno/ERANGE. Neither function is called any more.
• The member doc (L2328-L2332) says "scan_number() may skip strtoull()/strtoll() ... (see scan_number())". The check is in convert_number().

The comments also do not say where the shortcut is reached. For contiguous input without JSON_DIAGNOSTIC_POSITIONS, scan_number_bulk_contiguous() (L1741-L1756) converts integers itself and returns before convert_number(). The branch therefore runs only for stream input, for JSON_DIAGNOSTIC_POSITIONS builds, and when the bulk scanner falls back. A local accept() run on 1M-integer arrays matches this. On contiguous input, converting and discarding took the same time (46.45 vs 46.82 ms; 26.72 vs 26.27 ms, median of 31). With std::istringstream, discarding was about 5-7% faster.

Proposed change: Change comments only. Rewrite the convert_number() block to describe a digit-count check in front of convert_integer(). Keep the 18-digit bound and the json_sax_acceptor safety argument, and state that the check is reached only on the non-bulk path. Rewrite the member doc to say that only accept() sets the member and that the check lives in convert_number(). Whether a ~5% stream-only accept() speedup justifies passing the flag through basic_json::parser(), the parser ctor and the lexer ctor is a separate decision.

Verification: make amalgamate and the amalgamation check.

Sequencing: Draft #5601 moves the discard_number_values member, so the member doc edit may need a rebase. #5616's hunks end at L1496 and resume after L1563, so they do not overlap the convert_number() block.

5. Replace the "single source of truth for UTF-8 validation" claim in string_utils.hpp

The decode() doc (string_utils.hpp L54-L58) calls the Hoehrmann DFA "the single source of truth for UTF-8 validation in this library". In fact the DFA is used only by serializer::dump_escaped() and by is_valid_utf8(), which the CBOR/MessagePack/BSON/UBJSON readers call. Three other validators encode the RFC 3629 ranges on their own:

• the per-lead-byte switch in lexer::scan_string() (lexer.hpp L835-L955), which produces JSON-specific diagnostics;
• validate_one_utf8() (string_scan.hpp L149-L202), used by the lexer's bulk scan and, through valid_utf8_prefix(), by the BON8 bulk path (binary_reader.hpp L3861);
• the BON8 byte path in get_bon8_string() (binary_reader.hpp L3929-L3968).

With the current comment, a fix to the DFA can look complete even though it does not reach JSON text, the bulk scan or BON8. Each validator exists for a reason (diagnostics, speed, BON8 pushback semantics), so the fix is accurate documentation, not consolidation.

Proposed change: Change the comment only. List the four validators and what each is used for, and note that all four must accept the same set of sequences (RFC 3629, section 4), so a change to one needs matching changes to the others. A shared second-byte helper for validate_one_utf8() and the BON8 byte path was considered and dropped. It would save about four lines, and #5677 is editing that path.

Verification: Amalgamation and astyle checks.

6. Share the code point to UTF-8 encoding between the wide-string input helpers and the lexer

The 1/2/3/4-byte encoding ladder is written out by hand in wide_string_input_helper<..., 4>::fill_buffer() (input_adapters.hpp L451-L476), in the UTF-16 helper for BMP units (L513-L531) and surrogate pairs (L545-L552), and in the lexer's \u handling (lexer.hpp L502-L528). The copies have drifted: the UTF-32 helper masks the lead byte (& 0x1Fu, & 0x0Fu, & 0x07u), and the others do not. Each copy is correct for the ranges that reach it, but a reader has to verify every copy separately.

Not all of this code is duplication. The UTF-16 helper deliberately passes malformed surrogate units through so that the decoder rejects them (L533-L560), and the UTF-32 helper emits a 0xFF sentinel above U+10FFFF (L478-L487). Both must stay as they are.

Proposed change: Add one detail helper, e.g. template<typename Out> void encode_utf8(std::uint32_t cp, Out out) (precondition: cp <= 0x10FFFF). Use it only in the valid-code-point branches: UTF-32 wc <= 0x10FFFF, UTF-16 BMP units and valid pairs, and the lexer's \u path with out forwarding to add(). Keep the passthrough, the sentinel and the lexer's JSON_ASSERT.

Verification: The wide-string sections of unit-unicode.cpp, the wchar_t/char16_t/char32_t cases in unit-deserialization.cpp, and the \u/surrogate tests in unit-class_lexer.cpp. The lexer path is hot for escape-heavy strings, so check that the string-parsing benchmarks do not change.

Sequencing: #5704 (the fix for #5645) rewrites both wide_string_input_helper specializations (around L445, L541, L556). Do this after #5704 is merged, or fold it into #5704.

7. Fix stale include lists and doc comments in detail/input/ headers

Include lists (the headers compile standalone only through transitive includes):

• input_adapters.hpp L15-L16 includes <memory> and <numeric>, which are unused and left over from the shared_ptr adapter design. The header uses (std::min) (L378) without including <algorithm>.
• json_sax.hpp L152 uses std::numeric_limits but does not include <limits>.
• lexer.hpp L15 still lists strtoll, strtoull for <cstdlib>. Only strtof/strtod/strtold are used.

Comments:

• input_adapters.hpp L84-L85 says the stream adapter "Ignores a UFT Byte Order Mark". The typo aside, lexer::skip_bom() does this, not the adapter. L865 names a nonexistent input_buffer_adapter.
• lexer.hpp L2026 says get_string() "implicitly resets the token", but it resets nothing. L1098 carries a long // lgtm [cpp/use-of-goto] suppression. LGTM.com shut down in 2022, and CodeQL ignores it.
• binary_reader.hpp L4055-L4069: the get_number() doc opens with /*, not /*!, and its endianness list omits BON8, which also calls it. L3347-L3348 says UBJSON binary types "do not exist", but BJData 'B' arrays are read as binary (L3282-L3287).
• json_sax.hpp L95/L117 document elements as "-1 if unknown". It is a std::size_t, and the published docs say std::numeric_limits<std::size_t>::max().

Proposed change: Now: remove <memory>/<numeric> and add <algorithm> // min in input_adapters.hpp (json.hpp still gets the removed headers transitively), and add <limits> to json_sax.hpp. Fix the L85/L865 comments (name contiguous_bytes_input_adapter/span_input_adapter; BOM is handled by the lexer). Change lexer.hpp L2026 to /// return current string value. Make the get_number() doc /*! and add BON8, and reword L3347 so it says that UBJSON has no binary type and that BJData 'B' is handled in get_ubjson_array(). After #5616: fix the lexer.hpp L15 include comment, and replace the lgtm line with a short plain comment on why goto is used. After #5267: change "-1 if unknown" to detail::unknown_size().

Verification: Compile each changed header alone as C++11 with -fsyntax-only, and run the amalgamation check, the compiler matrix, clang-tidy and cppcheck.

Sequencing: #5616 touches lexer.hpp L15 and the doc block containing L1098. Draft #5267 adds @sa lines next to the "-1 if unknown" lines. #5699, #5702 and #5704 touch other hunks of input_adapters.hpp, but not these lines.

Related

• Recover from parse errors when parse_error() returns true #5601 (draft, parse-error recovery) touches json_sax.hpp, parser.hpp, lexer.hpp and string_utils.hpp. Items 2 and 4 overlap it directly; items 1, 3 and 5 only sit near its hunks.

This issue was written by Claude Code on behalf of @nlohmann.