[clang-tidy] false positive/negative for universal reference parameters

[yaoxinliu]

auto f1(auto&& v) { // ok, no warning
    return v;
}

// clang-tidy warning: Forwarding reference parameter 'v' is never forwarded inside the function body.
//    clang-tidy rule: cppcoreguidelines-missing-std-forward
//                see: https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/missing-std-forward.html
template<typename T>
auto f2(T&& v) { 
    return v;
}

As per P0257, v shall be moved into the return value if it is an rvalue-reference; otherwise just copy it. So return std::forward<T>(v); is not necessary; return v; is enough.

BTW, f1 is totally equivalent to f2, but f1 is ok while f2 is not. If the warning for f2 is not a false positive, then there must be a false negative for f1, either one or the other.

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: None (yaoxinliu)

``` auto f1(auto&& v) { // ok, no warning return v; }

// Trigger warning: Forwarding reference parameter 'v' is never forwarded inside the function body.
// cppcoreguidelines-missing-std-forward
template<typename T>
auto f2(T&& v) {
return v;
}

As per [P0257](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0527r1.html), `v` in `f2` shall be moved into the return value, so it is ok.

Moreover, `T&&` is a universal reference here; so, `v` is not sure to be an rvalue-reference, it may also be an lvalue-reference, in such a case, `v` should not be explicitly forwarded or moved. 
</details>

[carlosgalvezp]

The check is implementing the rule exactly as stated, so it's not a false positive:

Enforcement
Flag a function that takes a TP&& parameter (where TP is a template type parameter name) and does anything with it other than std::forwarding it exactly once on every static path, or std::forwarding it more than once but qualified with a different data member exactly once on every static path.

I suggest to open a ticket towards the CppCoreGuidelines to allow this use case as an exception.

[yaoxinliu]

The check is implementing the rule exactly as stated, so it's not a false positive:

Enforcement
Flag a function that takes a TP&& parameter (where TP is a template type parameter name) and does anything with it other than std::forwarding it exactly once on every static path, or std::forwarding it more than once but qualified with a different data member exactly once on every static path.

I suggest to open a ticket towards the CppCoreGuidelines to allow this use case as an exception.

@carlosgalvezp Why does f1 not trigger the same warning? If this is not a false positive for f2, there must be a false negative for f1, either one or the other.

[carlosgalvezp]

Yes, I agree f1 has a false negative, probably because "auto" template parameters is a relatively new thing and looks different in the AST.