Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove null check during access and allow non-copyable pointer types like unique_ptr #650

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions include/gsl/pointers
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,9 @@ using owner = T;
template <class T>
class not_null
{
public:
static_assert(std::is_assignable<T&, std::nullptr_t>::value, "T cannot be assigned nullptr.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cppcoreguidelines says in http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#gslview-views the following about not_null<T : "T can be any type for which ==nullptr is meaningful.", so I guess it would make sense to static_assert on this kind of type trait instead : https://ideone.com/nuYfaB


template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
public:
template <typename U, typename = typename std::enable_if<std::is_constructible<T, U>::value>::type>
constexpr not_null(U&& u) : ptr_(std::forward<U>(u))
{
Expects(ptr_ != nullptr);
Expand All @@ -86,14 +85,20 @@ public:
not_null(const not_null& other) = default;
not_null& operator=(const not_null& other) = default;

constexpr T get() const

constexpr T const & get() const
{
Ensures(ptr_ != nullptr);
/*
not_null constructors and assignment operators always verify ptr_ is
not set to null. As long as references to ptr_ are not exposed publicly
it is not additionally necessary to check the value against nullptr here.
*/
// Ensures(ptr_ != nullptr);
return ptr_;
}

constexpr operator T() const { return get(); }
constexpr T operator->() const { return get(); }
constexpr operator T const & () const { return get(); }
constexpr T const & operator->() const { return get(); }
constexpr decltype(auto) operator*() const { return *get(); }

// prevents compilation when someone attempts to assign a null pointer constant
Expand Down
54 changes: 54 additions & 0 deletions tests/notnull_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <stdint.h> // for uint16_t
#include <string> // for basic_string, operator==, string, operator<<
#include <typeinfo> // for type_info
#include <functional>

namespace gsl {
struct fail_fast;
Expand Down Expand Up @@ -56,6 +57,7 @@ struct CustomPtr
{
CustomPtr(T* p) : p_(p) {}
operator T*() { return p_; }
operator T*() const { return p_; }
bool operator!=(std::nullptr_t) const { return p_ != nullptr; }
T* p_ = nullptr;
};
Expand Down Expand Up @@ -329,3 +331,55 @@ TEST_CASE("TestNotNullCustomPtrComparison")
}

static_assert(std::is_nothrow_move_constructible<not_null<void *>>::value, "not_null must be no-throw move constructible");

struct UniquePointerTestStruct {
int i = 0;
};

TEST_CASE("TestNotNullUniquePtrComparison") {

{
using NotNull1 = not_null<std::unique_ptr<int>>;

// values are the same
CHECK((*NotNull1(std::make_unique<int>(42)) == *NotNull1(std::make_unique<int>(42))));
}
{
using NotNull1 = not_null<std::unique_ptr<UniquePointerTestStruct>>;

// values are the same
CHECK((NotNull1(std::make_unique<UniquePointerTestStruct>())->i == NotNull1(std::make_unique<UniquePointerTestStruct>())->i));
}
}


template<typename T>
struct UncopyableUnmovablePointerLikeType {
private:
T * t;

public:
UncopyableUnmovablePointerLikeType(T * pointer) : t(pointer) {}
UncopyableUnmovablePointerLikeType(const UncopyableUnmovablePointerLikeType&) = delete;
UncopyableUnmovablePointerLikeType(UncopyableUnmovablePointerLikeType &&) = delete;

operator T*() { return t; }
operator T*() const { return t; }
};


// this test case makes sure not_null works on move-only types like std::unique_ptr as well as verifying
// that no copy penalty is being paid for types with expensive copy constructors like std::shared_ptr
TEST_CASE("ConfirmCopyableAndMoveablePointerTypeNotRequired") {
int i = 5;
gsl::not_null<UncopyableUnmovablePointerLikeType<int>> fixed_in_place(&i);
CHECK(*fixed_in_place == 5);
}


TEST_CASE("std::function") {
gsl::not_null<std::function<int()>> nnsf([](){return 4;});
CHECK(nnsf.get()() == 4);
std::function<int()> empty_function;
CHECK_THROWS(nnsf = empty_function);
}