ref qualify parser methods to avoid use of dangling objects (#703)

To avoid using data belonging to a temporary, the parse functions are ref qualified to get a compile error if used on an rvalue. See https://github.com/simdjson/simdjson/issues/696

Compilation tests are also added, to make sure bad usage fails to compile.

Reviewed by jkeiser.

benchmark/parsingcompetition.cpp

@@ -108,9 +108,14 @@ bool bench(const char *filename, bool verbose, bool just_data, int repeat_multip
        repeat, volume, !just_data);
   }
 
-  if (!just_data)
-    BEST_TIME("simdjson (dynamic mem) ", simdjson::dom::parser().parse(p).error(), simdjson::SUCCESS,
+  if (!just_data) {
+    auto parse_dynamic=[](auto& str){
+        simdjson::dom::parser parser;
+        return parser.parse(str).error();
+    };
+    BEST_TIME("simdjson (dynamic mem) ", parse_dynamic(p), simdjson::SUCCESS,
               , repeat, volume, !just_data);
+  }
   // (static alloc)
   simdjson::dom::parser parser;
   BEST_TIME("simdjson ", parser.parse(p).error(), simdjson::SUCCESS, , repeat, volume,

include/simdjson/document.h

@@ -680,8 +680,8 @@ public:
    *         - CAPACITY if the parser does not have enough capacity and len > max_capacity.
    *         - other json errors if parsing fails.
    */
-  inline simdjson_result<element> load(const std::string &path) noexcept; 
-
+  inline simdjson_result<element> load(const std::string &path) & noexcept;
+  inline simdjson_result<element> load(const std::string &path) &&  = delete ;
   /**
    * Parse a JSON document and return a temporary reference to it.
    *
@@ -717,13 +717,17 @@ public:
    *         - CAPACITY if the parser does not have enough capacity and len > max_capacity.
    *         - other json errors if parsing fails.
    */
-  inline simdjson_result<element> parse(const uint8_t *buf, size_t len, bool realloc_if_needed = true) noexcept;
+  inline simdjson_result<element> parse(const uint8_t *buf, size_t len, bool realloc_if_needed = true) & noexcept;
+  inline simdjson_result<element> parse(const uint8_t *buf, size_t len, bool realloc_if_needed = true) && =delete;
   /** @overload parse(const uint8_t *buf, size_t len, bool realloc_if_needed) */
-  really_inline simdjson_result<element> parse(const char *buf, size_t len, bool realloc_if_needed = true) noexcept;
+  really_inline simdjson_result<element> parse(const char *buf, size_t len, bool realloc_if_needed = true) & noexcept;
+  really_inline simdjson_result<element> parse(const char *buf, size_t len, bool realloc_if_needed = true) && =delete;
   /** @overload parse(const uint8_t *buf, size_t len, bool realloc_if_needed) */
-  really_inline simdjson_result<element> parse(const std::string &s) noexcept;
+  really_inline simdjson_result<element> parse(const std::string &s) & noexcept;
+  really_inline simdjson_result<element> parse(const std::string &s) && =delete;
   /** @overload parse(const uint8_t *buf, size_t len, bool realloc_if_needed) */
-  really_inline simdjson_result<element> parse(const padded_string &s) noexcept;
+  really_inline simdjson_result<element> parse(const padded_string &s) & noexcept;
+  really_inline simdjson_result<element> parse(const padded_string &s) && =delete;
 
   /** @private We do not want to allow implicit conversion from C string to std::string. */
   really_inline simdjson_result<element> parse(const char *buf) noexcept = delete;
@@ -1303,4 +1307,4 @@ public:
 
 } // namespace simdjson
 
-#endif // SIMDJSON_DOCUMENT_H
+#endif // SIMDJSON_DOCUMENT_H

include/simdjson/inline/document.h

@@ -366,7 +366,7 @@ inline simdjson_result<size_t> parser::read_file(const std::string &path) noexce
   return bytes_read;
 }
 
