Don't keep a separate at_start boolean in object

.circleci/config.yml

@@ -105,13 +105,12 @@ commands:
           ctest $CTEST_FLAGS -L acceptance &&
           ctest $CTEST_FLAGS -LE acceptance -LE explicitonly
 
-  cmake_test:
+  cmake_assert_test:
     steps:
       - run: |
           cd build &&
           tools/json2json -h &&
-          ctest $CTEST_FLAGS -L assert  &&
-          ctest $CTEST_FLAGS -LE acceptance -LE explicitonly
+          ctest $CTEST_FLAGS -L assert
 
   cmake_test_all:
     steps:
@@ -155,12 +154,12 @@ jobs:
   assert-gcc10:
     description: Build the library with asserts on, install it and run tests
     executor: gcc10
-    environment: { CMAKE_FLAGS: -DSIMDJSON_GOOGLE_BENCHMARKS=OFF -DCXX_CMAKE_FLAGS_RELEASE=-O3 }
+    environment: { CMAKE_FLAGS: -DSIMDJSON_GOOGLE_BENCHMARKS=OFF -DCMAKE_CXX_FLAGS_RELEASE=-O3 }
     steps: [ cmake_test, cmake_assert_test ]
   assert-clang10:
     description: Build just the library, install it and do a basic test
     executor: clang10
-    environment: { CMAKE_FLAGS: -DSIMDJSON_GOOGLE_BENCHMARKS=OFF -DCXX_CMAKE_FLAGS_RELEASE=-O3 }
+    environment: { CMAKE_FLAGS: -DSIMDJSON_GOOGLE_BENCHMARKS=OFF -DCMAKE_CXX_FLAGS_RELEASE=-O3 }
     steps: [ cmake_test, cmake_assert_test ]
   gcc10-perftest:
     description: Build and run performance tests on GCC 10 and AVX 2 with a cmake static build, this test performance regression

include/simdjson/generic/implementation_simdjson_result_base-inl.h

@@ -5,13 +5,6 @@ namespace SIMDJSON_IMPLEMENTATION {
 // internal::implementation_simdjson_result_base<T> inline implementation
 //
 
-/**
- * Create a new empty result with error = UNINITIALIZED.
- */
-template<typename T>
-simdjson_really_inline implementation_simdjson_result_base<T>::~implementation_simdjson_result_base() noexcept {
-}
-
 template<typename T>
 simdjson_really_inline void implementation_simdjson_result_base<T>::tie(T &value, error_code &error) && noexcept {
   // on the clang compiler that comes with current macOS (Apple clang version 11.0.0),
@@ -70,9 +63,6 @@ simdjson_really_inline implementation_simdjson_result_base<T>::implementation_si
 template<typename T>
 simdjson_really_inline implementation_simdjson_result_base<T>::implementation_simdjson_result_base(T &&value) noexcept
     : implementation_simdjson_result_base(std::forward<T>(value), SUCCESS) {}
-template<typename T>
-simdjson_really_inline implementation_simdjson_result_base<T>::implementation_simdjson_result_base() noexcept
-    : implementation_simdjson_result_base(T{}, UNINITIALIZED) {}
 
 } // namespace SIMDJSON_IMPLEMENTATION
 } // namespace simdjson

include/simdjson/generic/implementation_simdjson_result_base.h

