From 16a60199df91a1c04f081222d798aab9a067d126 Mon Sep 17 00:00:00 2001 From: Werner Henze <34543625+beinhaerter@users.noreply.github.com> Date: Thu, 26 Dec 2024 18:00:36 +0100 Subject: [PATCH] strict_not_null for unique_ptr (#1179) - `strict_not_null>{ std::make_unique()}` failed to compile - `strict_not_null` ctor needs to move the passed `unique_ptr`, not copy - Copied `not_null` `TestNotNullConstructors` for `strict_not_null` - The `noexcept` specifiers on the `strict_not_null` and `not_null` constructors were out of sync. - Added unit test for `not_null>` and for `strict_not_null>` - Added unit test for `gsl::swap` for two `strict_not_null` - Added unit test for `gsl::swap` for `not_null` and `strict_not_null` Co-authored-by: Werner Henze --- include/gsl/pointers | 8 +-- tests/notnull_tests.cpp | 8 +++ tests/pointers_tests.cpp | 52 +++++++++++---- tests/strict_notnull_tests.cpp | 115 ++++++++++++++++++++++++++++++++- 4 files changed, 166 insertions(+), 17 deletions(-) diff --git a/include/gsl/pointers b/include/gsl/pointers index 78d191c..b93ca86 100644 --- a/include/gsl/pointers +++ b/include/gsl/pointers @@ -275,19 +275,19 @@ class strict_not_null : public not_null { public: template ::value>> - constexpr explicit strict_not_null(U&& u) : not_null(std::forward(u)) + constexpr explicit strict_not_null(U&& u) noexcept(std::is_nothrow_move_constructible::value) : not_null(std::forward(u)) {} template ::value>> - constexpr explicit strict_not_null(T u) : not_null(u) + constexpr explicit strict_not_null(T u) noexcept(std::is_nothrow_move_constructible::value) : not_null(std::move(u)) {} template ::value>> - constexpr strict_not_null(const not_null& other) : not_null(other) + constexpr strict_not_null(const not_null& other) noexcept(std::is_nothrow_move_constructible::value) : not_null(other) {} template ::value>> - constexpr strict_not_null(const strict_not_null& other) : not_null(other) + constexpr strict_not_null(const strict_not_null& other) noexcept(std::is_nothrow_move_constructible::value) : not_null(other) {} // To avoid invalidating the "not null" invariant, the contained pointer is actually copied diff --git a/tests/notnull_tests.cpp b/tests/notnull_tests.cpp index df8e5a9..b6f8b37 100644 --- a/tests/notnull_tests.cpp +++ b/tests/notnull_tests.cpp @@ -178,6 +178,14 @@ TEST(notnull_tests, TestNotNullConstructors) EXPECT_DEATH((not_null(pi)), expected); } + { + // from unique pointer + not_null> x( + std::make_unique(10)); // unique_ptr is nullptr assignable + + EXPECT_DEATH((not_null>(std::unique_ptr{})), expected); + } + { // from pointer to local int t = 42; diff --git a/tests/pointers_tests.cpp b/tests/pointers_tests.cpp index 673e184..7757c27 100644 --- a/tests/pointers_tests.cpp +++ b/tests/pointers_tests.cpp @@ -25,22 +25,50 @@ struct NotMoveAssignableCustomPtr TEST(pointers_test, swap) { // taken from gh-1129: - gsl::not_null> a(std::make_unique(0)); - gsl::not_null> b(std::make_unique(1)); + { + gsl::not_null> a(std::make_unique(0)); + gsl::not_null> b(std::make_unique(1)); - EXPECT_TRUE(*a == 0); - EXPECT_TRUE(*b == 1); + EXPECT_TRUE(*a == 0); + EXPECT_TRUE(*b == 1); - gsl::swap(a, b); + gsl::swap(a, b); - EXPECT_TRUE(*a == 1); - EXPECT_TRUE(*b == 0); + EXPECT_TRUE(*a == 1); + EXPECT_TRUE(*b == 0); - // Make sure our custom ptr can be used with not_null. The shared_pr is to prevent "unused" - // compiler warnings. - const auto shared_custom_ptr{std::make_shared()}; - gsl::not_null c{*shared_custom_ptr}; - EXPECT_TRUE(c.get() != nullptr); + // Make sure our custom ptr can be used with not_null. The shared_pr is to prevent "unused" + // compiler warnings. + const auto shared_custom_ptr{std::make_shared()}; + gsl::not_null c{*shared_custom_ptr}; + EXPECT_TRUE(c.get() != nullptr); + } + + { + gsl::strict_not_null> a{std::make_unique(0)}; + gsl::strict_not_null> b{std::make_unique(1)}; + + EXPECT_TRUE(*a == 0); + EXPECT_TRUE(*b == 1); + + gsl::swap(a, b); + + EXPECT_TRUE(*a == 1); + EXPECT_TRUE(*b == 0); + } + + { + gsl::not_null> a{std::make_unique(0)}; + gsl::strict_not_null> b{std::make_unique(1)}; + + EXPECT_TRUE(*a == 0); + EXPECT_TRUE(*b == 1); + + gsl::swap(a, b); + + EXPECT_TRUE(*a == 1); + EXPECT_TRUE(*b == 0); + } } #if __cplusplus >= 201703l diff --git a/tests/strict_notnull_tests.cpp b/tests/strict_notnull_tests.cpp index c1b1f54..82f9c5c 100644 --- a/tests/strict_notnull_tests.cpp +++ b/tests/strict_notnull_tests.cpp @@ -21,6 +21,15 @@ using namespace gsl; +// stand-in for a user-defined ref-counted class +template +struct RefCounted +{ + RefCounted(T* p) : p_(p) {} + operator T*() { return p_; } + T* p_; +}; + namespace { // clang-format off @@ -43,12 +52,116 @@ GSL_SUPPRESS(f.4) // NO-FORMAT: attribute // clang-format on bool strict_helper_const(strict_not_null p) { return *p == 12; } -#ifdef CONFIRM_COMPILATION_ERRORS int* return_pointer() { return nullptr; } +#ifdef CONFIRM_COMPILATION_ERRORS const int* return_pointer_const() { return nullptr; } #endif } // namespace +TEST(strict_notnull_tests, TestStrictNotNullConstructors) +{ + { +#ifdef CONFIRM_COMPILATION_ERRORS + strict_not_null p = nullptr; // yay...does not compile! + strict_not_null*> p1 = 0; // yay...does not compile! + strict_not_null p2; // yay...does not compile! + std::unique_ptr up = std::make_unique(120); + strict_not_null p3 = up; + + // Forbid non-nullptr assignable types + strict_not_null> f(std::vector{1}); + strict_not_null z(10); + strict_not_null> y({1, 2}); +#endif + } + + const auto terminateHandler = std::set_terminate([] { + std::cerr << "Expected Death. TestNotNullConstructors"; + std::abort(); + }); + const auto expected = GetExpectedDeathString(terminateHandler); + + { + // from shared pointer + int i = 12; + auto rp = RefCounted(&i); + strict_not_null p(rp); + EXPECT_TRUE(p.get() == &i); + + strict_not_null> x( + std::make_shared(10)); // shared_ptr is nullptr assignable + + int* pi = nullptr; + EXPECT_DEATH((strict_not_null(pi)), expected); + } + + { + // from unique pointer + strict_not_null> x( + std::make_unique(10)); // unique_ptr is nullptr assignable + + EXPECT_DEATH((strict_not_null>(std::unique_ptr{})), expected); + } + + { + // from pointer to local + int t = 42; + + strict_not_null x{&t}; + helper(&t); + helper_const(&t); + + EXPECT_TRUE(*x == 42); + } + + { + // from raw pointer + // from strict_not_null pointer + + int t = 42; + int* p = &t; + + strict_not_null x{p}; + helper(p); + helper_const(p); + helper(x); + helper_const(x); + + EXPECT_TRUE(*x == 42); + } + + { + // from raw const pointer + // from strict_not_null const pointer + + int t = 42; + const int* cp = &t; + + strict_not_null x{cp}; + helper_const(cp); + helper_const(x); + + EXPECT_TRUE(*x == 42); + } + + { + // from strict_not_null const pointer, using auto + int t = 42; + const int* cp = &t; + + auto x = strict_not_null{cp}; + + EXPECT_TRUE(*x == 42); + } + + { + // from returned pointer + + EXPECT_DEATH(helper(return_pointer()), expected); + EXPECT_DEATH(helper_const(return_pointer()), expected); + } +} + TEST(strict_notnull_tests, TestStrictNotNull) { {