-inline simdjson_result<element> parser::load(const std::string &path) noexcept {
+inline simdjson_result<element> parser::load(const std::string &path) & noexcept {
   size_t len;
   error_code code;
   read_file(path).tie(len, code);
@@ -382,7 +382,7 @@ inline document_stream parser::load_many(const std::string &path, size_t batch_s
   return document_stream(*this, (const uint8_t*)loaded_bytes.get(), len, batch_size, code);
 }
 
-inline simdjson_result<element> parser::parse(const uint8_t *buf, size_t len, bool realloc_if_needed) noexcept {
+inline simdjson_result<element> parser::parse(const uint8_t *buf, size_t len, bool realloc_if_needed) & noexcept {
   error_code code = ensure_capacity(len);
   if (code) { return code; }
 
@@ -405,13 +405,13 @@ inline simdjson_result<element> parser::parse(const uint8_t *buf, size_t len, bo
   error = UNINITIALIZED;
   return doc.root();
 }
-really_inline simdjson_result<element> parser::parse(const char *buf, size_t len, bool realloc_if_needed) noexcept {
+really_inline simdjson_result<element> parser::parse(const char *buf, size_t len, bool realloc_if_needed) & noexcept {
   return parse((const uint8_t *)buf, len, realloc_if_needed);
 }
-really_inline simdjson_result<element> parser::parse(const std::string &s) noexcept {
+really_inline simdjson_result<element> parser::parse(const std::string &s) & noexcept {
   return parse(s.data(), s.length(), s.capacity() - s.length() < SIMDJSON_PADDING);
 }
-really_inline simdjson_result<element> parser::parse(const padded_string &s) noexcept {
+really_inline simdjson_result<element> parser::parse(const padded_string &s) & noexcept {
   return parse(s.data(), s.length(), false);
 }
 

tests/CMakeLists.txt

@@ -77,7 +77,7 @@ if(NOT (MSVC AND MSVC_VERSION LESS 1920))
   add_compile_test(readme_examples_noexceptions readme_examples_noexceptions.cpp quicktests)
 
   # Compile tests that *should fail*
-  add_compile_test(readme_examples_will_fail_with_exceptions_off readme_examples.cpp FALSE quicktests)
+  add_compile_test(readme_examples_will_fail_with_exceptions_off readme_examples.cpp)
   target_compile_definitions(readme_examples_will_fail_with_exceptions_off PRIVATE SIMDJSON_EXCEPTIONS=0)
   set_tests_properties(readme_examples_will_fail_with_exceptions_off PROPERTIES WILL_FAIL TRUE)
 
@@ -108,3 +108,5 @@ endif()
 # add_executable(singleheader ./singleheadertest.cpp ${PROJECT_SOURCE_DIR}/singleheader/simdjson.cpp)
 # target_link_libraries(singleheader simdjson)
 # add_test(singleheader singleheader)
+
+add_subdirectory(compilation_failure_tests)

tests/compilation_failure_tests/CMakeLists.txt

@@ -0,0 +1,38 @@
+#
+# This directory contains files aimed to verify that constructs that
+# are supposed to fail at compile time, indeed do so.
+# To prevent bit rot, the same source file is compiled twice with
+# the macro COMPILATION_TEST_USE_FAILING_CODE set to 0 or 1.
+#
+
+# internal function for add_dual_compile_test
+# which adds a target and a test target trying to compile it
+function(detail_add_dual_compile_test TEST_NAME TEST_FILE)
+  add_executable(${TEST_NAME} ${TEST_FILE})
+  set_target_properties(${TEST_NAME} PROPERTIES
+                        EXCLUDE_FROM_ALL TRUE
+                        EXCLUDE_FROM_DEFAULT_BUILD TRUE)
+  add_test(NAME ${TEST_NAME}
+           COMMAND ${CMAKE_COMMAND} --build . --target ${TEST_NAME} --config $<CONFIGURATION>
+           WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
+endfunction(add_compile_test)
+
+# adds a compilation test. two targets are created, one expected to
+# succeed compilation and one that is expected to fail.
+function(add_dual_compile_test TEST_NAME TEST_FILE)
+    detail_add_dual_compile_test(${TEST_NAME}_should_compile ${TEST_FILE})
+    detail_add_dual_compile_test(${TEST_NAME}_should_not_compile ${TEST_FILE})
+    target_compile_definitions(${TEST_NAME}_should_compile     PRIVATE COMPILATION_TEST_USE_FAILING_CODE=0)
+    target_compile_definitions(${TEST_NAME}_should_not_compile PRIVATE COMPILATION_TEST_USE_FAILING_CODE=1)
+    set_tests_properties(${TEST_NAME}_should_compile           PROPERTIES WILL_FAIL FALSE)
+    set_tests_properties(${TEST_NAME}_should_not_compile       PROPERTIES WILL_FAIL TRUE)
+endfunction(add_compile_test)
+
+
+add_dual_compile_test(example_compiletest               example_compiletest.cpp)
+add_dual_compile_test(dangling_parser_load              dangling_parser_load.cpp)
+add_dual_compile_test(dangling_parser_parse_uint8       dangling_parser_parse_uint8.cpp)
+add_dual_compile_test(dangling_parser_parse_uchar       dangling_parser_parse_uchar.cpp)
+add_dual_compile_test(dangling_parser_parse_stdstring   dangling_parser_parse_stdstring.cpp)
+add_dual_compile_test(dangling_parser_parse_padstring   dangling_parser_parse_padstring.cpp)
+

tests/compilation_failure_tests/dangling_parser_load.cpp

@@ -0,0 +1,15 @@
+
+// this tests https://github.com/simdjson/simdjson/issues/696
+
+#include <iostream>
+#include "simdjson.h"
+
+int main() {
+#if COMPILATION_TEST_USE_FAILING_CODE
+    simdjson::dom::element tree = simdjson::dom::parser().load("tree-pretty.json");
+#else
+    simdjson::dom::parser parser;
+    simdjson::dom::element tree = parser.load("tree-pretty.json");
+#endif
+    std::cout << tree["type"] << std::endl;
+}

tests/compilation_failure_tests/dangling_parser_parse_padstring.cpp

@@ -0,0 +1,16 @@
+
+// this tests https://github.com/simdjson/simdjson/issues/696
+
+#include <iostream>
+#include "simdjson.h"
+
+int main() {
+    simdjson::padded_string buf;
+#if COMPILATION_TEST_USE_FAILING_CODE
+    simdjson::dom::element tree = simdjson::dom::parser().parse(buf);
+#else
+    simdjson::dom::parser parser;
+    simdjson::dom::element tree = parser.parse(buf);
+#endif
+    std::cout << tree["type"] << std::endl;
+}

tests/compilation_failure_tests/dangling_parser_parse_stdstring.cpp

@@ -0,0 +1,16 @@
+
+// this tests https://github.com/simdjson/simdjson/issues/696
+
+#include <iostream>
+#include "simdjson.h"
+
+int main() {
+    std::string buf;
+#if COMPILATION_TEST_USE_FAILING_CODE
+    simdjson::dom::element tree = simdjson::dom::parser().parse(buf);
+#else
+    simdjson::dom::parser parser;
+    simdjson::dom::element tree = parser.parse(buf);
+#endif
+    std::cout << tree["type"] << std::endl;
+}

tests/compilation_failure_tests/dangling_parser_parse_uchar.cpp

@@ -0,0 +1,17 @@
+
+// this tests https://github.com/simdjson/simdjson/issues/696
+
+#include <iostream>
+#include "simdjson.h"
+
+int main() {
+    const char buf[128]={};
+    const size_t len=sizeof(buf);
+#if COMPILATION_TEST_USE_FAILING_CODE
+    simdjson::dom::element tree = simdjson::dom::parser().parse(buf,len);
+#else
+    simdjson::dom::parser parser;
+    simdjson::dom::element tree = parser.parse(buf,len);
+#endif
+    std::cout << tree["type"] << std::endl;
+}

tests/compilation_failure_tests/dangling_parser_parse_uint8.cpp

@@ -0,0 +1,17 @@
+
+// this tests https://github.com/simdjson/simdjson/issues/696
+
+#include <iostream>
+#include "simdjson.h"
+
+int main() {
+    const uint8_t buf[128]={};
+    const size_t len=sizeof(buf);
+#if COMPILATION_TEST_USE_FAILING_CODE
+    simdjson::dom::element tree = simdjson::dom::parser().parse(buf,len);
+#else
+    simdjson::dom::parser parser;
+    simdjson::dom::element tree = parser.parse(buf,len);
+#endif
+    std::cout << tree["type"] << std::endl;
+}

tests/compilation_failure_tests/example_compiletest.cpp

@@ -0,0 +1,20 @@
+//example for a compile test
+
+// keep as much code as possible outside of the conditional compilation
+// so the thing that is to proven to not compile fails to compile because
+// some unrelated thing (like forgetting to include a header)
+
+#include <vector>
+
+int main() {
+
+#if COMPILATION_TEST_USE_FAILING_CODE
+    // code that is supposed to fail compiling (keep it short!)
+    xxx
+#else
+   // equivalent code that is supposed to compile (keep it short!)
+#endif
+
+    // common code
+
+}