@@ -30,7 +30,7 @@ struct implementation_simdjson_result_base {
   /**
    * Create a new empty result with error = UNINITIALIZED.
    */
-  simdjson_really_inline implementation_simdjson_result_base() noexcept;
+  simdjson_really_inline implementation_simdjson_result_base() noexcept = default;
 
   /**
    * Create a new error result.
@@ -47,11 +47,6 @@ struct implementation_simdjson_result_base {
    */
   simdjson_really_inline implementation_simdjson_result_base(T &&value, error_code error) noexcept;
 
-  /**
-   * Create a new empty result with error = UNINITIALIZED.
-   */
-  simdjson_really_inline ~implementation_simdjson_result_base() noexcept;
-
   /**
    * Move the value and the error to the provided variables.
    *
@@ -104,8 +99,8 @@ struct implementation_simdjson_result_base {
 
 #endif // SIMDJSON_EXCEPTIONS
 
-  T first;
-  error_code second;
+  T first{};
+  error_code second{UNINITIALIZED};
 }; // struct implementation_simdjson_result_base
 
 } // namespace SIMDJSON_IMPLEMENTATION

include/simdjson/generic/ondemand/document.h

@@ -20,17 +20,16 @@ class array_iterator;
  */
 class document {
 public:
-  simdjson_really_inline document(document &&other) noexcept = default;
-  simdjson_really_inline document &operator=(document &&other) noexcept = default;
-
   /**
    * Create a new invalid document.
    *
    * Exists so you can declare a variable and later assign to it before use.
    */
   simdjson_really_inline document() noexcept = default;
-  simdjson_really_inline document(const document &other) = delete;
-  simdjson_really_inline document &operator=(const document &other) = delete;
+  simdjson_really_inline document(const document &other) noexcept = delete;
+  simdjson_really_inline document(document &&other) noexcept = default;
+  simdjson_really_inline document &operator=(const document &other) noexcept = delete;
+  simdjson_really_inline document &operator=(document &&other) noexcept = default;
 
   /**
    * Cast this JSON value to an array.

include/simdjson/generic/ondemand/object-inl.h

@@ -43,18 +43,18 @@ namespace ondemand {
 //   In this state, iter.depth < depth, at_start == false, and error == SUCCESS.
 //
 
-simdjson_warn_unused simdjson_really_inline error_code object::find_field_raw(const std::string_view key) noexcept {
-  return iter.find_field_raw(key);
-}
-
 simdjson_really_inline simdjson_result<value> object::operator[](const std::string_view key) & noexcept {
-  SIMDJSON_TRY( find_field_raw(key) );
-  return value(iter.iter.child());
+  bool has_value;
+  SIMDJSON_TRY( iter.find_field_raw(key).get(has_value) );
+  if (!has_value) { return NO_SUCH_FIELD; }
+  return value(iter.child());
 }
 
 simdjson_really_inline simdjson_result<value> object::operator[](const std::string_view key) && noexcept {
-  SIMDJSON_TRY( find_field_raw(key) );
-  return value(iter.iter.child());
+  bool has_value;
+  SIMDJSON_TRY( iter.find_field_raw(key).get(has_value) );
+  if (!has_value) { return NO_SUCH_FIELD; }
+  return value(iter.child());
 }
 
 simdjson_really_inline simdjson_result<object> object::start(value_iterator &iter) noexcept {
@@ -72,16 +72,17 @@ simdjson_really_inline object object::started(value_iterator &iter) noexcept {
   return iter;
 }
 simdjson_really_inline object object::resume(const value_iterator &iter) noexcept {
-  return { iter, false };
+  return iter;
 }
 
-simdjson_really_inline object::object(const value_iterator &_iter, bool _at_start) noexcept
-  : iter{_iter, _at_start}    
+simdjson_really_inline object::object(const value_iterator &_iter) noexcept
+  : iter{_iter}
 {
 }
 
 simdjson_really_inline object_iterator object::begin() noexcept {
-  SIMDJSON_ASSUME( iter.at_start || iter.iter._json_iter->_depth < iter.iter._depth );
+  // Expanded version of SIMDJSON_ASSUME( iter.at_field_start() || !iter.is_open() )
+  SIMDJSON_ASSUME( (iter._json_iter->token.index == iter._start_index + 1) || (iter._json_iter->_depth < iter._depth) );
   return iter;
 }
 simdjson_really_inline object_iterator object::end() noexcept {

include/simdjson/generic/ondemand/object.h

@@ -50,11 +50,11 @@ protected:
   static simdjson_really_inline simdjson_result<object> try_start(value_iterator &iter) noexcept;
   static simdjson_really_inline object started(value_iterator &iter) noexcept;
   static simdjson_really_inline object resume(const value_iterator &iter) noexcept;
-  simdjson_really_inline object(const value_iterator &iter, bool at_start = true) noexcept;
+  simdjson_really_inline object(const value_iterator &iter) noexcept;
 
   simdjson_warn_unused simdjson_really_inline error_code find_field_raw(const std::string_view key) noexcept;
 
-  object_iterator iter{};
+  value_iterator iter{};
 
   friend class value;
   friend class document;

include/simdjson/generic/ondemand/object_iterator-inl.h

@@ -6,9 +6,8 @@ namespace ondemand {
 // object_iterator
 //
 
-simdjson_really_inline object_iterator::object_iterator(const value_iterator &_iter, bool _at_start) noexcept
-  : iter{_iter},
-    at_start{_at_start}
+simdjson_really_inline object_iterator::object_iterator(const value_iterator &_iter) noexcept
+  : iter{_iter}
 {}
 
 simdjson_really_inline simdjson_result<field> object_iterator::operator*() noexcept {
@@ -80,39 +79,6 @@ simdjson_really_inline object_iterator &object_iterator::operator++() noexcept {
 //   In this state, iter.depth < depth, at_start == false, and error == SUCCESS.
 //
 
-simdjson_warn_unused simdjson_really_inline error_code object_iterator::find_field_raw(const std::string_view key) noexcept {
-  if (!iter.is_open()) { return NO_SUCH_FIELD; }
-
-  // Unless this is the first field, we need to advance past the , and check for }
-  error_code error;
-  bool has_value;
-  if (at_start) {
-    at_start = false;
-    has_value = true;
-  } else {
-    if ((error = iter.skip_child() )) { iter.abandon(); return error; }
-    if ((error = iter.has_next_field().get(has_value) )) { iter.abandon(); return error; }
-  }
-  while (has_value) {
-    // Get the key
-    raw_json_string actual_key;
-    if ((error = iter.field_key().get(actual_key) )) { iter.abandon(); return error; };
-    if ((error = iter.field_value() )) { iter.abandon(); return error; }
-
-    // Check if it matches
-    if (actual_key == key) {
-      logger::log_event(iter, "match", key, -2);
-      return SUCCESS;
-    }
-    logger::log_event(iter, "no match", key, -2);
-    SIMDJSON_TRY( iter.skip_child() ); // Skip the value entirely
-    if ((error = iter.has_next_field().get(has_value) )) { iter.abandon(); return error; }
-  }
-
-  // If the loop ended, we're out of fields to look at.
-  return NO_SUCH_FIELD;
-}
-
 } // namespace ondemand
 } // namespace SIMDJSON_IMPLEMENTATION
 } // namespace simdjson

include/simdjson/generic/ondemand/object_iterator.h

@@ -29,11 +29,6 @@ public:
   // Checks for ']' and ','
   simdjson_really_inline object_iterator &operator++() noexcept;
 
-  /**
-   * Find the field with the given key. May be used in place of ++.
-   */
-  simdjson_warn_unused simdjson_really_inline error_code find_field_raw(const std::string_view key) noexcept;
-
 private:
   /**
    * The underlying JSON iterator.
@@ -42,17 +37,8 @@ private:
    * is first used, and never changes afterwards.
    */
   value_iterator iter{};
-  /**
-   * Whether we are at the start.
-   *
-   * PERF NOTE: this should be elided into inline control flow: it is only used for the first []
-   * or * call, and SSA optimizers commonly do first-iteration loop optimization.
-   *
-   * SAFETY: this is not safe; the object_iterator can be copied freely, so the state CAN be lost.
-   */
-  bool at_start{};
 
-  simdjson_really_inline object_iterator(const value_iterator &iter, bool at_start = true) noexcept;
+  simdjson_really_inline object_iterator(const value_iterator &iter) noexcept;
   friend struct simdjson_result<object_iterator>;
   friend class object;
 };

include/simdjson/generic/ondemand/token_iterator.h

@@ -83,6 +83,7 @@ protected:
 
   friend class json_iterator;
   friend class value_iterator;
+  friend class object;
   friend simdjson_really_inline void logger::log_line(const json_iterator &iter, const char *title_prefix, const char *title, std::string_view detail, int delta, int depth_delta) noexcept;
 };
 

