diff --git a/docs/mkdocs/docs/features/json_view.md b/docs/mkdocs/docs/features/json_view.md index 3074e4b0a..53b05f848 100644 --- a/docs/mkdocs/docs/features/json_view.md +++ b/docs/mkdocs/docs/features/json_view.md @@ -138,7 +138,9 @@ document: `#!cpp auto v = json_document::parse(text).root();` does not compile. [`JSON_DIAGNOSTIC_POSITIONS`](../api/macros/json_diagnostic_positions.md), the exceptions of views name the path and the bytes of the value as those of `#!cpp basic_json` do; as a view has neither parent pointers nor stored positions, both are found from the document's index when the exception is thrown, which takes time linear in the - size of the document (only for an exception). The other macros only affect the values `materialize()` produces, + size of the document (only for an exception). One exception has neither: `key()` of an iterator over an array or a + primitive value ([`invalid_iterator.207`](../home/exceptions.md#jsonexceptioninvalid_iterator207)), as a view's + iterator does not know the value it iterates over. The other macros only affect the values `materialize()` produces, through `BasicJsonType`. ## What is different diff --git a/include/nlohmann/detail/view/errors.hpp b/include/nlohmann/detail/view/errors.hpp index e651fdb06..adb61c8d9 100644 --- a/include/nlohmann/detail/view/errors.hpp +++ b/include/nlohmann/detail/view/errors.hpp @@ -238,7 +238,15 @@ void value_range(const document_data& d, const node* n, std::size_t& start, std: /// a value: "(/path) " with JSON_DIAGNOSTICS (none for the root), then /// "(bytes a-b) " with JSON_DIAGNOSTIC_POSITIONS (with JSON_DIAGNOSTICS, only /// if there is a path) -inline std::string diagnostics(const error_site site) +/// +/// This and the throw helpers that take an error_site are templates, so that +/// the path and range code is instantiated only where an exception can be +/// thrown: instantiated in every translation unit (as a non-template inline +/// function makes it), it changed GCC's early inlining of unrelated view +/// functions, so that GCC 13 no longer inlined a recursive walk over a view +/// into itself (2.7 times the instructions). +template +std::string diagnostics(const error_site site) { #if JSON_DIAGNOSTICS || JSON_DIAGNOSTIC_POSITIONS if (site.doc() == nullptr || site.value() == nullptr) @@ -291,7 +299,7 @@ inline std::string diagnostics(const error_site site) template std::string diagnostics_of(const View& v) { - return diagnostics(site_of(v)); + return diagnostics<>(site_of(v)); } // Exceptions are thrown out of line, so that the accessors that may throw stay @@ -318,19 +326,22 @@ std::string diagnostics_of(const View& v) NLOHMANN_VIEW_THROW(invalid_iterator::create(id, msg, nullptr)); } -[[noreturn]] NLOHMANN_VIEW_NOINLINE inline void throw_type_error(const error_site site, exception_id id, const char* prefix, const char* type) +template +[[noreturn]] NLOHMANN_VIEW_NOINLINE void throw_type_error(const error_site site, exception_id id, const char* prefix, const char* type) { - NLOHMANN_VIEW_THROW(type_error::create(id, concat(diagnostics(site), prefix, type), nullptr)); + NLOHMANN_VIEW_THROW(type_error::create(id, concat(diagnostics<>(site), prefix, type), nullptr)); } -[[noreturn]] NLOHMANN_VIEW_NOINLINE inline void throw_out_of_range(const error_site site, exception_id id, const std::string& msg) +template +[[noreturn]] NLOHMANN_VIEW_NOINLINE void throw_out_of_range(const error_site site, exception_id id, const std::string& msg) { - NLOHMANN_VIEW_THROW(out_of_range::create(id, concat(diagnostics(site), msg), nullptr)); + NLOHMANN_VIEW_THROW(out_of_range::create(id, concat(diagnostics<>(site), msg), nullptr)); } -[[noreturn]] NLOHMANN_VIEW_NOINLINE inline void throw_invalid_iterator(const error_site site, exception_id id, const char* msg) +template +[[noreturn]] NLOHMANN_VIEW_NOINLINE void throw_invalid_iterator(const error_site site, exception_id id, const char* msg) { - NLOHMANN_VIEW_THROW(invalid_iterator::create(id, concat(diagnostics(site), msg), nullptr)); + NLOHMANN_VIEW_THROW(invalid_iterator::create(id, concat(diagnostics<>(site), msg), nullptr)); } /// a parse error without a position (as those of json_pointer) @@ -347,7 +358,8 @@ template } /// out_of_range.402: the JSON Pointer token '-' of an array of @a size elements -[[noreturn]] NLOHMANN_VIEW_NOINLINE inline void throw_array_index_past_end(const error_site site, std::size_t size) +template +[[noreturn]] NLOHMANN_VIEW_NOINLINE void throw_array_index_past_end(const error_site site, std::size_t size) { throw_out_of_range(site, exception_id::pointer_past_the_end_index, concat("array index '-' (", std::to_string(size), ") is out of range")); } diff --git a/include/nlohmann/detail/view/iterator.hpp b/include/nlohmann/detail/view/iterator.hpp index e53946912..fa0cc744e 100644 --- a/include/nlohmann/detail/view/iterator.hpp +++ b/include/nlohmann/detail/view/iterator.hpp @@ -67,17 +67,9 @@ class view_iterator /// @param[in] pos the element, or the key of the member /// @param[in] object whether pos is a key (its value is the next node) - /// @param[in] container the value iterated over (the context of the - /// exceptions, kept only with JSON_DIAGNOSTICS or - /// JSON_DIAGNOSTIC_POSITIONS) - view_iterator(const document_data* d, const node* pos, bool object, const node* container) noexcept + view_iterator(const document_data* d, const node* pos, bool object) noexcept : m_doc(d), m_pos(pos), m_value_offset(object ? 1 : 0) -#if JSON_DIAGNOSTICS || JSON_DIAGNOSTIC_POSITIONS - , m_container(container) -#endif - { - static_cast(container); - } + {} NLOHMANN_VIEW_ALWAYS_INLINE View operator*() const noexcept { @@ -113,12 +105,15 @@ class view_iterator } /// the key of the current object member; throws invalid_iterator.207 for - /// other iterators, like basic_json's iterators + /// other iterators, like basic_json's iterators (but without the context + /// JSON_DIAGNOSTICS and JSON_DIAGNOSTIC_POSITIONS add: the iterator does not + /// know the value it iterates over, and keeping it would make iterators + /// larger, which keeps GCC from inlining recursive walks) string_view_t key() const { if (NLOHMANN_VIEW_UNLIKELY(m_value_offset == 0)) { - throw_invalid_iterator(site(), exception_id::iterator_key_not_object, "cannot use key() for non-object iterators"); + throw_invalid_iterator(exception_id::iterator_key_not_object, "cannot use key() for non-object iterators"); } return string_view_t(m_doc->str(*m_pos), m_pos->len); } @@ -135,22 +130,9 @@ class view_iterator } private: - /// the value iterated over, as the context of an exception - error_site site() const noexcept - { -#if JSON_DIAGNOSTICS || JSON_DIAGNOSTIC_POSITIONS - return error_site(m_doc, m_container); -#else - return error_site(); -#endif - } - const document_data* m_doc = nullptr; const node* m_pos = nullptr; std::size_t m_value_offset = 0; ///< 1 for objects: the value follows its key -#if JSON_DIAGNOSTICS || JSON_DIAGNOSTIC_POSITIONS - const node* m_container = nullptr; -#endif }; /*! diff --git a/include/nlohmann/json_view.hpp b/include/nlohmann/json_view.hpp index 880610ca5..95f0422dc 100644 --- a/include/nlohmann/json_view.hpp +++ b/include/nlohmann/json_view.hpp @@ -481,7 +481,7 @@ class basic_json_view return end(); } const node* const k = find_key(key); - return k != nullptr ? iterator(m_doc, k, true, m_node) : end(); + return k != nullptr ? iterator(m_doc, k, true) : end(); } iterator find(const char* key) const @@ -544,18 +544,18 @@ class basic_json_view { if (NLOHMANN_VIEW_LIKELY(is_structured())) { - return iterator(m_doc, navigation::first(*m_doc, m_node), is_object(), m_node); + return iterator(m_doc, navigation::first(*m_doc, m_node), is_object()); } - return iterator(m_doc, m_node, false, m_node); + return iterator(m_doc, m_node, false); } NLOHMANN_VIEW_ALWAYS_INLINE iterator end() const noexcept { if (NLOHMANN_VIEW_LIKELY(is_structured())) { - return iterator(m_doc, navigation::end(*m_doc, m_node), is_object(), m_node); + return iterator(m_doc, navigation::end(*m_doc, m_node), is_object()); } - return iterator(m_doc, (is_null() || is_discarded()) ? m_node : m_node + 1, false, m_node); + return iterator(m_doc, (is_null() || is_discarded()) ? m_node : m_node + 1, false); } iterator cbegin() const noexcept diff --git a/single_include/nlohmann/json_view.hpp b/single_include/nlohmann/json_view.hpp index beaf95b01..faaa06bf4 100644 --- a/single_include/nlohmann/json_view.hpp +++ b/single_include/nlohmann/json_view.hpp @@ -4744,7 +4744,15 @@ void value_range(const document_data& d, const node* n, std::size_t& start, std: /// a value: "(/path) " with JSON_DIAGNOSTICS (none for the root), then /// "(bytes a-b) " with JSON_DIAGNOSTIC_POSITIONS (with JSON_DIAGNOSTICS, only /// if there is a path) -inline std::string diagnostics(const error_site site) +/// +/// This and the throw helpers that take an error_site are templates, so that +/// the path and range code is instantiated only where an exception can be +/// thrown: instantiated in every translation unit (as a non-template inline +/// function makes it), it changed GCC's early inlining of unrelated view +/// functions, so that GCC 13 no longer inlined a recursive walk over a view +/// into itself (2.7 times the instructions). +template +std::string diagnostics(const error_site site) { #if JSON_DIAGNOSTICS || JSON_DIAGNOSTIC_POSITIONS if (site.doc() == nullptr || site.value() == nullptr) @@ -4797,7 +4805,7 @@ inline std::string diagnostics(const error_site site) template std::string diagnostics_of(const View& v) { - return diagnostics(site_of(v)); + return diagnostics<>(site_of(v)); } // Exceptions are thrown out of line, so that the accessors that may throw stay @@ -4824,19 +4832,22 @@ std::string diagnostics_of(const View& v) NLOHMANN_VIEW_THROW(invalid_iterator::create(id, msg, nullptr)); } -[[noreturn]] NLOHMANN_VIEW_NOINLINE inline void throw_type_error(const error_site site, exception_id id, const char* prefix, const char* type) +template +[[noreturn]] NLOHMANN_VIEW_NOINLINE void throw_type_error(const error_site site, exception_id id, const char* prefix, const char* type) { - NLOHMANN_VIEW_THROW(type_error::create(id, concat(diagnostics(site), prefix, type), nullptr)); + NLOHMANN_VIEW_THROW(type_error::create(id, concat(diagnostics<>(site), prefix, type), nullptr)); } -[[noreturn]] NLOHMANN_VIEW_NOINLINE inline void throw_out_of_range(const error_site site, exception_id id, const std::string& msg) +template +[[noreturn]] NLOHMANN_VIEW_NOINLINE void throw_out_of_range(const error_site site, exception_id id, const std::string& msg) { - NLOHMANN_VIEW_THROW(out_of_range::create(id, concat(diagnostics(site), msg), nullptr)); + NLOHMANN_VIEW_THROW(out_of_range::create(id, concat(diagnostics<>(site), msg), nullptr)); } -[[noreturn]] NLOHMANN_VIEW_NOINLINE inline void throw_invalid_iterator(const error_site site, exception_id id, const char* msg) +template +[[noreturn]] NLOHMANN_VIEW_NOINLINE void throw_invalid_iterator(const error_site site, exception_id id, const char* msg) { - NLOHMANN_VIEW_THROW(invalid_iterator::create(id, concat(diagnostics(site), msg), nullptr)); + NLOHMANN_VIEW_THROW(invalid_iterator::create(id, concat(diagnostics<>(site), msg), nullptr)); } /// a parse error without a position (as those of json_pointer) @@ -4853,7 +4864,8 @@ template } /// out_of_range.402: the JSON Pointer token '-' of an array of @a size elements -[[noreturn]] NLOHMANN_VIEW_NOINLINE inline void throw_array_index_past_end(const error_site site, std::size_t size) +template +[[noreturn]] NLOHMANN_VIEW_NOINLINE void throw_array_index_past_end(const error_site site, std::size_t size) { throw_out_of_range(site, exception_id::pointer_past_the_end_index, concat("array index '-' (", std::to_string(size), ") is out of range")); } @@ -7748,17 +7760,9 @@ class view_iterator /// @param[in] pos the element, or the key of the member /// @param[in] object whether pos is a key (its value is the next node) - /// @param[in] container the value iterated over (the context of the - /// exceptions, kept only with JSON_DIAGNOSTICS or - /// JSON_DIAGNOSTIC_POSITIONS) - view_iterator(const document_data* d, const node* pos, bool object, const node* container) noexcept + view_iterator(const document_data* d, const node* pos, bool object) noexcept : m_doc(d), m_pos(pos), m_value_offset(object ? 1 : 0) -#if JSON_DIAGNOSTICS || JSON_DIAGNOSTIC_POSITIONS - , m_container(container) -#endif - { - static_cast(container); - } + {} NLOHMANN_VIEW_ALWAYS_INLINE View operator*() const noexcept { @@ -7794,12 +7798,15 @@ class view_iterator } /// the key of the current object member; throws invalid_iterator.207 for - /// other iterators, like basic_json's iterators + /// other iterators, like basic_json's iterators (but without the context + /// JSON_DIAGNOSTICS and JSON_DIAGNOSTIC_POSITIONS add: the iterator does not + /// know the value it iterates over, and keeping it would make iterators + /// larger, which keeps GCC from inlining recursive walks) string_view_t key() const { if (NLOHMANN_VIEW_UNLIKELY(m_value_offset == 0)) { - throw_invalid_iterator(site(), exception_id::iterator_key_not_object, "cannot use key() for non-object iterators"); + throw_invalid_iterator(exception_id::iterator_key_not_object, "cannot use key() for non-object iterators"); } return string_view_t(m_doc->str(*m_pos), m_pos->len); } @@ -7816,22 +7823,9 @@ class view_iterator } private: - /// the value iterated over, as the context of an exception - error_site site() const noexcept - { -#if JSON_DIAGNOSTICS || JSON_DIAGNOSTIC_POSITIONS - return error_site(m_doc, m_container); -#else - return error_site(); -#endif - } - const document_data* m_doc = nullptr; const node* m_pos = nullptr; std::size_t m_value_offset = 0; ///< 1 for objects: the value follows its key -#if JSON_DIAGNOSTICS || JSON_DIAGNOSTIC_POSITIONS - const node* m_container = nullptr; -#endif }; /*! @@ -9835,7 +9829,7 @@ class basic_json_view return end(); } const node* const k = find_key(key); - return k != nullptr ? iterator(m_doc, k, true, m_node) : end(); + return k != nullptr ? iterator(m_doc, k, true) : end(); } iterator find(const char* key) const @@ -9898,18 +9892,18 @@ class basic_json_view { if (NLOHMANN_VIEW_LIKELY(is_structured())) { - return iterator(m_doc, navigation::first(*m_doc, m_node), is_object(), m_node); + return iterator(m_doc, navigation::first(*m_doc, m_node), is_object()); } - return iterator(m_doc, m_node, false, m_node); + return iterator(m_doc, m_node, false); } NLOHMANN_VIEW_ALWAYS_INLINE iterator end() const noexcept { if (NLOHMANN_VIEW_LIKELY(is_structured())) { - return iterator(m_doc, navigation::end(*m_doc, m_node), is_object(), m_node); + return iterator(m_doc, navigation::end(*m_doc, m_node), is_object()); } - return iterator(m_doc, (is_null() || is_discarded()) ? m_node : m_node + 1, false, m_node); + return iterator(m_doc, (is_null() || is_discarded()) ? m_node : m_node + 1, false); } iterator cbegin() const noexcept diff --git a/tests/src/unit-json_view.cpp b/tests/src/unit-json_view.cpp index db7e1b6d5..c823ef9e2 100644 --- a/tests/src/unit-json_view.cpp +++ b/tests/src/unit-json_view.cpp @@ -970,7 +970,8 @@ TEST_CASE("json_view element access and iteration") CHECK(exception_of([&] { static_cast(v.at(5)); }) == exception_of([&] { static_cast(j.at(5)); })); if (!(j.is_object() && j.empty())) // (key() of an end iterator) { - CHECK(exception_of([&] { static_cast(v.begin().key()); }) == exception_of([&] { static_cast(j.begin().key()); })); + // (without the context of basic_json: view iterators do not know the value they iterate over) + CHECK(exception_of([&] { static_cast(v.begin().key()); }) == (j.is_object() ? "" : "[json.exception.invalid_iterator.207] cannot use key() for non-object iterators")); } if (!j.empty() || j.is_null()) { diff --git a/tests/src/unit-json_view_positions.cpp b/tests/src/unit-json_view_positions.cpp index b187365b3..7eb46f506 100644 --- a/tests/src/unit-json_view_positions.cpp +++ b/tests/src/unit-json_view_positions.cpp @@ -113,7 +113,8 @@ void check_exceptions(const View& v, const json& j) if (!v.is_object()) { CHECK(exception_of_call([&] { static_cast(v["k"]); }) == exception_of_call([&] { static_cast(j["k"]); })); - CHECK(exception_of_call([&] { static_cast(v.begin().key()); }) == exception_of_call([&] { static_cast(j.begin().key()); })); + // (without the context of basic_json: view iterators do not know the value they iterate over) + CHECK(exception_of_call([&] { static_cast(v.begin().key()); }) == "[json.exception.invalid_iterator.207] cannot use key() for non-object iterators"); } if (!v.is_array()) {