Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return a better error when a user tries to cast a document that has already been accessed to a value #2085

Merged
merged 4 commits into from
Nov 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions doc/basics.md
Original file line number Diff line number Diff line change
Expand Up @@ -542,9 +542,7 @@ support for users who avoid exceptions. See [the simdjson error handling documen
* **Tree Walking and JSON Element Types:** Sometimes you don't necessarily have a document
with a known type, and are trying to generically inspect or walk over JSON elements.
You can also represent arbitrary JSON values with
`ondemand::value` instances: it can represent anything except a scalar document (lone number, string, null or Boolean). You can check for scalar documents with the method `scalar()`.
You can query the type of a document or a value with the `type()` method.
The `type()` method does not consume or validate documents and values, but it tells you whether they are
`ondemand::value` instances: it can represent anything except a scalar document (lone number, string, null or Boolean). You can check for scalar documents with the method `scalar()`. You can cast a document that is either an array or an object to an `ondemand::value` instance immediately after you create the document instance: you cannot create a `ondemand::value` instance from a document that has already been accessed as it would mean that you would have two instances of the object or array simultaneously (see [rewinding](#rewinding)). You can query the type of a document or a value with the `type()` method. The `type()` method does not consume or validate documents and values, but it tells you whether they are
- arrays (`json_type::array`),
- objects (`json_type::object`)
- numbers (`json_type::number`),
Expand Down
9 changes: 8 additions & 1 deletion include/simdjson/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ enum error_code {
INVALID_URI_FRAGMENT, ///< Invalid URI fragment
UNEXPECTED_ERROR, ///< indicative of a bug in simdjson
PARSER_IN_USE, ///< parser is already in use.
OUT_OF_ORDER_ITERATION, ///< tried to iterate an array or object out of order
OUT_OF_ORDER_ITERATION, ///< tried to iterate an array or object out of order (checked when SIMDJSON_DEVELOPMENT_CHECKS=1)
INSUFFICIENT_PADDING, ///< The JSON doesn't have enough padding for simdjson to safely parse it.
INCOMPLETE_ARRAY_OR_OBJECT, ///< The document ends early.
SCALAR_DOCUMENT_AS_VALUE, ///< A scalar document is treated as a value.
Expand All @@ -51,6 +51,13 @@ enum error_code {
NUM_ERROR_CODES
};

/**
* It is the convention throughout the code that the macro SIMDJSON_DEVELOPMENT_CHECKS determines whether
* we check for OUT_OF_ORDER_ITERATION. The logic behind it is that these errors only occurs when the code
* that was written while breaking some simdjson::ondemand requirement. They should not occur in released
* code after these issues were fixed.
*/

/**
* Get the error message for the given error code.
*
Expand Down
12 changes: 11 additions & 1 deletion include/simdjson/generic/ondemand/document-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,17 @@ simdjson_inline simdjson_result<object> document::start_or_resume_object() noexc
simdjson_inline simdjson_result<value> document::get_value() noexcept {
// Make sure we start any arrays or objects before returning, so that start_root_<object/array>()
// gets called.
iter.assert_at_document_depth();

// It is the convention throughout the code that the macro `SIMDJSON_DEVELOPMENT_CHECKS` determines whether
// we check for OUT_OF_ORDER_ITERATION. Proper on::demand code should never trigger this error.
#if SIMDJSON_DEVELOPMENT_CHECKS
if (!iter.at_root()) { return OUT_OF_ORDER_ITERATION; }
lemire marked this conversation as resolved.
Show resolved Hide resolved
#endif
// assert_at_root() serves two purposes: in Debug mode, whether or not
// SIMDJSON_DEVELOPMENT_CHECKS is set or not, it checks that we are at the root of
// the document (this will typically be redundant). In release mode, it generates
// SIMDJSON_ASSUME statements to allow the compiler to make assumptions.
iter.assert_at_root();
lemire marked this conversation as resolved.
Show resolved Hide resolved
switch (*iter.peek()) {
case '[': {
// The following lines check that the document ends with ].
Expand Down
17 changes: 14 additions & 3 deletions include/simdjson/generic/ondemand/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ class document {
/**
* Cast this JSON value to a value when the document is an object or an array.
*
* You must not have begun iterating through the object or array. When
* SIMDJSON_DEVELOPMENT_CHECKS is set to 1 (which is the case when building in Debug mode
* by default), and you have already begun iterating,
* you will get an OUT_OF_ORDER_ITERATION error. If you have begun iterating, you can use
* rewind() to reset the document to its initial state before calling this method.
*
* @returns A value if a JSON array or object cannot be found.
* @returns SCALAR_DOCUMENT_AS_VALUE error is the document is a scalar (see is_scalar() function).
*/
Expand Down Expand Up @@ -268,10 +274,15 @@ class document {
*/
simdjson_inline operator bool() noexcept(false);
/**
* Cast this JSON value to a value.
* Cast this JSON value to a value when the document is an object or an array.
*
* You must not have begun iterating through the object or array. When
* SIMDJSON_DEVELOPMENT_CHECKS is defined, and you have already begun iterating,
* you will get an OUT_OF_ORDER_ITERATION error. If you have begun iterating, you can use
* rewind() to reset the document to its initial state before calling this method.
*
* @returns A value value.
* @exception if a JSON value cannot be found
* @returns A value value if a JSON array or object cannot be found.
* @exception SCALAR_DOCUMENT_AS_VALUE error is the document is a scalar (see is_scalar() function).
*/
simdjson_inline operator value() noexcept(false);
#endif
Expand Down