[PATCH] D119165: [clang-tidy] Add processing lambda captures at bugprone-use-after-move check

Ivan Murashko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 11 11:08:36 PST 2022


ivanmurashko added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:416
 
+  auto CallMoveMatcherLambda = lambdaExpr(
+      forEachLambdaCapture(lambdaCapture(capturesVar(varDecl(
----------------
sammccall wrote:
> sammccall wrote:
> > do we want to bind the lambda itself as `moving-call`?
> we should probably have a comment explaining *why* lambdas get handled specially.
> 
> If I understand right:
> - the normal matcher would already match
> - but either MovingCall or ContainingLambda (which?) point at unhelpful nodes and
> - we end up doing the analysis inside the lambda rather than in the enclosing function
> - so never find the following use
> 
> (I wonder if it's possible to fix this slightly more directly by tweaking the MovingCall or ContainingLambda logic)
> If I understand right:

There are some troubles with the original matcher. The most obvious one is correctly described at your comment :
The original matcher
```
callExpr(callee(functionDecl(hasName("::std::move"))),
               ... hasAncestor(lambdaExpr().bind("containing-lambda")),
               ...
```
applied to the code
```
auto []() {                     // lambda_1
   int a = 0;
   ...
   auto [](aa = std::move(a)) { // lambda_2
   }
}
```
will return *lambda_2* binded to the "containing-lambda" but we expect it to be *lambda_1*



> (I wonder if it's possible to fix this slightly more directly by tweaking the MovingCall or ContainingLambda logic)
It would be good to find it if it's possible


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:1356
+int lambdaCaptures() {
+  int a = 0;
+  int b = 0;
----------------
sammccall wrote:
> It's pretty interesting that use-after-move fires for ints!
> 
> Someone might decide to "fix" that though, so probably best to use A like the other tests.
There is also another case that I want to address as a separate patch.
```
void autoCapture() {
  auto var = [](auto &&res) {
    auto f = [blk = std::move(res)]() {};
    return std::move(res);
  };
}
```
This one is matched as `UnresolvedLookupExpr` and requires another modified matcher


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119165/new/

https://reviews.llvm.org/D119165



More information about the cfe-commits mailing list