Author: flx Date: Thu Dec 15 20:47:56 2016 New Revision: 289912 URL: http://llvm.org/viewvc/llvm-project?rev=289912&view=rev Log: [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 Modified: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param.cpp Modified: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp?rev=289912&r1=289911&r2=289912&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp Thu Dec 15 20:47:56 2016 @@ -47,6 +47,17 @@ bool isReferencedOutsideOfCallExpr(const 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(c 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(), Modified: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param.cpp?rev=289912&r1=289911&r2=289912&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param.cpp Thu Dec 15 20:47:56 2016 @@ -225,6 +225,15 @@ void PositiveMoveOnCopyAssignment(Expens // 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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits