maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[clang-tidy] Do not move parameter if only DeclRefExpr occurs inside of a loop 

Summary: This fixes a bug where the performance-unnecessary-value-param check suggests a fix to move the parameter inside of a loop which could be invoked multiple times.

Reviewers: sbenza, aaron.ballman, alexfh

Subscribers: JDevlieghere, cfe-commits

Differential Revision: https://reviews.llvm.org/D27187

llvm-svn: 289912
This commit is contained in:
Felix Berger 2016-12-16 02:47:56 +00:00
parent f024a56cb8
commit 519de4b692
2 changed files with 22 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
View File

@ -47,6 +47,17 @@ bool isReferencedOutsideOfCallExpr(const FunctionDecl &Function,
return !Matches.empty();
}
bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Stmt &Stmt,
ASTContext &Context) {
auto Matches =
match(findAll(declRefExpr(
equalsNode(&DeclRef),
unless(hasAncestor(stmt(anyOf(forStmt(), cxxForRangeStmt(),
whileStmt(), doStmt())))))),
Stmt, Context);
return Matches.empty();
}
} // namespace
UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
@ -105,6 +116,8 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
if (!IsConstQualified) {
auto CanonicalType = Param->getType().getCanonicalType();
if (AllDeclRefExprs.size() == 1 &&
!hasLoopStmtAncestor(**AllDeclRefExprs.begin(), *Function->getBody(),
*Result.Context) &&
((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) &&
utils::decl_ref_expr::isCopyConstructorArgument(
**AllDeclRefExprs.begin(), *Function->getBody(),

9
clang-tools-extra/test/clang-tidy/performance-unnecessary-value-param.cpp
View File

@ -225,6 +225,15 @@ void PositiveMoveOnCopyAssignment(ExpensiveMovableType E) {
// CHECK-FIXES: F = std::move(E);
}
// The argument could be moved but is not since copy statement is inside a loop.
void PositiveNoMoveInsideLoop(ExpensiveMovableType E) {
// CHECK-MESSAGES: [[@LINE-1]]:52: warning: the parameter 'E' is copied
// CHECK-FIXES: void PositiveNoMoveInsideLoop(const ExpensiveMovableType& E) {
for (;;) {
auto F = E;
}
}
void PositiveConstRefNotMoveConstructible(ExpensiveToCopyType T) {
// CHECK-MESSAGES: [[@LINE-1]]:63: warning: the parameter 'T' is copied
// CHECK-FIXES: void PositiveConstRefNotMoveConstructible(const ExpensiveToCopyType& T) {