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

1

6

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)
Nightwalker answered 3/7, 2018 at 15:12 Comment(6)
I seen to remember in the discussions something about that check should only be in the std::unique_ptr specialization to make sure the not null std::unique_ptr isn't dereferenced after being moved from. I still don't like it though tbh.Complect
@FrançoisAndrieux, Please see update about the implementation of Ensures. Actually, it depends on flags, I attached the default one. As far as I understand, it actually costs something in run-timeNightwalker
Allowing moving not_null(not_null&& other) = default; is the reason. Maybe it should be non-moveable "not_null" - or some more sophisticated solution - because, as it is implemented right now - I doubt I am going to use itChoreography
@PiotrNycz, Actually, as far as I see in the standard, the moved from objects are left in unspecified but valid state. The experiment shows the pointer values do not change when moved from.Nightwalker
This probably also allows the compiler to optimize out null checks being performed later as it can establish that the program would have terminated if it was null.Eldreeda
See the discussion in github.com/Microsoft/GSL/pull/449.Kohn
P
5

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.

Polyptych answered 3/7, 2018 at 21:37 Comment(1)
How about std::shared_ptr? I thought the spec required that std::move() nullify the original, too?Pullman

© 2022 - 2024 — McMap. All rights reserved.