[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter marked 2 inline comments as done.
ccotter added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp:103-125
+  StatementMatcher MoveCallMatcher = callExpr(
+      anyOf(callee(functionDecl(hasName("::std::move"))),
+            callee(unresolvedLookupExpr(hasAnyDeclaration(
+                namedDecl(hasUnderlyingDecl(hasName("::std::move"))))))),
+      argumentCountIs(1), hasArgument(0, moveArgumentOf(StrictMode, Param)));
+
+  SmallVector<BoundNodes, 1> Matches;
----------------
PiotrZSL wrote:
> ```
> unless(hasDescendant(callExpr(
>       anyOf(callee(functionDecl(hasName("::std::move"))),
>             callee(unresolvedLookupExpr(hasAnyDeclaration(
>                 namedDecl(hasUnderlyingDecl(hasName("::std::move"))))))),
>       argumentCountIs(1), hasArgument(0, moveArgumentOf(StrictMode, Param)))))
> ```
> 
> any here you could also check lambdas, if you want to ignore copies, but not should why you should do that, but you can always bind this to "move", or implement lambda checking as matcher.... unless(hasAncestor(lambdaExpr(isCapturedByCopy....
> 
> Simply as you have some tests already, try pack as much as you can into registerMatchers, because once you split this across multiple submatches in diffrent functions, then it could be hard to maintain.
Good call. Single matcher expr is cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569