include/simdjson/generic/ondemand/value_iterator-inl.h

@@ -51,31 +51,38 @@ simdjson_warn_unused simdjson_really_inline simdjson_result<bool> value_iterator
   }
 }
 
-simdjson_warn_unused simdjson_really_inline simdjson_result<bool> value_iterator::find_field_raw(const char *key) noexcept {
-  // We assume we are sitting at a key: at "key": <value>
-  assert_at_child();
+/**
+ * Find the field with the given key. May be used in place of ++.
+ */
+simdjson_warn_unused simdjson_really_inline simdjson_result<bool> value_iterator::find_field_raw(const std::string_view key) noexcept {
+  if (!is_open()) { return false; }
 
-  bool has_next;
-  do {
+  // Unless this is the first field, we need to advance past the , and check for }
+  error_code error;
+  bool has_value;
+  if (at_first_field()) {
+    has_value = true;
+  } else {
+    if ((error = skip_child() )) { abandon(); return error; }
+    if ((error = has_next_field().get(has_value) )) { abandon(); return error; }
+  }
+  while (has_value) {
     // Get the key
     raw_json_string actual_key;
-    SIMDJSON_TRY( require_raw_json_string().get(actual_key) );
-    if (*_json_iter->advance() != ':') { return _json_iter->report_error(TAPE_ERROR, "Missing colon in object field"); }
+    if ((error = field_key().get(actual_key) )) { abandon(); return error; };
+    if ((error = field_value() )) { abandon(); return error; }
 
-    // Check if the key matches, and return if so
+    // Check if it matches
     if (actual_key == key) {
-      logger::log_event(*_json_iter, "match", key);
+      logger::log_event(*this, "match", key, -2);
       return true;
     }
+    logger::log_event(*this, "no match", key, -2);
+    SIMDJSON_TRY( skip_child() ); // Skip the value entirely
+    if ((error = has_next_field().get(has_value) )) { abandon(); return error; }
+  }
 
-    // Skip the value so we can look at the next key
-    logger::log_event(*_json_iter, "non-match", key);
-    SIMDJSON_TRY( skip_child() );
-
-    // Check whether the next token is , or }
-    SIMDJSON_TRY( has_next_field().get(has_next) );
-  } while (has_next);
-  logger::log_event(*_json_iter, "no matches", key);
+  // If the loop ended, we're out of fields to look at.
   return false;
 }
 
@@ -382,6 +389,11 @@ simdjson_really_inline bool value_iterator::is_open() const noexcept {
   return _json_iter->depth() >= depth();
 }
 
+simdjson_really_inline bool value_iterator::at_first_field() const noexcept {
+  SIMDJSON_ASSUME( _json_iter->token.index > _start_index );
+  return _json_iter->token.index == _start_index + 1;
+}
+
 simdjson_really_inline void value_iterator::abandon() noexcept {
   _json_iter->abandon();
 }

include/simdjson/generic/ondemand/value_iterator.h

@@ -55,6 +55,11 @@ public:
    */
   simdjson_really_inline bool is_open() const noexcept;
 
+  /**
+   * Tell whether the value is at an object's first field (just after the {).
+   */
+  simdjson_really_inline bool at_first_field() const noexcept;
+
   /**
    * Abandon all iteration.
    */
@@ -158,7 +163,7 @@ public:
    * unescape it. This works well for typical ASCII and UTF-8 keys (almost all of them), but may
    * fail to match some keys with escapes (\u, \n, etc.).
    */
-  simdjson_warn_unused simdjson_really_inline simdjson_result<bool> find_field_raw(const char *key) noexcept;
+  simdjson_warn_unused simdjson_really_inline simdjson_result<bool> find_field_raw(const std::string_view key) noexcept;
 
   /** @} */
 
@@ -262,10 +267,10 @@ protected:
   simdjson_really_inline simdjson_result<double> parse_root_double(const uint8_t *json, uint32_t max_len) const noexcept;
 
   simdjson_really_inline void assert_at_start() const noexcept;
-  simdjson_really_inline void assert_at_root() const noexcept; 
+  simdjson_really_inline void assert_at_root() const noexcept;
   simdjson_really_inline void assert_at_child() const noexcept;
-  simdjson_really_inline void assert_at_next() const noexcept; 
-  simdjson_really_inline void assert_at_non_root_start() const noexcept; 
+  simdjson_really_inline void assert_at_next() const noexcept;
+  simdjson_really_inline void assert_at_non_root_start() const noexcept;
 
   friend class document;
   friend class object;

tests/ondemand/ondemand_assert_out_of_order_values.cpp

@@ -10,7 +10,7 @@ using namespace simdjson::builtin;
 
 // This ensures the compiler can't rearrange them into the proper order (which causes it to work!)
 simdjson_never_inline int check_point(simdjson_result<ondemand::value> xval, simdjson_result<ondemand::value> yval) {
-  // Verify the expected release behavior 
+  // Verify the expected release behavior
   error_code error;
   uint64_t x = 0;
   if ((error = xval.get(x))) { std::cerr << "error getting x: " << error << std::endl; }
@@ -21,26 +21,36 @@ simdjson_never_inline int check_point(simdjson_result<ondemand::value> xval, sim
   return 0;
 }
 
+int test_check_point() {
+  auto json = R"(
+    {
+      "x": 1,
+      "y": 2 3
+  )"_padded;
+  ondemand::parser parser;
+  auto doc = parser.iterate(json);
+  return check_point(doc["x"], doc["y"]);
+}
+
+bool fork_failed_with_assert() {
+  int status = 0;
+  wait(&status);
+  return WIFSIGNALED(status);
+}
+
 int main(void) {
+
+  // To verify that the program asserts (which sends a signal), we fork a new process and use wait()
+  // to check the resulting error code. If it's a signal error code, we consider the
+  // test to have passed.
+  // From https://stackoverflow.com/a/33694733
+
   pid_t pid = fork();
   if (pid == -1) { std::cerr << "fork failed" << std::endl; return 1; }
   if (pid) {
     // Parent - wait child and interpret its result
-    int status = 0;
-    wait(&status);
-    if(!WIFSIGNALED(status)) {
-      std::cerr << "Expected program to abort, but it exited successfully with status " << status << std::endl;
-      return 1;
-    }
-    return 0; // Signal-terminated means success
+    return fork_failed_with_assert() ? 0 : 1;
   } else {
-    auto json = R"(
-      {
-        "x": 1,
-        "y": 2 3
-    )"_padded;
-    ondemand::parser parser;
-    auto doc = parser.iterate(json);
-    return check_point(doc["x"], doc["y"]);
+    test_check_point();
   }
 }