[clang-tools-extra] [clang-tidy] Add support for lambdas in cppcoreguidelines-owning-memory (PR #77246)

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 5 13:36:23 PST 2024


================
@@ -147,10 +161,51 @@ void OwningMemoryCheck::registerMatchers(MatchFinder *Finder) {
   // Matching on functions, that return an owner/resource, but don't declare
   // their return type as owner.
   Finder->addMatcher(
-      functionDecl(hasDescendant(returnStmt(hasReturnValue(ConsideredOwner))
-                                     .bind("bad_owner_return")),
-                   unless(returns(qualType(hasDeclaration(OwnerDecl)))))
-          .bind("function_decl"),
+      functionDecl(
+          decl().bind("function_decl"),
+          hasBody(stmt(
+              stmt().bind("body"),
+              hasDescendant(
+                  returnStmt(hasReturnValue(ConsideredOwner),
+                             // Ignore sub-lambda expressions
+                             hasAncestor(stmt(anyOf(equalsBoundNode("body"),
+                                                    lambdaExpr()))
+                                             .bind("scope")),
+                             hasAncestor(stmt(equalsBoundNode("scope"),
+                                              equalsBoundNode("body"))),
+                             // Ignore sub-functions
+                             hasAncestor(functionDecl().bind("context")),
+                             hasAncestor(functionDecl(
+                                 equalsBoundNode("context"),
+                                 equalsBoundNode("function_decl"))))
+                      .bind("bad_owner_return")))),
+          returns(qualType(unless(hasDeclaration(OwnerDecl))).bind("result"))),
+      this);
+
+  // Matching on lambdas, that return an owner/resource, but don't declare
+  // their return type as owner.
+  Finder->addMatcher(
+      lambdaExpr(
+          hasAncestor(decl(ScopeDeclaration).bind("scope-decl")),
+          hasLambdaBody(stmt(
+              stmt().bind("body"),
+              hasDescendant(
+                  returnStmt(
+                      hasReturnValue(ConsideredOwner),
+                      // Ignore sub-lambdas
+                      hasAncestor(
+                          stmt(anyOf(equalsBoundNode("body"), lambdaExpr()))
+                              .bind("scope")),
+                      hasAncestor(stmt(equalsBoundNode("scope"),
+                                       equalsBoundNode("body"))),
+                      // Ignore sub-functions
+                      hasAncestor(decl(ScopeDeclaration).bind("context")),
+                      hasAncestor(decl(equalsBoundNode("context"),
+                                       equalsBoundNode("scope-decl"))))
+                      .bind("bad_owner_return")))),
+          hasCallOperator(returns(
+              qualType(unless(hasDeclaration(OwnerDecl))).bind("result"))))
+          .bind("lambda"),
----------------
PiotrZSL wrote:

This won't work. What it does is:
- find first return in function/lambda
- check if this return is not in other function/lambda

This mean that it won't detect issue in this MakeS lambda:
```
 void testLambdaInLambdaWithDoubleReturns() {
    const auto MakeS = []() -> S* {
      const auto MakeS2 = []() -> S* {
        return ::gsl::owner<S*>{new S(1)};
      };
     return ::gsl::owner<S*>{new S(2)};
    };
  }

```
but it will work only for S2, where current code works for both.

https://github.com/llvm/llvm-project/pull/77246


More information about the cfe-commits mailing list