[clang-tidy] bugprone-unchecked-optional-access false positive

[Skylion007]

    template <typename T>
    static handle cast(T &&src, return_value_policy policy, handle parent) {
        if (!src) {
            return none().release();
        }
        if (!std::is_lvalue_reference<T>::value) {
            policy = return_value_policy_override<Value>::policy(policy);
        }
        // This NOLINT should not be required. It can't get here because of an early return
        // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
        return value_conv::cast(*std::forward<T>(src), policy, parent);

Clang-Tidy flags this behavior is bugprone when it check has_value through the boolean cast overload if(!src) and then proceeds with the optional access only in the if else branch the avoids the early return. We encountered this issue when upgrading clang-tidy here: pybind/pybind11#4387. Obviously, this is an easy fix for a nolint, but it would better if the check recognized early returns as valid checks for bugprone-unchecked-optional-access. I suspect this is actually caused by clang-tidy not recognizing the operator! in the flow as a valid check on the optional access. It could also have something to with the fact that std::optional is merely a template parameter in this caster.

[llvmbot]

@llvm/issue-subscribers-clang-tidy

[AMS21]

I seems to rather have something todo with the call to std::forward since this code also triggers the false positive: godbolt

#include <optional>
#include <utility>

int g(std::optional<int>&& o) {
    if (!o)
        return -1;
    
    return *std::forward<std::optional<int>>(o);
}

[AMS21]

It seems confused by every function which returns its input godbolt

#include <optional>

template <typename T>
T identity(T t) {
    return t;
}

int f(std::optional<int>&& o) {
    if (!o) return -1;
    
    return *identity(o);
}

[AMS21]

My best guess would be that the has_value property for every std::optional returned from a function is set to false. Hence the following code also gives a warning godbolt:

#include <optional>

std::optional<int> a() { return 42; }

int f() { return *a(); }

Although a never returns an empty optional.

[AMS21]

But together a basic patch which handles the std::forward part.
https://reviews.llvm.org/D147383