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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
