strict_not_null for unique_ptr (#1179)

- `strict_not_null<std::unique_ptr<int>>{ std::make_unique<int>()}` 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<unique_ptr<T>>` and for `strict_not_null<unique_ptr<T>>`
- 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 <w.henze@avm.de>

Author: beinhaerter

include/gsl/pointers

@@ -275,19 +275,19 @@ class strict_not_null : public not_null<T>
 {
 public:
     template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
-    constexpr explicit strict_not_null(U&& u) : not_null<T>(std::forward<U>(u))
+    constexpr explicit strict_not_null(U&& u) noexcept(std::is_nothrow_move_constructible<T>::value) : not_null<T>(std::forward<U>(u))
     {}
 
     template <typename = std::enable_if_t<!std::is_same<std::nullptr_t, T>::value>>
-    constexpr explicit strict_not_null(T u) : not_null<T>(u)
+    constexpr explicit strict_not_null(T u) noexcept(std::is_nothrow_move_constructible<T>::value) : not_null<T>(std::move(u))
     {}
 
     template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
-    constexpr strict_not_null(const not_null<U>& other) : not_null<T>(other)
+    constexpr strict_not_null(const not_null<U>& other) noexcept(std::is_nothrow_move_constructible<T>::value) : not_null<T>(other)
     {}
 
     template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
-    constexpr strict_not_null(const strict_not_null<U>& other) : not_null<T>(other)
+    constexpr strict_not_null(const strict_not_null<U>& other) noexcept(std::is_nothrow_move_constructible<T>::value) : not_null<T>(other)
     {}
 
     // To avoid invalidating the "not null" invariant, the contained pointer is actually copied

tests/notnull_tests.cpp

@@ -178,6 +178,14 @@ TEST(notnull_tests, TestNotNullConstructors)
         EXPECT_DEATH((not_null<decltype(pi)>(pi)), expected);
     }
 
+    {
+        // from unique pointer
+        not_null<std::unique_ptr<int>> x(
+            std::make_unique<int>(10)); // unique_ptr<int> is nullptr assignable
+
+        EXPECT_DEATH((not_null<std::unique_ptr<int>>(std::unique_ptr<int>{})), expected);
+    }
+
     {
         // from pointer to local
         int t = 42;

tests/pointers_tests.cpp

@@ -25,22 +25,50 @@ struct NotMoveAssignableCustomPtr
 TEST(pointers_test, swap)
 {
     // taken from gh-1129:
-    gsl::not_null<std::unique_ptr<int>> a(std::make_unique<int>(0));
-    gsl::not_null<std::unique_ptr<int>> b(std::make_unique<int>(1));
+    {
+        gsl::not_null<std::unique_ptr<int>> a(std::make_unique<int>(0));
+        gsl::not_null<std::unique_ptr<int>> b(std::make_unique<int>(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<NotMoveAssignableCustomPtr>()};
-    gsl::not_null<NotMoveAssignableCustomPtr> 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<NotMoveAssignableCustomPtr>()};
+        gsl::not_null<NotMoveAssignableCustomPtr> c{*shared_custom_ptr};
+        EXPECT_TRUE(c.get() != nullptr);
+    }
+
+    {
+        gsl::strict_not_null<std::unique_ptr<int>> a{std::make_unique<int>(0)};
+        gsl::strict_not_null<std::unique_ptr<int>> b{std::make_unique<int>(1)};
+
+        EXPECT_TRUE(*a == 0);
+        EXPECT_TRUE(*b == 1);
+
+        gsl::swap(a, b);
+
+        EXPECT_TRUE(*a == 1);
+        EXPECT_TRUE(*b == 0);
+    }
+
+    {
+        gsl::not_null<std::unique_ptr<int>> a{std::make_unique<int>(0)};
+        gsl::strict_not_null<std::unique_ptr<int>> b{std::make_unique<int>(1)};
+
+        EXPECT_TRUE(*a == 0);
+        EXPECT_TRUE(*b == 1);
+
+        gsl::swap(a, b);
+
+        EXPECT_TRUE(*a == 1);
+        EXPECT_TRUE(*b == 0);
+    }
 }
 
 #if __cplusplus >= 201703l

tests/strict_notnull_tests.cpp

@@ -21,6 +21,15 @@
 
 using namespace gsl;
 
+// stand-in for a user-defined ref-counted class
+template <typename T>
+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<const int*> 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<int*> p = nullptr;          // yay...does not compile!
+        strict_not_null<std::vector<char>*> p1 = 0; // yay...does not compile!
+        strict_not_null<int*> p2;                   // yay...does not compile!
+        std::unique_ptr<int> up = std::make_unique<int>(120);
+        strict_not_null<int*> p3 = up;
+
+        // Forbid non-nullptr assignable types
+        strict_not_null<std::vector<int>> f(std::vector<int>{1});
+        strict_not_null<int> z(10);
+        strict_not_null<std::vector<int>> 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<int>(&i);
+        strict_not_null<int*> p(rp);
+        EXPECT_TRUE(p.get() == &i);
+
+        strict_not_null<std::shared_ptr<int>> x(
+            std::make_shared<int>(10)); // shared_ptr<int> is nullptr assignable
+
+        int* pi = nullptr;
+        EXPECT_DEATH((strict_not_null<decltype(pi)>(pi)), expected);
+    }
+
+    {
+        // from unique pointer
+        strict_not_null<std::unique_ptr<int>> x(
+            std::make_unique<int>(10)); // unique_ptr<int> is nullptr assignable
+
+        EXPECT_DEATH((strict_not_null<std::unique_ptr<int>>(std::unique_ptr<int>{})), expected);
+    }
+
+    {
+        // from pointer to local
+        int t = 42;
+
+        strict_not_null<int*> 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<int*> 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<const int*> 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<const int*>{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)
 {
     {