c++nullcpp-core-guidelinesguideline-support-library

Why gsl::not_null ensures ptr is not null on get()?


In the Microsoft implementation of guidelines support library I see the following piece of code:

template<class T>
class not_null {
    ...
    template <typename U, typename = std::enable_if_t<std::is_convertible<U, T>::value>>
    constexpr explicit not_null(U&& u) : ptr_(std::forward<U>(u)) {
        Expects(ptr_ != nullptr);
    }
    ...
    constexpr T get() const {
        Ensures(ptr_);
        return ptr_;
    }
    ...
    T ptr_;
}

All the constructors of gsl::not_null which take possibly pointers check these pointers are not null, but we still check stored value of pointer (ptr_) against null on each dereference. Why do we have this check, given that in C++ we typically don't pay for what we don't need?

UP: Ensures is implemented as follows (with default flags):

#define GSL_LIKELY(x) (!!(x))
...
#define GSL_CONTRACT_CHECK(type, cond)                         \
(GSL_LIKELY(cond) ? static_cast<void>(0) : gsl::details::terminate())
...
#define Ensures(cond) GSL_CONTRACT_CHECK("Postcondition", cond)

Solution

  • The comments are already giving the idea why removing null check from not_null::get() is not desirable. The main problem is that the change allows dereferencing a smart pointer after move.

    For examples, see the following discussion on a PR that enables usage of not_null<unique_ptr> and how the change is incompatible with removing the null check from not_null::get()

    https://github.com/Microsoft/GSL/pull/675

    As for performance concerns, compiler optimizer should be able to remove many of the null checks, but not all, of course. If some checks are not removed but seem to be removable, we should fix compiler optimizations.