C26431 DONT_TEST_NOTNULL

"The type of expression is already gsl::not_null. Do not test it for nullness."

C++ Core Guidelines: F.23: Use a not_null<T> to indicate that "null" is not a valid value

The marker type gsl::not_null from Guidelines Support Library is used to clearly indicate values that are never null pointers. It causes a hard failure if such assumption is not held at run time. So, obviously, there is no need to check for nullness if expression evaluates to a result of type gsl::not_null.

Remarks

  • Since gsl::not_null itself is a thin pointer wrapper class, the rule actually tracks temporary variables that hold results from calls to the overloaded conversion operator (which returns contained pointer object). Such logic makes this rule applicable to expressions that involve variables and eventually have result of the gsl::not_null type. But it currently skips expressions that contain function calls returning gsl::not_null.
    • Current heuristic for nullness checks detects the following contexts:
    • symbol expression in a branch condition, for example "if (p) { ... }";
    • non-bitwise logical operations;
    • comparison operations where one operand is a constant expression that evaluates to zero.

Example

unnecessary null checks reveal questionable logic

class type {
public:
    template<class T> bool is() const;
    template<class T> gsl::not_null<const T*> as() const;
    //...
};

class alias_type : public type {
public:
    gsl::not_null<const type*> get_underlying_type() const;
    gsl::not_null<const type*> get_root_type() const
    {
        const auto ut = get_underlying_type();
        if (ut)                                     // C26431
        {
            const auto uat = ut->as<alias_type>();
            if (uat)                                // C26431, also incorrect use of API!
                return uat->get_root_type();

            return ut;
        }

        return this;                                // Alias to nothing? Actually, dead code!
    }
    //...
};

unnecessary null checks reveal questionable logic - reworked

    //...
    gsl::not_null<const type*> get_root_type() const
    {
        const auto ut = get_underlying_type();
        if (ut->is<alias_type>())
            return ut->as<alias_type>()->get_root_type();

        return ut;
    }
    //...