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

[Julian Schmidt via cfe-commits]

================
@@ -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"),
----------------
5chmidti wrote:

While looking at this PR again, I came up with this matcher that makes checking for sub-lambdas/-functions more readable:

```c++
  // Matching on functions, that return an owner/resource, but don't declare
  // their return type as owner.
  Finder->addMatcher(
      functionDecl(
          decl().bind("function_decl"),
          hasBody(stmt(stmt().bind("body"),
                       hasDescendant(returnStmt(hasReturnValue(ConsideredOwner))
                                         .bind("bad_owner_return")),
                       unless(hasDescendant(decl(
                           ScopeDeclaration, hasDescendant(stmt(equalsBoundNode(
                                                 "bad_owner_return")))))),
                       unless(hasDescendant(lambdaExpr(hasDescendant(
                           stmt(equalsBoundNode("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(
          hasLambdaBody(stmt(
              stmt().bind("body"),
              hasDescendant(returnStmt(hasReturnValue(ConsideredOwner))
                                .bind("bad_owner_return")),
              unless(hasDescendant(decl(
                  ScopeDeclaration,
                  hasDescendant(stmt(equalsBoundNode("bad_owner_return")))))),
              unless(hasDescendant(lambdaExpr(
                  hasDescendant(stmt(equalsBoundNode("bad_owner_return")))))))),
          hasCallOperator(returns(
              qualType(unless(hasDeclaration(OwnerDecl))).bind("result"))))
          .bind("lambda"),
      this);
```

Note that I used `ScopeDeclaration` in both matchers, while you only used it in the matcher where `lambdaExpr` was the top matcher. I'm not sure if this was intended by you or not.

There is also an opportunity to give these sub-matchers good names and reuse them (either parts of them or whole), e.g.: `ReturnIsPartOfCurrentBody` for both `unless` sub-matchers.

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