diff options
author | Felix Berger <flx@google.com> | 2016-05-13 02:47:56 +0000 |
---|---|---|
committer | Felix Berger <flx@google.com> | 2016-05-13 02:47:56 +0000 |
commit | 938ce786e6fc969f0411dd896a7b9fb9e615fa58 (patch) | |
tree | 6110185c6940fcb935782412e612c7972366e62d /clang-tidy/performance | |
parent | a53fccc212e28611777edd0c520c5d83ff388ac5 (diff) |
[clang-tidy] - PerformanceUnnecesaryCopyInitialization - only trigger for decl stmts with single VarDecl.
Summary: This fixes bug: https://llvm.org/bugs/show_bug.cgi?id=27325
Reviewers: alexfh
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D19865
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@269389 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'clang-tidy/performance')
-rw-r--r-- | clang-tidy/performance/UnnecessaryCopyInitialization.cpp | 43 | ||||
-rw-r--r-- | clang-tidy/performance/UnnecessaryCopyInitialization.h | 5 |
2 files changed, 28 insertions, 20 deletions
diff --git a/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tidy/performance/UnnecessaryCopyInitialization.cpp index aa37c651..dfaff827 100644 --- a/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -20,10 +20,6 @@ namespace { void recordFixes(const VarDecl &Var, ASTContext &Context, DiagnosticBuilder &Diagnostic) { - // Do not propose fixes in macros since we cannot place them correctly. - if (Var.getLocation().isMacroID()) - return; - Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context); if (!Var.getType().isLocalConstQualified()) Diagnostic << utils::fixit::changeVarDeclToConst(Var); @@ -57,14 +53,16 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) { auto localVarCopiedFrom = [](const internal::Matcher<Expr> &CopyCtorArg) { return compoundStmt( forEachDescendant( - varDecl(hasLocalStorage(), - hasType(matchers::isExpensiveToCopy()), - hasInitializer(cxxConstructExpr( - hasDeclaration(cxxConstructorDecl( - isCopyConstructor())), - hasArgument(0, CopyCtorArg)) - .bind("ctorCall"))) - .bind("newVarDecl"))) + declStmt( + has(varDecl(hasLocalStorage(), + hasType(matchers::isExpensiveToCopy()), + hasInitializer( + cxxConstructExpr( + hasDeclaration(cxxConstructorDecl( + isCopyConstructor())), + hasArgument(0, CopyCtorArg)) + .bind("ctorCall"))) + .bind("newVarDecl"))).bind("declStmt"))) .bind("blockStmt"); }; @@ -84,6 +82,11 @@ void UnnecessaryCopyInitialization::check( const auto *OldVar = Result.Nodes.getNodeAs<VarDecl>("oldVarDecl"); const auto *BlockStmt = Result.Nodes.getNodeAs<Stmt>("blockStmt"); const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall"); + // Do not propose fixes if the DeclStmt has multiple VarDecls or in macros + // since we cannot place them correctly. + bool IssueFix = + Result.Nodes.getNodeAs<DeclStmt>("declStmt")->isSingleDecl() && + !NewVar->getLocation().isMacroID(); // A constructor that looks like T(const T& t, bool arg = false) counts as a // copy only when it is called with default arguments for the arguments after @@ -93,14 +96,16 @@ void UnnecessaryCopyInitialization::check( return; if (OldVar == nullptr) { - handleCopyFromMethodReturn(*NewVar, *BlockStmt, *Result.Context); + handleCopyFromMethodReturn(*NewVar, *BlockStmt, IssueFix, *Result.Context); } else { - handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, *Result.Context); + handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, IssueFix, + *Result.Context); } } void UnnecessaryCopyInitialization::handleCopyFromMethodReturn( - const VarDecl &Var, const Stmt &BlockStmt, ASTContext &Context) { + const VarDecl &Var, const Stmt &BlockStmt, bool IssueFix, + ASTContext &Context) { bool IsConstQualified = Var.getType().isConstQualified(); if (!IsConstQualified && !isOnlyUsedAsConst(Var, BlockStmt, Context)) return; @@ -114,12 +119,13 @@ void UnnecessaryCopyInitialization::handleCopyFromMethodReturn( "const reference but is only used as const " "reference; consider making it a const reference") << &Var; - recordFixes(Var, Context, Diagnostic); + if (IssueFix) + recordFixes(Var, Context, Diagnostic); } void UnnecessaryCopyInitialization::handleCopyFromLocalVar( const VarDecl &NewVar, const VarDecl &OldVar, const Stmt &BlockStmt, - ASTContext &Context) { + bool IssueFix, ASTContext &Context) { if (!isOnlyUsedAsConst(NewVar, BlockStmt, Context) || !isOnlyUsedAsConst(OldVar, BlockStmt, Context)) return; @@ -128,7 +134,8 @@ void UnnecessaryCopyInitialization::handleCopyFromLocalVar( "local copy %0 of the variable %1 is never modified; " "consider avoiding the copy") << &NewVar << &OldVar; - recordFixes(NewVar, Context, Diagnostic); + if (IssueFix) + recordFixes(NewVar, Context, Diagnostic); } } // namespace performance diff --git a/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tidy/performance/UnnecessaryCopyInitialization.h index 02c7866a..dcfc6a8d 100644 --- a/clang-tidy/performance/UnnecessaryCopyInitialization.h +++ b/clang-tidy/performance/UnnecessaryCopyInitialization.h @@ -33,9 +33,10 @@ public: private: void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt, - ASTContext &Context); + bool IssueFix, ASTContext &Context); void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar, - const Stmt &BlockStmt, ASTContext &Context); + const Stmt &BlockStmt, bool IssueFix, + ASTContext &Context); }; } // namespace performance |