shuaiwang updated this revision to Diff 158408.
shuaiwang added a comment.
Add test case
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50102
Files:
clang-tidy/performance/UnnecessaryValueParamCheck.cpp
test/clang-tidy/performance-unnecessary-value-param.cpp
Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===================================================================
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -15,6 +15,20 @@
void useAsConstReference(const ExpensiveToCopyType &);
void useByValue(ExpensiveToCopyType);
+template <class T> class Vector {
+ public:
+ using iterator = T*;
+ using const_iterator = const T*;
+
+ Vector(const Vector&);
+ Vector& operator=(const Vector&);
+
+ iterator begin();
+ iterator end();
+ const_iterator begin() const;
+ const_iterator end() const;
+};
+
// This class simulates std::pair<>. It is trivially copy constructible
// and trivially destructible, but not trivially copy assignable.
class SomewhatTrivial {
@@ -59,6 +73,14 @@
useByValue(Obj);
}
+void positiveVector(Vector<ExpensiveToCopyType> V) {
+ // CHECK-MESSAGES: [[@LINE-1]]:49: warning: the parameter 'V' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
+ // CHECK-FIXES: void positiveVector(const Vector<ExpensiveToCopyType>& V) {
+ for (const auto& Obj : V) {
+ useByValue(Obj);
+ }
+}
+
void positiveWithComment(const ExpensiveToCopyType /* important */ S);
// CHECK-FIXES: void positiveWithComment(const ExpensiveToCopyType& /* important */ S);
void positiveWithComment(const ExpensiveToCopyType /* important */ S) {
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -10,6 +10,7 @@
#include "UnnecessaryValueParamCheck.h"
#include "../utils/DeclRefExprUtils.h"
+#include "../utils/ExprMutationAnalyzer.h"
#include "../utils/FixItHintUtils.h"
#include "../utils/Matchers.h"
#include "../utils/TypeTraits.h"
@@ -98,43 +99,52 @@
void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param");
const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("functionDecl");
- const size_t Index = std::find(Function->parameters().begin(),
- Function->parameters().end(), Param) -
- Function->parameters().begin();
- bool IsConstQualified =
- Param->getType().getCanonicalType().isConstQualified();
-
- auto AllDeclRefExprs = utils::decl_ref_expr::allDeclRefExprs(
- *Param, *Function, *Result.Context);
- auto ConstDeclRefExprs = utils::decl_ref_expr::constReferenceDeclRefExprs(
- *Param, *Function, *Result.Context);
// Do not trigger on non-const value parameters when they are not only used as
// const.
- if (!isSubset(AllDeclRefExprs, ConstDeclRefExprs))
+ if (utils::ExprMutationAnalyzer(Function->getBody(), Result.Context)
+ .isMutated(Param))
return;
+ if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(Function)) {
+ for (const auto *Init : Ctor->inits()) {
+ if (utils::ExprMutationAnalyzer(Init->getInit(), Result.Context)
+ .isMutated(Param))
+ return;
+ }
+ }
+
+ const bool IsConstQualified =
+ Param->getType().getCanonicalType().isConstQualified();
// If the parameter is non-const, check if it has a move constructor and is
// only referenced once to copy-construct another object or whether it has a
// move assignment operator and is only referenced once when copy-assigned.
// In this case wrap DeclRefExpr with std::move() to avoid the unnecessary
// copy.
- if (!IsConstQualified && AllDeclRefExprs.size() == 1) {
- auto CanonicalType = Param->getType().getCanonicalType();
- const auto &DeclRefExpr = **AllDeclRefExprs.begin();
-
- if (!hasLoopStmtAncestor(DeclRefExpr, *Function, *Result.Context) &&
- ((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) &&
- utils::decl_ref_expr::isCopyConstructorArgument(
- DeclRefExpr, *Function, *Result.Context)) ||
- (utils::type_traits::hasNonTrivialMoveAssignment(CanonicalType) &&
- utils::decl_ref_expr::isCopyAssignmentArgument(
- DeclRefExpr, *Function, *Result.Context)))) {
- handleMoveFix(*Param, DeclRefExpr, *Result.Context);
- return;
+ if (!IsConstQualified) {
+ auto AllDeclRefExprs = utils::decl_ref_expr::allDeclRefExprs(
+ *Param, *Function, *Result.Context);
+ if (AllDeclRefExprs.size() == 1) {
+ auto CanonicalType = Param->getType().getCanonicalType();
+ const auto &DeclRefExpr = **AllDeclRefExprs.begin();
+
+ if (!hasLoopStmtAncestor(DeclRefExpr, *Function, *Result.Context) &&
+ ((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) &&
+ utils::decl_ref_expr::isCopyConstructorArgument(
+ DeclRefExpr, *Function, *Result.Context)) ||
+ (utils::type_traits::hasNonTrivialMoveAssignment(CanonicalType) &&
+ utils::decl_ref_expr::isCopyAssignmentArgument(
+ DeclRefExpr, *Function, *Result.Context)))) {
+ handleMoveFix(*Param, DeclRefExpr, *Result.Context);
+ return;
+ }
}
}
+ const size_t Index = std::find(Function->parameters().begin(),
+ Function->parameters().end(), Param) -
+ Function->parameters().begin();
+
auto Diag =
diag(Param->getLocation(),
IsConstQualified ? "the const qualified parameter %0 is "
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
