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

[Chris Cotter via Phabricator via cfe-commits]

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


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp:64-74
+                  hasAncestor(compoundStmt(hasParent(lambdaExpr(
+                      has(cxxRecordDecl(
+                          has(cxxMethodDecl(ToParam, hasName("operator()"))))),
+                      optionally(hasDescendant(MoveCallMatcher)))))),
+                  hasAncestor(cxxConstructorDecl(
+                      ToParam, isDefinition(), unless(isMoveConstructor()),
+                      optionally(hasDescendant(MoveCallMatcher)))),
----------------
PiotrZSL wrote:
> looks like lambda context is visible as both operator() and as body to lambaExpr directly.
> this mean that it may match twice, once as functionDecl, and once as lambdaExpr.
> You can merge functionDecl with cxxConstructorDecl, just do same trick like with isMoveAssignmentOperator.
> 
> I would probably start with functionDecl, and then would try do some trick like forEachParam, but we dont have such matcher....
> You missing also isDefinition() in functionDecl even that you got hasBody.
I don't remember why I ever had lambda specific logic here, but it doesn't seem necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141569