[clang-tidy] False positive for cppcoreguidelines-missing-std-forward when forwarding into lambda

[nilssonk]

The following seemingly valid code

#include <iostream>
#include <utility>

template<typename F>
auto make_lambda(F&& f)
{
    return [f = std::forward<F>(f)] { f(); };
}

int main()
{
    auto g = make_lambda([] { std::cout << "hello\n"; });
    g();
}

results in the error

repro.cpp:5:22: error: forwarding reference parameter 'f' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward,-warnings-as-errors]
    5 | auto make_lambda(F&& f)

Encountered on

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 22.04.3 LTS
Release:        22.04
Codename:       jammy

$ clang-tidy --version
Ubuntu LLVM version 17.0.2
  Optimized build.

installed with the script from https://apt.llvm.org/llvm.sh .

[AMS21]

Simplified test case to

#include <utility>

template <typename T>
void f(T&& x)
{
    [y = std::forward(x)] {};
}

godbolt

[jcsxky]

This is not a false positive, because variable f used in

template<typename F>
auto make_lambda(F&& f)
{
    return [f = std::forward<F>(f)] { f(); };
}

is located in lambda expression, not in function make_lambda.

[nilssonk]

This is not a false positive, because variable f
is located in lambda expression, not in function make_lambda.

See the simplified test case by @AMS21

[jcsxky]

This is not a false positive, because variable f
is located in lambda expression, not in function make_lambda.

See the simplified test case by @AMS21

I see two cases in test of cppcoreguidelines-missing-std-forward, but I'm not sure whether it's same with the code you list here.

template <class T>
void lambda_value_capture(T&& t) {
  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
  [=]() { T other = std::forward<T>(t); };
}

template <class T>
void lambda_value_reference(T&& t) {
  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
  [&]() { T other = std::forward<T>(t); };
}

If they are different and indeed it's a bug, I'm pleasure to fix it.

[iillyyaa]

@jcsxky I am watching this ticket as well, and can offer some insight on your question.

The two lambdas in the test cases you show are never invoked in the body of the function being tested. So, in essence, the lambda body in both is dead code, with the possible exception that T's copy constructor might have an observable side-effect. I am not sure whether this clang-tidy check would be discerning enough to see that. However, if I were to add an immediate invocation of these lambdas, then they would become unconditionally "live", and then there would indeed be a clear difference between them.

 template <class T>
 void lambda_value_capture(T&& t) {
   // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
-  [=]() { T other = std::forward<T>(t); };
+  [=]() { T other = std::forward<T>(t); }();
 }
 
 template <class T>
 void lambda_value_reference(T&& t) {
   // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
-  [&]() { T other = std::forward<T>(t); };
+  [&]() { T other = std::forward<T>(t); }();
 }

The first one captures a copy of t into the lambda without taking advantage of t's value category for making the copy, so even in the presence of std::forward<T>(t) in the lambda body, this should still be flagged as failing the check.

The second one captures a reference to t, and then in the body of the lambda makes a copy, while taking advantage of t's value category. So this case should not be flagged as failing the check.

There is a place for a third case that could be added:

template <class T>
void lambda_value_forwarded_capture(T&& t) {
  [t = std::forward<T>(t)]() mutable { T other = std::move(t); }();
}

This is distinct from the first two, in that it performs an effective forwarding at capture time.

I hope this is helpful.

[jcsxky]

@iillyyaa Thanks for your detail explanation!
Captures a reference or forwarding at capture time are both can pass this clang-tidy checker, right? I will look into this checker later and have a look whether I can fix it.

[iillyyaa]

Captures a reference or forwarding at capture time are both can pass this clang-tidy checker, right?

Capturing by reference and then forwarding in the lambda body, or capturing by value from a forwarded reference should pass.

Capturing by value without forwarding at capture time should not pass.

Thanks for looking into it!