[PATCH] D28022: [clang-tidy] Handle constructors in performance-unnecessary-value-param

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:114
     if (AllDeclRefExprs.size() == 1 &&
-        !hasLoopStmtAncestor(**AllDeclRefExprs.begin(), *Function->getBody(),
-                             *Result.Context) &&
+        !hasLoopStmtAncestor(**AllDeclRefExprs.begin(),
+                             *Function->getDefinition(), *Result.Context) &&
----------------
How about pulling out variables for `**AllDeclRefExprs.begin()` and `*Function->getDefinition()` to remove some boilerplate?


================
Comment at: clang-tidy/utils/DeclRefExprUtils.h:35
 
+// Returns set of all DeclRefExprs to VarDecl in Decl.
+llvm::SmallPtrSet<const DeclRefExpr *, 16>
----------------
Please convert the comment (and the other ones the patch touches) to doxygen style.


================
Comment at: clang-tidy/utils/DeclRefExprUtils.h:55-56
 
 // Returns true if DeclRefExpr is the argument of a copy-assignment operator
 // call expr.
+bool isCopyAssignmentArgument(const DeclRefExpr &DeclRef, const Decl &Decl,
----------------
The comment is now confusing. It says nothing about `Decl` and doesn't make it clear where the "call expr" (btw, it should either be `CallExpr`, if it references to the corresponding type or "call expression" otherwise) comes from. Same above.


https://reviews.llvm.org/D28022