Skip to content

Commit 392288b

Browse files
jkeiserjkeiser
committed
Check for EOF when skipping containers
Revert that? Or not
1 parent 80d61cb commit 392288b

File tree

6 files changed

+28
-37
lines changed

6 files changed

+28
-37
lines changed

‎benchmark/bench_sax.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ simdjson_really_inline void read_tweets(ondemand::parser &parser, padded_string
141141
if (!iter.has_next_field() || !iter.find_field_raw("screen_name")) { throw; }
142142
tweet.user.screen_name = iter.get_raw_json_string().value().unescape(iter);
143143

144-
iter.skip_container(); // Skip the rest of the user object
144+
if (iter.skip_container()) { throw; } // Skip the rest of the user object
145145
}
146146

147147
if (!iter.has_next_field() || !iter.find_field_raw("retweet_count")) { throw; }
@@ -152,7 +152,7 @@ simdjson_really_inline void read_tweets(ondemand::parser &parser, padded_string
152152

153153
tweets.push_back(tweet);
154154

155-
iter.skip_container(); // Skip the rest of the tweet object
155+
if (iter.skip_container()) { throw; } // Skip the rest of the tweet object
156156

157157
} while (iter.has_next_element());
158158
}

‎src/generic/ondemand/array-inl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ simdjson_really_inline array &array::operator=(array &&other) noexcept = default
5151
simdjson_really_inline array::~array() noexcept {
5252
if (iter.is_alive()) {
5353
logger::log_event(*iter, "unfinished", "array");
54-
iter->skip_container();
54+
SIMDJSON_UNUSED auto _err = iter->skip_container();
5555
iter.release();
5656
}
5757
}

‎src/generic/ondemand/json_iterator-inl.h

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ SIMDJSON_WARN_UNUSED simdjson_really_inline simdjson_result<bool> json_iterator:
7171
return true;
7272
}
7373
logger::log_event(*this, "non-match", key);
74-
skip(); // Skip the value so we can look at the next key
74+
SIMDJSON_TRY( skip() ); // Skip the value so we can look at the next key
7575

7676
SIMDJSON_TRY( has_next_field().get(has_next) );
7777
} while (has_next);
@@ -201,35 +201,23 @@ simdjson_really_inline bool json_iterator::root_is_null() noexcept {
201201
return is_null();
202202
}
203203

204-
205-
simdjson_really_inline void json_iterator::skip() noexcept {
206-
uint32_t depth = 0;
207-
do {
208-
switch (*advance()) {
209-
// TODO consider whether matching braces is a requirement: if non-matching braces indicates
210-
// *missing* braces, then future lookups are not in the object/arrays they think they are,
211-
// violating the rule "validate enough structure that the user can be confident they are
212-
// looking at the right values."
213-
case ']': case '}':
214-
logger::log_end_value(*this, "skip");
215-
depth--;
216-
break;
217-
// PERF TODO does it skip the depth check when we don't decrement depth?
218-
case '[': case '{':
219-
logger::log_start_value(*this, "skip");
220-
depth++;
221-
break;
222-
default:
223-
logger::log_value(*this, "skip", "");
224-
break;
225-
}
226-
} while (depth > 0);
204+
SIMDJSON_WARN_UNUSED simdjson_really_inline error_code json_iterator::skip() noexcept {
205+
switch (*advance()) {
206+
// PERF TODO does it skip the depth check when we don't decrement depth?
207+
case '[': case '{':
208+
logger::log_start_value(*this, "skip");
209+
return skip_container();
210+
default:
211+
logger::log_value(*this, "skip", "");
212+
return SUCCESS;
213+
}
227214
}
228215

229-
simdjson_really_inline bool json_iterator::skip_container() noexcept {
216+
SIMDJSON_WARN_UNUSED simdjson_really_inline error_code json_iterator::skip_container() noexcept {
230217
uint32_t depth = 1;
231218
// The loop breaks only when depth-- happens.
232-
while (true) {
219+
auto end = &parser->dom_parser.structural_indexes[parser->dom_parser.n_structural_indexes];
220+
while (index <= end) {
233221
uint8_t ch = *advance();
234222
switch (ch) {
235223
// TODO consider whether matching braces is a requirement: if non-matching braces indicates
@@ -239,7 +227,7 @@ simdjson_really_inline bool json_iterator::skip_container() noexcept {
239227
case ']': case '}':
240228
logger::log_end_value(*this, "skip");
241229
depth--;
242-
if (depth == 0) { logger::log_event(*this, "end skip", ""); return ch == ']'; }
230+
if (depth == 0) { logger::log_event(*this, "end skip", ""); return SUCCESS; }
243231
break;
244232
// PERF TODO does it skip the depth check when we don't decrement depth?
245233
case '[': case '{':
@@ -250,7 +238,10 @@ simdjson_really_inline bool json_iterator::skip_container() noexcept {
250238
logger::log_value(*this, "skip", "");
251239
break;
252240
}
253-
};
241+
}
242+
243+
logger::log_error(*this, "not enough close braces");
244+
return TAPE_ERROR;
254245
}
255246

256247
simdjson_really_inline bool json_iterator::at_start() const noexcept {

‎src/generic/ondemand/json_iterator.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,22 +111,22 @@ class json_iterator : public token_iterator {
111111
/**
112112
* Skips a JSON value, whether it is a scalar, array or object.
113113
*/
114-
simdjson_really_inline void skip() noexcept;
114+
SIMDJSON_WARN_UNUSED simdjson_really_inline error_code skip() noexcept;
115115

116116
/**
117117
* Skips to the end of a JSON object or array.
118118
*
119119
* @return true if this was the end of an array, false if it was the end of an object.
120120
*/
121-
simdjson_really_inline bool skip_container() noexcept;
121+
SIMDJSON_WARN_UNUSED simdjson_really_inline error_code skip_container() noexcept;
122122

123123
/**
124124
* Tell whether the iterator is still at the start
125125
*/
126126
simdjson_really_inline bool at_start() const noexcept;
127127

128128
/**
129-
* Tell whether the iterator has reached EOF
129+
* Tell whether the iterator is at the EOF mark
130130
*/
131131
simdjson_really_inline bool at_eof() const noexcept;
132132

‎src/generic/ondemand/object-inl.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ simdjson_really_inline object &object::operator=(object &&other) noexcept = defa
5656
simdjson_really_inline object::~object() noexcept {
5757
if (iter.is_alive()) {
5858
logger::log_event(*iter, "unfinished", "object");
59-
iter->skip_container();
59+
SIMDJSON_UNUSED auto _err = iter->skip_container();
6060
iter.release();
6161
}
6262
}
@@ -85,7 +85,7 @@ simdjson_really_inline simdjson_result<value> object::operator[](const std::stri
8585
return value::start(iter.borrow());
8686
}
8787
logger::log_event(*iter, "no match", key, -2);
88-
iter->skip(); // Skip the value entirely
88+
SIMDJSON_TRY( iter->skip() ); // Skip the value entirely
8989
if ((error = iter->has_next_field().get(has_value) )) { return report_error(); }
9090
}
9191

‎src/generic/ondemand/value-inl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ simdjson_really_inline value::~value() noexcept {
2121
if (iter.is_alive()) {
2222
if (*json == '[' || *json == '{') {
2323
logger::log_start_value(*iter, "unused");
24-
iter->skip_container();
24+
SIMDJSON_UNUSED auto _err = iter->skip_container();
2525
} else {
2626
logger::log_value(*iter, "unused");
2727
}

0 commit comments

Comments
 (0)