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-31 09:55:42 +00:00 · 2026-10-30 23:26:28 +00:00
6 changed files with 284 additions and 218 deletions
@@ -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?
+```
@@ -268,10 +268,10 @@ class lexer : public lexer_base<BasicJsonType>
  public:
    using token_type = typename lexer_base<BasicJsonType>::token_type;

-    explicit lexer(InputAdapterType&& adapter, bool ignore_comments_ = false, bool discard_values_ = false) noexcept
+    explicit lexer(InputAdapterType&& adapter, bool ignore_comments_ = false, bool discard_number_values_ = false) noexcept
        : ia(std::move(adapter))
        , ignore_comments(ignore_comments_)
-        , discard_values(discard_values_)
+        , discard_number_values(discard_number_values_)
    {
        // the token string of an error before the first token is everything
        // read so far
@@ -388,8 +388,6 @@ class lexer : public lexer_base<BasicJsonType>
    checks if it is inside the range. If a violation was detected, set up an
    error message and return false. Otherwise, return true.

-    @param[in] keep    whether the bytes are added to token_buffer (false for
-                       accept(), see scan())
    @param[in] ranges  list of integers; interpreted as list of pairs of
                       inclusive lower and upper bound, respectively

@@ -398,18 +396,17 @@ class lexer : public lexer_base<BasicJsonType>

    @return true if and only if no range violation was detected
    */
-    template<bool Keep>
-    bool next_byte_in_range(std::integral_constant<bool, Keep> keep, std::initializer_list<char_int_type> ranges)
+    bool next_byte_in_range(std::initializer_list<char_int_type> ranges)
    {
        JSON_ASSERT(ranges.size() == 2 || ranges.size() == 4 || ranges.size() == 6);
-        add_string_char(keep, current);
+        add(current);

        for (auto range = ranges.begin(); range != ranges.end(); ++range)
        {
            get();
            if (JSON_HEDLEY_LIKELY(*range <= current && current <= *(++range))) // NOLINT(bugprone-inc-dec-in-conditions)
            {
-                add_string_char(keep, current);
+                add(current);
            }
            else
            {
@@ -422,16 +419,14 @@ class lexer : public lexer_base<BasicJsonType>
    }

    /// contiguous input: bulk-append the run of ordinary characters and complete
-    /// well-formed UTF-8 sequences starting at the current read position (only
-    /// skip it if @a keep is false), then
+    /// well-formed UTF-8 sequences starting at the current read position, then
    /// read the first byte that needs individual handling (the closing quote, an
    /// escape, a control character, or an ill-formed UTF-8 byte) like get().
    /// That byte is taken from the buffer that was just scanned. Through get(),
    /// Apple Clang merged this read with the other get() calls of the scan
    /// function, which load the read position again right after it was stored
    /// (3% of json::accept() on jeopardy.json on Apple M1).
-    template<bool Keep>
-    char_int_type get_after_string_run(std::true_type /*bulk*/, std::integral_constant<bool, Keep> keep)
+    char_int_type get_after_string_run(std::true_type /*bulk*/)
    {
        // a pending unget must be consumed through the normal path first
        if (next_unget)
@@ -449,7 +444,7 @@ class lexer : public lexer_base<BasicJsonType>
        const std::size_t pos = lexer_string_bulk_run(data, remaining);
        if (pos != 0)
        {
-            append_string_run(keep, data, pos);
+            token_buffer.append(reinterpret_cast<const typename string_t::value_type*>(data), pos);
            bulk_consume(reinterpret_cast<const char*>(data), pos);
        }
        if (pos == remaining)
@@ -493,8 +488,7 @@ class lexer : public lexer_base<BasicJsonType>
    }

    /// streaming input: no bulk fast path
-    template<bool Keep>
-    char_int_type get_after_string_run(std::false_type /*bulk*/, std::integral_constant<bool, Keep> /*keep*/)
+    char_int_type get_after_string_run(std::false_type /*bulk*/)
    {
        return get();
    }
@@ -508,18 +502,13 @@ class lexer : public lexer_base<BasicJsonType>
    may contain \0 bytes), and token_buffer.size() is the number of bytes in the
    string.

-    @param[in] keep  whether the string is copied into token_buffer; if false
-                     (accept(), see scan()), it is only checked, with the same
-                     errors, and token_buffer stays empty
-
    @return token_type::value_string if string could be successfully scanned,
            token_type::parse_error otherwise

    @note In case of errors, variable error_message contains a textual
          description.
    */
-    template<bool Keep>
-    token_type scan_string(std::integral_constant<bool, Keep> keep)
+    token_type scan_string()
    {
        // reset token_buffer (ignore opening quote)
        reset();
@@ -531,7 +520,7 @@ class lexer : public lexer_base<BasicJsonType>
        {
            // bulk-consume ordinary characters from contiguous input, then
            // get the next character, the one that ends the run
-            switch (get_after_string_run(std::integral_constant<bool, bulk_scan>(), keep))
+            switch (get_after_string_run(std::integral_constant<bool, bulk_scan>()))
            {
                // end of file while parsing the string
                case char_traits<char_type>::eof():
@@ -553,35 +542,35 @@ class lexer : public lexer_base<BasicJsonType>
                    {
                        // quotation mark
                        case '\"':
-                            add_string_char(keep, '\"');
+                            add('\"');
                            break;
                        // reverse solidus
                        case '\\':
-                            add_string_char(keep, '\\');
+                            add('\\');
                            break;
                        // solidus
                        case '/':
-                            add_string_char(keep, '/');
+                            add('/');
                            break;
                        // backspace
                        case 'b':
-                            add_string_char(keep, '\b');
+                            add('\b');
                            break;
                        // form feed
                        case 'f':
-                            add_string_char(keep, '\f');
+                            add('\f');
                            break;
                        // line feed
                        case 'n':
-                            add_string_char(keep, '\n');
+                            add('\n');
                            break;
                        // carriage return
                        case 'r':
-                            add_string_char(keep, '\r');
+                            add('\r');
                            break;
                        // tab
                        case 't':
-                            add_string_char(keep, '\t');
+                            add('\t');
                            break;

                        // unicode escapes
@@ -649,7 +638,10 @@ class lexer : public lexer_base<BasicJsonType>
                            JSON_ASSERT(0x00 <= codepoint && codepoint <= 0x10FFFF);

                            // translate codepoint into bytes
-                            add_codepoint(keep, codepoint);
+                            encode_utf8(static_cast<std::uint32_t>(codepoint), [this](std::uint32_t byte)
+                            {
+                                add(static_cast<char_int_type>(byte));
+                            });

                            break;
                        }
@@ -832,7 +824,7 @@ class lexer : public lexer_base<BasicJsonType>
                case 0x7E:
                case 0x7F:
                {
-                    add_string_char(keep, current);
+                    add(current);
                    break;
                }

@@ -868,7 +860,7 @@ class lexer : public lexer_base<BasicJsonType>
                case 0xDE:
                case 0xDF:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!next_byte_in_range(keep, {0x80, 0xBF})))
+                    if (JSON_HEDLEY_UNLIKELY(!next_byte_in_range({0x80, 0xBF})))
                    {
                        return token_type::parse_error;
                    }
@@ -878,7 +870,7 @@ class lexer : public lexer_base<BasicJsonType>
                // U+0800..U+0FFF: bytes E0 A0..BF 80..BF
                case 0xE0:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0xA0, 0xBF, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0xA0, 0xBF, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -902,7 +894,7 @@ class lexer : public lexer_base<BasicJsonType>
                case 0xEE:
                case 0xEF:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0x80, 0xBF, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0x80, 0xBF, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -912,7 +904,7 @@ class lexer : public lexer_base<BasicJsonType>
                // U+D000..U+D7FF: bytes ED 80..9F 80..BF
                case 0xED:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0x80, 0x9F, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0x80, 0x9F, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -922,7 +914,7 @@ class lexer : public lexer_base<BasicJsonType>
                // U+10000..U+3FFFF F0 90..BF 80..BF 80..BF
                case 0xF0:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0x90, 0xBF, 0x80, 0xBF, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0x90, 0xBF, 0x80, 0xBF, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -934,7 +926,7 @@ class lexer : public lexer_base<BasicJsonType>
                case 0xF2:
                case 0xF3:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0x80, 0xBF, 0x80, 0xBF, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0x80, 0xBF, 0x80, 0xBF, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -944,7 +936,7 @@ class lexer : public lexer_base<BasicJsonType>
                // U+100000..U+10FFFF F4 80..8F 80..BF 80..BF
                case 0xF4:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0x80, 0x8F, 0x80, 0xBF, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0x80, 0x8F, 0x80, 0xBF, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -1435,7 +1427,7 @@ scan_number_done:
    token_type convert_number(token_type number_type, std::size_t mantissa_end)
    {
        // accept() only needs to know whether the input is valid, so it sets
-        // discard_values (see json.hpp), and an integer token whose
+        // discard_number_values (see json.hpp), and an integer token whose
        // digit count shows that it fits is reported without calling
        // convert_integer(). A number with up to 18 digits always fits into
        // both std::uint64_t and std::int64_t (18 nines is about 1e18, below
@@ -1450,7 +1442,7 @@ scan_number_done:
        // json_sax_acceptor, whose number callbacks discard their argument and
        // return true, and the parser rejects neither integers nor finite
        // floats. value_unsigned/value_integer are left unset here, so a caller
-        // that reads the converted value must not set discard_values.
+        // that reads the converted value must not set discard_number_values.
        //
        // On contiguous input, scan_number_bulk_contiguous() converts integer
        // tokens itself and does not pass them to this function, unless
@@ -1458,7 +1450,7 @@ scan_number_done:
        // reached for input without bulk access (e.g. streams), with
        // JSON_DIAGNOSTIC_POSITIONS, or when scan_number_bulk_contiguous()
        // falls back to scan_number().
-        if (discard_values)
+        if (discard_number_values)
        {
            constexpr std::size_t safe_digit_count = 18;
            if (number_type == token_type::value_unsigned && token_buffer.size() <= safe_digit_count)
@@ -1709,7 +1701,7 @@ scan_number_done:
        // w < 10^19, so a value below 10^max_exponent10 cannot overflow, and a
        // value that underflows is a finite number as well. Only a token that
        // may overflow has to be converted to find out.
-        if (JSON_HEDLEY_UNLIKELY(discard_values && s.exponent + 19 <= std::numeric_limits<number_float_t>::max_exponent10))
+        if (JSON_HEDLEY_UNLIKELY(discard_number_values && s.exponent + 19 <= std::numeric_limits<number_float_t>::max_exponent10))
        {
            value_float = 0;
            return true;
@@ -2041,34 +2033,6 @@ scan_number_done:
        token_buffer.push_back(static_cast<typename string_t::value_type>(c));
    }

-    /// add a character of a string token; accept() checks the string but does
-    /// not keep its value (@a keep is false)
-    void add_string_char(std::true_type /*keep*/, char_int_type c)
-    {
-        token_buffer.push_back(static_cast<typename string_t::value_type>(c));
-    }
-
-    static void add_string_char(std::false_type /*keep*/, char_int_type /*c*/) noexcept {}
-
-    /// add the UTF-8 bytes of a code point from a \\u escape to a string token
-    void add_codepoint(std::true_type /*keep*/, int codepoint)
-    {
-        encode_utf8(static_cast<std::uint32_t>(codepoint), [this](std::uint32_t byte)
-        {
-            add(static_cast<char_int_type>(byte));
-        });
-    }
-
-    static void add_codepoint(std::false_type /*keep*/, int /*codepoint*/) noexcept {}
-
-    /// append a run of ordinary string characters (see get_after_string_run())
-    void append_string_run(std::true_type /*keep*/, const unsigned char* data, std::size_t n)
-    {
-        token_buffer.append(reinterpret_cast<const typename string_t::value_type*>(data), n);
-    }
-
-    static void append_string_run(std::false_type /*keep*/, const unsigned char* /*data*/, std::size_t /*n*/) noexcept {}
-
  public:
    /////////////////////
    // value getters
@@ -2463,11 +2427,7 @@ scan_number_done:

            // string
            case '\"':
-                // accept() (discard_values) checks strings without copying them
-                // into token_buffer. The choice is made once per string: a
-                // check inside the string loop made number-heavy documents up
-                // to 3 % slower (mesh.json, Apple M1)
-                return discard_values ? scan_string(std::false_type {}) : scan_string(std::true_type {});
+                return scan_string();

            // number
            case '-':
@@ -2552,13 +2512,11 @@ scan_number_done:
    std::size_t decimal_point_position = std::string::npos;

    /// whether the caller only needs the token types and never looks at the
-    /// values of number and string tokens; set only by accept(), which parses
-    /// through json_sax_acceptor. When set, convert_number() skips converting
-    /// integer tokens whose digit count guarantees that they fit into 64 bits
-    /// (see there), floats are converted only when they may overflow (see
-    /// convert_accumulated_float()), and strings are checked without being
-    /// copied into token_buffer (see scan())
-    const bool discard_values = false;
+    /// converted numeric values; set only by accept(), which parses through
+    /// json_sax_acceptor. When set, convert_number() skips converting integer
+    /// tokens whose digit count guarantees that they fit into 64 bits (see
+    /// there)
+    const bool discard_number_values = false;
 };

 }  // namespace detail
@@ -89,9 +89,9 @@ class parser
                    const bool allow_exceptions_ = true,
                    const bool ignore_comments = false,
                    const bool ignore_trailing_commas_ = false,
-                    const bool discard_values_ = false)
+                    const bool discard_number_values_ = false)
        : callback(std::move(cb))
-        , m_lexer(std::move(adapter), ignore_comments, discard_values_)
+        , m_lexer(std::move(adapter), ignore_comments, discard_number_values_)
        , allow_exceptions(allow_exceptions_)
        , ignore_trailing_commas(ignore_trailing_commas_)
    {
@@ -184,11 +184,11 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
        const bool allow_exceptions = true,
        const bool ignore_comments = false,
        const bool ignore_trailing_commas = false,
-        const bool discard_values = false
+        const bool discard_number_values = false
    )
    {
        return ::nlohmann::detail::parser<basic_json, InputAdapterType>(std::move(adapter),
-               std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas, discard_values);
+               std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas, discard_number_values);
    }

  private:
@@ -13124,10 +13124,10 @@ class lexer : public lexer_base<BasicJsonType>
  public:
    using token_type = typename lexer_base<BasicJsonType>::token_type;

-    explicit lexer(InputAdapterType&& adapter, bool ignore_comments_ = false, bool discard_values_ = false) noexcept
+    explicit lexer(InputAdapterType&& adapter, bool ignore_comments_ = false, bool discard_number_values_ = false) noexcept
        : ia(std::move(adapter))
        , ignore_comments(ignore_comments_)
-        , discard_values(discard_values_)
+        , discard_number_values(discard_number_values_)
    {
        // the token string of an error before the first token is everything
        // read so far
@@ -13244,8 +13244,6 @@ class lexer : public lexer_base<BasicJsonType>
    checks if it is inside the range. If a violation was detected, set up an
    error message and return false. Otherwise, return true.

-    @param[in] keep    whether the bytes are added to token_buffer (false for
-                       accept(), see scan())
    @param[in] ranges  list of integers; interpreted as list of pairs of
                       inclusive lower and upper bound, respectively

@@ -13254,18 +13252,17 @@ class lexer : public lexer_base<BasicJsonType>

    @return true if and only if no range violation was detected
    */
-    template<bool Keep>
-    bool next_byte_in_range(std::integral_constant<bool, Keep> keep, std::initializer_list<char_int_type> ranges)
+    bool next_byte_in_range(std::initializer_list<char_int_type> ranges)
    {
        JSON_ASSERT(ranges.size() == 2 || ranges.size() == 4 || ranges.size() == 6);
-        add_string_char(keep, current);
+        add(current);

        for (auto range = ranges.begin(); range != ranges.end(); ++range)
        {
            get();
            if (JSON_HEDLEY_LIKELY(*range <= current && current <= *(++range))) // NOLINT(bugprone-inc-dec-in-conditions)
            {
-                add_string_char(keep, current);
+                add(current);
            }
            else
            {
@@ -13278,16 +13275,14 @@ class lexer : public lexer_base<BasicJsonType>
    }

    /// contiguous input: bulk-append the run of ordinary characters and complete
-    /// well-formed UTF-8 sequences starting at the current read position (only
-    /// skip it if @a keep is false), then
+    /// well-formed UTF-8 sequences starting at the current read position, then
    /// read the first byte that needs individual handling (the closing quote, an
    /// escape, a control character, or an ill-formed UTF-8 byte) like get().
    /// That byte is taken from the buffer that was just scanned. Through get(),
    /// Apple Clang merged this read with the other get() calls of the scan
    /// function, which load the read position again right after it was stored
    /// (3% of json::accept() on jeopardy.json on Apple M1).
-    template<bool Keep>
-    char_int_type get_after_string_run(std::true_type /*bulk*/, std::integral_constant<bool, Keep> keep)
+    char_int_type get_after_string_run(std::true_type /*bulk*/)
    {
        // a pending unget must be consumed through the normal path first
        if (next_unget)
@@ -13305,7 +13300,7 @@ class lexer : public lexer_base<BasicJsonType>
        const std::size_t pos = lexer_string_bulk_run(data, remaining);
        if (pos != 0)
        {
-            append_string_run(keep, data, pos);
+            token_buffer.append(reinterpret_cast<const typename string_t::value_type*>(data), pos);
            bulk_consume(reinterpret_cast<const char*>(data), pos);
        }
        if (pos == remaining)
@@ -13349,8 +13344,7 @@ class lexer : public lexer_base<BasicJsonType>
    }

    /// streaming input: no bulk fast path
-    template<bool Keep>
-    char_int_type get_after_string_run(std::false_type /*bulk*/, std::integral_constant<bool, Keep> /*keep*/)
+    char_int_type get_after_string_run(std::false_type /*bulk*/)
    {
        return get();
    }
@@ -13364,18 +13358,13 @@ class lexer : public lexer_base<BasicJsonType>
    may contain \0 bytes), and token_buffer.size() is the number of bytes in the
    string.

-    @param[in] keep  whether the string is copied into token_buffer; if false
-                     (accept(), see scan()), it is only checked, with the same
-                     errors, and token_buffer stays empty
-
    @return token_type::value_string if string could be successfully scanned,
            token_type::parse_error otherwise

    @note In case of errors, variable error_message contains a textual
          description.
    */
-    template<bool Keep>
-    token_type scan_string(std::integral_constant<bool, Keep> keep)
+    token_type scan_string()
    {
        // reset token_buffer (ignore opening quote)
        reset();
@@ -13387,7 +13376,7 @@ class lexer : public lexer_base<BasicJsonType>
        {
            // bulk-consume ordinary characters from contiguous input, then
            // get the next character, the one that ends the run
-            switch (get_after_string_run(std::integral_constant<bool, bulk_scan>(), keep))
+            switch (get_after_string_run(std::integral_constant<bool, bulk_scan>()))
            {
                // end of file while parsing the string
                case char_traits<char_type>::eof():
@@ -13409,35 +13398,35 @@ class lexer : public lexer_base<BasicJsonType>
                    {
                        // quotation mark
                        case '\"':
-                            add_string_char(keep, '\"');
+                            add('\"');
                            break;
                        // reverse solidus
                        case '\\':
-                            add_string_char(keep, '\\');
+                            add('\\');
                            break;
                        // solidus
                        case '/':
-                            add_string_char(keep, '/');
+                            add('/');
                            break;
                        // backspace
                        case 'b':
-                            add_string_char(keep, '\b');
+                            add('\b');
                            break;
                        // form feed
                        case 'f':
-                            add_string_char(keep, '\f');
+                            add('\f');
                            break;
                        // line feed
                        case 'n':
-                            add_string_char(keep, '\n');
+                            add('\n');
                            break;
                        // carriage return
                        case 'r':
-                            add_string_char(keep, '\r');
+                            add('\r');
                            break;
                        // tab
                        case 't':
-                            add_string_char(keep, '\t');
+                            add('\t');
                            break;

                        // unicode escapes
@@ -13505,7 +13494,10 @@ class lexer : public lexer_base<BasicJsonType>
                            JSON_ASSERT(0x00 <= codepoint && codepoint <= 0x10FFFF);

                            // translate codepoint into bytes
-                            add_codepoint(keep, codepoint);
+                            encode_utf8(static_cast<std::uint32_t>(codepoint), [this](std::uint32_t byte)
+                            {
+                                add(static_cast<char_int_type>(byte));
+                            });

                            break;
                        }
@@ -13688,7 +13680,7 @@ class lexer : public lexer_base<BasicJsonType>
                case 0x7E:
                case 0x7F:
                {
-                    add_string_char(keep, current);
+                    add(current);
                    break;
                }

@@ -13724,7 +13716,7 @@ class lexer : public lexer_base<BasicJsonType>
                case 0xDE:
                case 0xDF:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!next_byte_in_range(keep, {0x80, 0xBF})))
+                    if (JSON_HEDLEY_UNLIKELY(!next_byte_in_range({0x80, 0xBF})))
                    {
                        return token_type::parse_error;
                    }
@@ -13734,7 +13726,7 @@ class lexer : public lexer_base<BasicJsonType>
                // U+0800..U+0FFF: bytes E0 A0..BF 80..BF
                case 0xE0:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0xA0, 0xBF, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0xA0, 0xBF, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -13758,7 +13750,7 @@ class lexer : public lexer_base<BasicJsonType>
                case 0xEE:
                case 0xEF:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0x80, 0xBF, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0x80, 0xBF, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -13768,7 +13760,7 @@ class lexer : public lexer_base<BasicJsonType>
                // U+D000..U+D7FF: bytes ED 80..9F 80..BF
                case 0xED:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0x80, 0x9F, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0x80, 0x9F, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -13778,7 +13770,7 @@ class lexer : public lexer_base<BasicJsonType>
                // U+10000..U+3FFFF F0 90..BF 80..BF 80..BF
                case 0xF0:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0x90, 0xBF, 0x80, 0xBF, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0x90, 0xBF, 0x80, 0xBF, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -13790,7 +13782,7 @@ class lexer : public lexer_base<BasicJsonType>
                case 0xF2:
                case 0xF3:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0x80, 0xBF, 0x80, 0xBF, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0x80, 0xBF, 0x80, 0xBF, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -13800,7 +13792,7 @@ class lexer : public lexer_base<BasicJsonType>
                // U+100000..U+10FFFF F4 80..8F 80..BF 80..BF
                case 0xF4:
                {
-                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range(keep, {0x80, 0x8F, 0x80, 0xBF, 0x80, 0xBF}))))
+                    if (JSON_HEDLEY_UNLIKELY(!(next_byte_in_range({0x80, 0x8F, 0x80, 0xBF, 0x80, 0xBF}))))
                    {
                        return token_type::parse_error;
                    }
@@ -14291,7 +14283,7 @@ scan_number_done:
    token_type convert_number(token_type number_type, std::size_t mantissa_end)
    {
        // accept() only needs to know whether the input is valid, so it sets
-        // discard_values (see json.hpp), and an integer token whose
+        // discard_number_values (see json.hpp), and an integer token whose
        // digit count shows that it fits is reported without calling
        // convert_integer(). A number with up to 18 digits always fits into
        // both std::uint64_t and std::int64_t (18 nines is about 1e18, below
@@ -14306,7 +14298,7 @@ scan_number_done:
        // json_sax_acceptor, whose number callbacks discard their argument and
        // return true, and the parser rejects neither integers nor finite
        // floats. value_unsigned/value_integer are left unset here, so a caller
-        // that reads the converted value must not set discard_values.
+        // that reads the converted value must not set discard_number_values.
        //
        // On contiguous input, scan_number_bulk_contiguous() converts integer
        // tokens itself and does not pass them to this function, unless
@@ -14314,7 +14306,7 @@ scan_number_done:
        // reached for input without bulk access (e.g. streams), with
        // JSON_DIAGNOSTIC_POSITIONS, or when scan_number_bulk_contiguous()
        // falls back to scan_number().
-        if (discard_values)
+        if (discard_number_values)
        {
            constexpr std::size_t safe_digit_count = 18;
            if (number_type == token_type::value_unsigned && token_buffer.size() <= safe_digit_count)
@@ -14565,7 +14557,7 @@ scan_number_done:
        // w < 10^19, so a value below 10^max_exponent10 cannot overflow, and a
        // value that underflows is a finite number as well. Only a token that
        // may overflow has to be converted to find out.
-        if (JSON_HEDLEY_UNLIKELY(discard_values && s.exponent + 19 <= std::numeric_limits<number_float_t>::max_exponent10))
+        if (JSON_HEDLEY_UNLIKELY(discard_number_values && s.exponent + 19 <= std::numeric_limits<number_float_t>::max_exponent10))
        {
            value_float = 0;
            return true;
@@ -14897,34 +14889,6 @@ scan_number_done:
        token_buffer.push_back(static_cast<typename string_t::value_type>(c));
    }

-    /// add a character of a string token; accept() checks the string but does
-    /// not keep its value (@a keep is false)
-    void add_string_char(std::true_type /*keep*/, char_int_type c)
-    {
-        token_buffer.push_back(static_cast<typename string_t::value_type>(c));
-    }
-
-    static void add_string_char(std::false_type /*keep*/, char_int_type /*c*/) noexcept {}
-
-    /// add the UTF-8 bytes of a code point from a \\u escape to a string token
-    void add_codepoint(std::true_type /*keep*/, int codepoint)
-    {
-        encode_utf8(static_cast<std::uint32_t>(codepoint), [this](std::uint32_t byte)
-        {
-            add(static_cast<char_int_type>(byte));
-        });
-    }
-
-    static void add_codepoint(std::false_type /*keep*/, int /*codepoint*/) noexcept {}
-
-    /// append a run of ordinary string characters (see get_after_string_run())
-    void append_string_run(std::true_type /*keep*/, const unsigned char* data, std::size_t n)
-    {
-        token_buffer.append(reinterpret_cast<const typename string_t::value_type*>(data), n);
-    }
-
-    static void append_string_run(std::false_type /*keep*/, const unsigned char* /*data*/, std::size_t /*n*/) noexcept {}
-
  public:
    /////////////////////
    // value getters
@@ -15319,11 +15283,7 @@ scan_number_done:

            // string
            case '\"':
-                // accept() (discard_values) checks strings without copying them
-                // into token_buffer. The choice is made once per string: a
-                // check inside the string loop made number-heavy documents up
-                // to 3 % slower (mesh.json, Apple M1)
-                return discard_values ? scan_string(std::false_type {}) : scan_string(std::true_type {});
+                return scan_string();

            // number
            case '-':
@@ -15408,13 +15368,11 @@ scan_number_done:
    std::size_t decimal_point_position = std::string::npos;

    /// whether the caller only needs the token types and never looks at the
-    /// values of number and string tokens; set only by accept(), which parses
-    /// through json_sax_acceptor. When set, convert_number() skips converting
-    /// integer tokens whose digit count guarantees that they fit into 64 bits
-    /// (see there), floats are converted only when they may overflow (see
-    /// convert_accumulated_float()), and strings are checked without being
-    /// copied into token_buffer (see scan())
-    const bool discard_values = false;
+    /// converted numeric values; set only by accept(), which parses through
+    /// json_sax_acceptor. When set, convert_number() skips converting integer
+    /// tokens whose digit count guarantees that they fit into 64 bits (see
+    /// there)
+    const bool discard_number_values = false;
 };

 }  // namespace detail
@@ -22623,9 +22581,9 @@ class parser
                    const bool allow_exceptions_ = true,
                    const bool ignore_comments = false,
                    const bool ignore_trailing_commas_ = false,
-                    const bool discard_values_ = false)
+                    const bool discard_number_values_ = false)
        : callback(std::move(cb))
-        , m_lexer(std::move(adapter), ignore_comments, discard_values_)
+        , m_lexer(std::move(adapter), ignore_comments, discard_number_values_)
        , allow_exceptions(allow_exceptions_)
        , ignore_trailing_commas(ignore_trailing_commas_)
    {
@@ -37022,11 +36980,11 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
        const bool allow_exceptions = true,
        const bool ignore_comments = false,
        const bool ignore_trailing_commas = false,
-        const bool discard_values = false
+        const bool discard_number_values = false
                                 )
    {
        return ::nlohmann::detail::parser<basic_json, InputAdapterType>(std::move(adapter),
-            std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas, discard_values);
+            std::move(cb), allow_exceptions, ignore_comments, ignore_trailing_commas, discard_number_values);
    }

  private:
@@ -2982,50 +2982,6 @@ TEST_CASE("accept() agrees with parse() on floats around the overflow limit")
    }
 }

-TEST_CASE("accept() agrees with parse() on strings, which it checks without copying them")
-{
-    // accept() scans strings and object keys with a separate instantiation of
-    // the string scanner that does not copy them into the token buffer; it
-    // must accept and reject exactly what parse() does
-    const std::vector<std::string> pieces =
-    {
-        "", "abc", "\\\"\\\\\\/\\b\\f\\n\\r\\t", "\\u0041\\u00e4\\u20AC", "\\ud83d\\ude00", "\\uD83D\\uDE00",
-        "\\ud800", "\\udc00", "\\ud800\\u0041", "\\ud800x", "\\udbff\\udfff", "\\u12", "\\u12g4", "\\x", "\\",
-        "\xc3\xa4", "\xe2\x82\xac", "\xf0\x9f\x98\x80", "\xf4\x8f\xbf\xbf", "\xc0\xaf", "\xed\xa0\x80", "\xf4\x90\x80\x80",
-        "\xff", "\xe2\x82", "\xc3", "\x01", "\x1f", "\x7f", "\n", std::string(1, '\0')
-    };
-
-    for (std::size_t run = 0; run <= 70; ++run)
-    {
-        for (const std::string& piece : pieces)
-        {
-            // a run of ordinary characters puts the piece at every offset of the bulk scanners
-            const std::string str = std::string(run, 'a') + piece;
-            for (const std::string& text :
-                    {
-                        "\"" + str + "\"", "[\"" + str + "\"]", "{\"" + str + "\":1}", "{\"k\":\"" + str + "\",\"" + str + "\":[\"" + str + "\"]}",
-                        "[\"" + str
-                    })
-            {
-                CAPTURE(text)
-                for (int options = 0; options < 4; ++options)
-                {
-                    const bool ignore_comments = (options & 1) != 0;
-                    const bool ignore_trailing_commas = (options & 2) != 0;
-                    const bool parses = !json::parse(text, nullptr, false, ignore_comments, ignore_trailing_commas).is_discarded();
-                    CHECK(json::accept(text, ignore_comments, ignore_trailing_commas) == parses);
-
-                    std::istringstream stream(text);
-                    CHECK(json::accept(stream, ignore_comments, ignore_trailing_commas) == parses);
-
-                    const std::list<char> chars(text.begin(), text.end());
-                    CHECK(json::accept(chars.begin(), chars.end(), ignore_comments, ignore_trailing_commas) == parses);
-                }
-            }
-        }
-    }
-}
-
 namespace
 {
 // a DOM parser that reports parse errors without throwing, as in the sax_no_exception example