From 5cbc1ddf4b38354e64717cba9c0c7d03acec22f9 Mon Sep 17 00:00:00 2001 From: vector-of-bool Date: Tue, 26 Sep 2023 13:38:24 -0600 Subject: [PATCH 1/4] Bring our own make_unique --- .../v_noabi/bsoncxx/stdx/make_unique.hpp | 146 +++++++++++++----- src/bsoncxx/test/CMakeLists.txt | 2 +- src/bsoncxx/test/make_unique.test.cpp | 43 ++++++ 3 files changed, 153 insertions(+), 38 deletions(-) create mode 100644 src/bsoncxx/test/make_unique.test.cpp diff --git a/src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/make_unique.hpp b/src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/make_unique.hpp index 7bc90d50c4..a5b38b0f55 100644 --- a/src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/make_unique.hpp +++ b/src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/make_unique.hpp @@ -14,52 +14,124 @@ #pragma once -#include - -#if defined(BSONCXX_POLY_USE_MNMLSTC) - -#include - -namespace bsoncxx { -BSONCXX_INLINE_NAMESPACE_BEGIN -namespace stdx { - -using ::core::make_unique; - -} // namespace stdx -BSONCXX_INLINE_NAMESPACE_END -} // namespace bsoncxx - -#elif defined(BSONCXX_POLY_USE_BOOST) - -#include - -namespace bsoncxx { -BSONCXX_INLINE_NAMESPACE_BEGIN -namespace stdx { - -using ::boost::make_unique; - -} // namespace stdx -BSONCXX_INLINE_NAMESPACE_END -} // namespace bsoncxx - -#elif __cplusplus >= 201402L || (defined(_MSVC_LANG) && _MSVC_LANG >= 201402L) - +#include #include +#include +#include + +#include namespace bsoncxx { BSONCXX_INLINE_NAMESPACE_BEGIN namespace stdx { -using ::std::make_unique; +namespace detail { + +// Switch backend of make_unique by the type we are creating. +// It would be easier to 'if constexpr' on whether we are an array and whether to direct-init or +// value-init, but we don't have if-constexpr and we need it to guard against an uterance of a +// possibly-illegal 'new' expression. +template +struct make_unique_impl { + // For make_unique: + template ()...))> + static std::unique_ptr make(std::true_type /* direct-init */, Args&&... args) { + return std::unique_ptr(new T(std::forward(args)...)); + } + + // For make_unique_for_overwrite: + template + static std::unique_ptr make(std::false_type /* value-init */) { + return std::unique_ptr(new T); + } +}; + +// For unbounded arrays: +template +struct make_unique_impl { + template ()])> + static std::unique_ptr make(ShouldDirectInit, std::size_t count) { + // These can share a function via a plain if, because both new expressions + // must be semantically valid + if (ShouldDirectInit()) { + return std::unique_ptr(new Elem[count]()); + } else { + return std::unique_ptr(new Elem[count]); + } + } +}; + +// Bounded arrays are disallowed: +template +struct make_unique_impl {}; + +// References are nonsense: +template +struct make_unique_impl {}; + +// References are nonsense: +template +struct make_unique_impl {}; + +} // namespace detail + +/** + * @brief Create a new std::unique_ptr that points-to the given type, direct-initialized based on + * the given arguments. + * + * @tparam T Any non-array object type or any array of unknown bound. + * @param args If T is a non-array object type, these are the constructor arguments used to + * direct-initialize the object. If T is an array of unknown bound, then the sole argument must be a + * single std::size_t that specifies the number of objects to allocate in the array. + * + * Requires: + * - If T is an array of unknown bounds, then args... must be a single size_t and the element type + * of T must be value-initializable. + * - Otherwise, if T is a non-array object type, then T must be direct-initializable with arguments + * `args...` + * - Otherwise, this function is excluded from overload resolution. + */ +template , + typename = decltype(Impl::make(std::true_type{}, std::declval()...))> +std::unique_ptr make_unique(Args&&... args) { + return Impl::make(std::true_type{}, std::forward(args)...); +} + +/** + * @brief Create a new std::unique_ptr that points-to a default-initialized instance of the given + * type. + * + * @tparam T A non-array object type or an array of unknown bound + * @param args If T is an object type, then args... must no arguments are allowed. + * If T is an array of unknown bound, then args... must be a single size_t specifying + * the length of the array to allocate. + * + * Requires: + * - T must be value-initializable + * - If T is an array of unknown bounds, then args... must be a single size_t + * - Otherwise, if T is a non-array object type, args... must be empty + * - Otherwise, this function is excluded from overload resolution + */ +template , + typename = decltype(Impl::make(std::false_type{}, std::declval()...))> +std::unique_ptr make_unique_for_overwrite(Args&&... args) { + return Impl::make(std::false_type{}, std::forward(args)...); +} } // namespace stdx BSONCXX_INLINE_NAMESPACE_END } // namespace bsoncxx -#else -#error "Cannot find a valid polyfill for make_unique" -#endif - #include diff --git a/src/bsoncxx/test/CMakeLists.txt b/src/bsoncxx/test/CMakeLists.txt index 35bc52b3b8..649bd00557 100644 --- a/src/bsoncxx/test/CMakeLists.txt +++ b/src/bsoncxx/test/CMakeLists.txt @@ -19,7 +19,7 @@ endif() # Allow `#include ` include_directories(${THIRD_PARTY_SOURCE_DIR}/..) -file (GLOB src_bsoncxx_test_DIST_cpps RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} *.cpp) +file (GLOB src_bsoncxx_test_DIST_cpps CONFIGURE_DEPENDS RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} *.cpp) add_executable(test_bson ${THIRD_PARTY_SOURCE_DIR}/catch/main.cpp diff --git a/src/bsoncxx/test/make_unique.test.cpp b/src/bsoncxx/test/make_unique.test.cpp new file mode 100644 index 0000000000..254dbc7749 --- /dev/null +++ b/src/bsoncxx/test/make_unique.test.cpp @@ -0,0 +1,43 @@ +#include +#include +#include + +#include +#include + +namespace { + +struct something { + something(int val) : value(val) {} + int value; +}; + +TEST_CASE("Create a unique_ptr") { + auto ptr = bsoncxx::stdx::make_unique(12); + CHECKED_IF(ptr) { + CHECK(*ptr == 12); + } + + auto thing = bsoncxx::stdx::make_unique(5); + CHECKED_IF(thing) { + CHECK(thing->value == 5); + } +} + +TEST_CASE("Create a unique_ptr") { + const unsigned length = 12; + auto ptr = bsoncxx::stdx::make_unique(length); + CHECKED_IF(ptr) { + // All elements are direct-initialized, which produces '0' for `int` + CHECK(ptr[0] == 0); + auto res = std::equal_range(ptr.get(), ptr.get() + length, 0); + CHECK(res.first == ptr.get()); + CHECK(res.second == (ptr.get() + length)); + } + + ptr = bsoncxx::stdx::make_unique_for_overwrite(length); + std::fill_n(ptr.get(), length, 42); + CHECK(std::all_of(ptr.get(), ptr.get() + length, [](int n) { return n == 42; })); +} + +} // namespace From aa8b45accdab5be24b8712039b4414ebdc88a804 Mon Sep 17 00:00:00 2001 From: vector-of-bool Date: Wed, 11 Oct 2023 12:06:46 -0600 Subject: [PATCH 2/4] Minor CMake cleanup --- src/bsoncxx/CMakeLists.txt | 1 + src/bsoncxx/test/CMakeLists.txt | 2 +- src/mongocxx/CMakeLists.txt | 3 ++- src/mongocxx/test/CMakeLists.txt | 33 ++++++++++++++++++-------------- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/bsoncxx/CMakeLists.txt b/src/bsoncxx/CMakeLists.txt index 1e39d51960..3b8e2b0c38 100644 --- a/src/bsoncxx/CMakeLists.txt +++ b/src/bsoncxx/CMakeLists.txt @@ -188,6 +188,7 @@ else() endif() if(ENABLE_TESTS) target_compile_definitions(bsoncxx_testing PUBLIC BSONCXX_TESTING) + add_library(bsoncxx::testing ALIAS bsoncxx_testing) endif() set (libdir "\${prefix}/${CMAKE_INSTALL_LIBDIR}") diff --git a/src/bsoncxx/test/CMakeLists.txt b/src/bsoncxx/test/CMakeLists.txt index 649bd00557..14f9da6410 100644 --- a/src/bsoncxx/test/CMakeLists.txt +++ b/src/bsoncxx/test/CMakeLists.txt @@ -26,7 +26,7 @@ add_executable(test_bson ${src_bsoncxx_test_DIST_cpps} ) -target_link_libraries(test_bson bsoncxx_testing ${libbson_target}) +target_link_libraries(test_bson PRIVATE bsoncxx::testing ${libbson_target}) target_include_directories(test_bson PRIVATE ${libbson_include_directories}) target_compile_definitions(test_bson PRIVATE ${libbson_definitions}) diff --git a/src/mongocxx/CMakeLists.txt b/src/mongocxx/CMakeLists.txt index ecd52ed45d..a5cf93d2e8 100644 --- a/src/mongocxx/CMakeLists.txt +++ b/src/mongocxx/CMakeLists.txt @@ -196,8 +196,9 @@ else() endif() endif() if(ENABLE_TESTS) - target_link_libraries(mongocxx_mocked PUBLIC bsoncxx_testing) + target_link_libraries(mongocxx_mocked PUBLIC bsoncxx::testing) target_compile_definitions(mongocxx_mocked PUBLIC MONGOCXX_TESTING) + add_library(mongocxx::mocked ALIAS mongocxx_mocked) if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") target_compile_options(mongocxx_mocked PRIVATE /bigobj) endif() diff --git a/src/mongocxx/test/CMakeLists.txt b/src/mongocxx/test/CMakeLists.txt index 1a70f56eb9..231e5a8b5a 100644 --- a/src/mongocxx/test/CMakeLists.txt +++ b/src/mongocxx/test/CMakeLists.txt @@ -103,7 +103,7 @@ add_executable(test_driver set(THREADS_PREFER_PTHREAD_FLAG ON) find_package(Threads REQUIRED) -target_link_libraries(test_driver Threads::Threads) +target_link_libraries(test_driver PRIVATE Threads::Threads) add_executable(test_logging ${THIRD_PARTY_SOURCE_DIR}/catch/main.cpp @@ -186,19 +186,24 @@ add_executable(test_versioned_api ${spec_test_common} ) -target_link_libraries(test_driver mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_logging mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_instance mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_client_side_encryption_specs mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_crud_specs mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_gridfs_specs mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_command_monitoring_specs mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_transactions_specs mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_retryable_reads_specs mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_read_write_concern_specs mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_mongohouse_specs mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_unified_format_spec mongocxx_mocked ${libmongoc_target}) -target_link_libraries(test_versioned_api mongocxx_mocked ${libmongoc_target}) +set_property( + TARGET + test_driver + test_logging + test_instance + test_client_side_encryption_specs + test_crud_specs + test_gridfs_specs + test_command_monitoring_specs + test_transactions_specs + test_retryable_reads_specs + test_read_write_concern_specs + test_mongohouse_specs + test_unified_format_spec + test_versioned_api + APPEND PROPERTY LINK_LIBRARIES + mongocxx::mocked ${libmongoc_target} +) target_include_directories(test_driver PRIVATE ${libmongoc_include_directories}) target_include_directories(test_logging PRIVATE ${libmongoc_include_directories}) From 165a531ab337bbe75728a8b10b3d23b58c824cda Mon Sep 17 00:00:00 2001 From: vector-of-bool Date: Wed, 18 Oct 2023 14:34:22 -0600 Subject: [PATCH 3/4] No checked_if --- src/bsoncxx/test/make_unique.test.cpp | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/bsoncxx/test/make_unique.test.cpp b/src/bsoncxx/test/make_unique.test.cpp index 254dbc7749..15cd58ec6a 100644 --- a/src/bsoncxx/test/make_unique.test.cpp +++ b/src/bsoncxx/test/make_unique.test.cpp @@ -14,26 +14,23 @@ struct something { TEST_CASE("Create a unique_ptr") { auto ptr = bsoncxx::stdx::make_unique(12); - CHECKED_IF(ptr) { - CHECK(*ptr == 12); - } + REQUIRE(ptr); + CHECK(*ptr == 12); auto thing = bsoncxx::stdx::make_unique(5); - CHECKED_IF(thing) { - CHECK(thing->value == 5); - } + REQUIRE(thing); + CHECK(thing->value == 5); } TEST_CASE("Create a unique_ptr") { const unsigned length = 12; auto ptr = bsoncxx::stdx::make_unique(length); - CHECKED_IF(ptr) { - // All elements are direct-initialized, which produces '0' for `int` - CHECK(ptr[0] == 0); - auto res = std::equal_range(ptr.get(), ptr.get() + length, 0); - CHECK(res.first == ptr.get()); - CHECK(res.second == (ptr.get() + length)); - } + REQUIRE(ptr); + // All elements are direct-initialized, which produces '0' for `int` + CHECK(ptr[0] == 0); + auto res = std::equal_range(ptr.get(), ptr.get() + length, 0); + CHECK(res.first == ptr.get()); + CHECK(res.second == (ptr.get() + length)); ptr = bsoncxx::stdx::make_unique_for_overwrite(length); std::fill_n(ptr.get(), length, 42); From f4677e441c77eb151017d6245981ba458cb1d97e Mon Sep 17 00:00:00 2001 From: vector-of-bool Date: Wed, 18 Oct 2023 16:09:26 -0600 Subject: [PATCH 4/4] Wrong comment --- .../include/bsoncxx/v_noabi/bsoncxx/stdx/make_unique.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/make_unique.hpp b/src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/make_unique.hpp index a5b38b0f55..5f07376cb1 100644 --- a/src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/make_unique.hpp +++ b/src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/make_unique.hpp @@ -117,7 +117,7 @@ std::unique_ptr make_unique(Args&&... args) { * the length of the array to allocate. * * Requires: - * - T must be value-initializable + * - T must be default-initializable * - If T is an array of unknown bounds, then args... must be a single size_t * - Otherwise, if T is a non-array object type, args... must be empty * - Otherwise, this function is excluded from overload resolution