diff --git a/include/nlohmann/detail/input/lexer.hpp b/include/nlohmann/detail/input/lexer.hpp index 58f7f3be9..c241e793b 100644 --- a/include/nlohmann/detail/input/lexer.hpp +++ b/include/nlohmann/detail/input/lexer.hpp @@ -1446,8 +1446,7 @@ scan_number_done: */ char_int_type get() { - ++position.chars_read_total; - ++position.chars_read_current_line; + advance_position(); if (next_unget) { @@ -1459,6 +1458,23 @@ scan_number_done: current = ia.get_character(); } + return track_after_read(); + } + + /// shared head of get() / get_ignoring_pending_unget(): bump the + /// per-character position counters (line-count-on-'\n' bookkeeping is + /// handled afterwards, in track_after_read(), once `current` is known) + void advance_position() noexcept + { + ++position.chars_read_total; + ++position.chars_read_current_line; + } + + /// shared tail of get() / get_ignoring_pending_unget(): capture the + /// character for error messages (if needed) and update line/column + /// bookkeeping for the character now in `current` + char_int_type track_after_read() + { // seekable adapters reconstruct the token lazily on error (see // get_token_string), so the eager per-character copy is skipped capture_char(std::integral_constant {}); @@ -1472,6 +1488,29 @@ scan_number_done: return current; } + /*! + @brief like get(), but for call sites that can prove no unget() is pending + + get() has to check the `next_unget` flag on every call, because a + previous token may have ended with unget() (e.g. scan_number() always + ungets the character that terminated the number, so the next call to + scan() can see it again). skip_whitespace() reads that first, + possibly-ungotten character via a plain get(), but every further + character it reads is guaranteed to be a fresh read: nothing between + those calls invokes unget(). This variant skips the (otherwise always + false) next_unget branch for those calls; it is not a general + replacement for get(). + */ + char_int_type get_ignoring_pending_unget() + { + JSON_ASSERT(!next_unget); + + advance_position(); + current = ia.get_character(); + + return track_after_read(); + } + /// seekable adapter: nothing to capture, the token is rebuilt on error void capture_char(std::true_type /*lazy*/) const noexcept {} @@ -1665,13 +1704,37 @@ scan_number_done: return true; } + /// whether `current` is one of the four JSON whitespace characters + bool current_is_whitespace() const noexcept + { + return current == ' ' || current == '\t' || current == '\n' || current == '\r'; + } + void skip_whitespace() { + // the first character may be a pending unget() left over from the + // previous token (see get_ignoring_pending_unget()); every + // subsequent character read by this loop is guaranteed fresh, since + // nothing below calls unget() + get(); + + if (!current_is_whitespace()) + { + return; + } + + // this is written as an if-guarded do-while (rather than a plain + // while loop) because that shape is what lets both GCC and Clang + // keep the input adapter's read pointer in a register across + // iterations; the equivalent while-loop measurably defeated that + // optimization in testing, turning long whitespace runs (e.g. the + // indentation of pretty-printed JSON) from a register-only loop + // into one that reloads the pointer from memory every character do { - get(); + get_ignoring_pending_unget(); } - while (current == ' ' || current == '\t' || current == '\n' || current == '\r'); + while (current_is_whitespace()); } token_type scan() diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index e08fbdaa8..f247453e8 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -9229,8 +9229,7 @@ scan_number_done: */ char_int_type get() { - ++position.chars_read_total; - ++position.chars_read_current_line; + advance_position(); if (next_unget) { @@ -9242,6 +9241,23 @@ scan_number_done: current = ia.get_character(); } + return track_after_read(); + } + + /// shared head of get() / get_ignoring_pending_unget(): bump the + /// per-character position counters (line-count-on-'\n' bookkeeping is + /// handled afterwards, in track_after_read(), once `current` is known) + void advance_position() noexcept + { + ++position.chars_read_total; + ++position.chars_read_current_line; + } + + /// shared tail of get() / get_ignoring_pending_unget(): capture the + /// character for error messages (if needed) and update line/column + /// bookkeeping for the character now in `current` + char_int_type track_after_read() + { // seekable adapters reconstruct the token lazily on error (see // get_token_string), so the eager per-character copy is skipped capture_char(std::integral_constant {}); @@ -9255,6 +9271,29 @@ scan_number_done: return current; } + /*! + @brief like get(), but for call sites that can prove no unget() is pending + + get() has to check the `next_unget` flag on every call, because a + previous token may have ended with unget() (e.g. scan_number() always + ungets the character that terminated the number, so the next call to + scan() can see it again). skip_whitespace() reads that first, + possibly-ungotten character via a plain get(), but every further + character it reads is guaranteed to be a fresh read: nothing between + those calls invokes unget(). This variant skips the (otherwise always + false) next_unget branch for those calls; it is not a general + replacement for get(). + */ + char_int_type get_ignoring_pending_unget() + { + JSON_ASSERT(!next_unget); + + advance_position(); + current = ia.get_character(); + + return track_after_read(); + } + /// seekable adapter: nothing to capture, the token is rebuilt on error void capture_char(std::true_type /*lazy*/) const noexcept {} @@ -9448,13 +9487,37 @@ scan_number_done: return true; } + /// whether `current` is one of the four JSON whitespace characters + bool current_is_whitespace() const noexcept + { + return current == ' ' || current == '\t' || current == '\n' || current == '\r'; + } + void skip_whitespace() { + // the first character may be a pending unget() left over from the + // previous token (see get_ignoring_pending_unget()); every + // subsequent character read by this loop is guaranteed fresh, since + // nothing below calls unget() + get(); + + if (!current_is_whitespace()) + { + return; + } + + // this is written as an if-guarded do-while (rather than a plain + // while loop) because that shape is what lets both GCC and Clang + // keep the input adapter's read pointer in a register across + // iterations; the equivalent while-loop measurably defeated that + // optimization in testing, turning long whitespace runs (e.g. the + // indentation of pretty-printed JSON) from a register-only loop + // into one that reloads the pointer from memory every character do { - get(); + get_ignoring_pending_unget(); } - while (current == ' ' || current == '\t' || current == '\n' || current == '\r'); + while (current_is_whitespace()); } token_type scan() diff --git a/tests/src/unit-class_parser.cpp b/tests/src/unit-class_parser.cpp index 90ab51066..7d86994e1 100644 --- a/tests/src/unit-class_parser.cpp +++ b/tests/src/unit-class_parser.cpp @@ -1486,6 +1486,71 @@ TEST_CASE("parser class") CHECK(accept_helper("\"\\uD80C\\uFFFF\"") == false); } +#if !defined(JSON_NOEXCEPTION) + SECTION("issue #5412 - whitespace skipping bookkeeping (compact vs. pretty-printed)") + { + // lexer::skip_whitespace() reads its first character with get() (to + // honor a possibly pending unget() from the previous token) and every + // further whitespace character with get_ignoring_pending_unget() (a + // get() variant that skips the then-always-false next_unget check). + // This must not change the reported byte offset, line, or column of + // a syntax error, even when a long run of whitespace containing + // multiple newlines is skipped beforehand (as with pretty-printed + // input). The expected values below were captured from the + // unmodified do-while(get()) loop, so any regression that miscounts + // characters or newlines while skipping whitespace changes them. + const auto check_error = [](const std::string & input, std::size_t expected_byte, + const std::string & expected_what) + { + CAPTURE(input) + try + { + json _ = json::parse(input); + FAIL_CHECK("expected a parse_error, but parsing succeeded"); + } + catch (const json::parse_error& e) + { + CHECK(e.byte == expected_byte); + CHECK(std::string(e.what()) == expected_what); + } + }; + + // a nested document, serialized both compactly and pretty-printed + // (dump(4)), each truncated right before the final closing '}' so + // that the parser hits EOF after skipping all of the (in the + // pretty-printed case, substantial) indentation whitespace + const json doc = + { + {"a", 1}, + {"b", json::array({true, false, nullptr, "x"})}, + {"c", json::object({{"d", 3.14}, {"e", json::array({1, 2, 3})}})} + }; + + const std::string compact = doc.dump(); + const std::string pretty = doc.dump(4); + + check_error(compact.substr(0, compact.size() - 1), 60, + "[json.exception.parse_error.101] parse error at line 1, column 60: syntax error while parsing object - unexpected end of input; expected '}'"); + check_error(pretty.substr(0, pretty.size() - 1), 193, + "[json.exception.parse_error.101] parse error at line 17, column 1: syntax error while parsing object - unexpected end of input; expected '}'"); + + // an invalid token appearing after several indented, multi-line + // whitespace runs vs. the same document without any of that + // whitespace + check_error(R"({ + "a": 1, + "b": [ + true, + false + ], + "c": @ +})", 70, + "[json.exception.parse_error.101] parse error at line 7, column 10: syntax error while parsing value - invalid literal; last read: '\"c\": @'"); + check_error(R"({"a":1,"b":[true,false],"c":@})", 29, + "[json.exception.parse_error.101] parse error at line 1, column 29: syntax error while parsing value - invalid literal; last read: '\"c\":@'"); + } +#endif + SECTION("tests found by mutate++") { // test case to make sure no comma precedes the first key