Warning C26431

The type of expression 'expr' is already gsl::not_null. Do not test it for nullness (f.23)

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

The marker type gsl::not_null from the Guidelines Support Library is used to clearly indicate values that are never null pointers. It causes a hard failure if the assumption doesn't hold at run time. So, obviously, there's no need to check for null if an 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 objects). Such logic makes this rule applicable to expressions that involve variables and eventually have a result of the gsl::not_null type. However, it currently skips expressions that contain function calls returning gsl::not_null.

The current heuristic for null checks detects the following contexts:

• a 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.

Code analysis name: DONT_TEST_NOTNULL

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;
    }
    //...