Author: Felix Berger Date: 2021-06-18T15:25:17-04:00 New Revision: bdd5da9dec61072f693726d9ed2a94c78e431ba2
URL: https://github.com/llvm/llvm-project/commit/bdd5da9dec61072f693726d9ed2a94c78e431ba2 DIFF: https://github.com/llvm/llvm-project/commit/bdd5da9dec61072f693726d9ed2a94c78e431ba2.diff LOG: [clang-tidy] performance-unnecessary-copy-initialization: Directly examine the initializing var's initializer. This fixes false positive cases where a reference is initialized outside of a block statement and then its initializing variable is modified. Another case is when the looped over container is modified. Differential Revision: https://reviews.llvm.org/D103021 Reviewed-by: ymandel Added: Modified: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp index 27ce36e49a073..f75a3a901ecd5 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -12,6 +12,7 @@ #include "../utils/LexerUtils.h" #include "../utils/Matchers.h" #include "../utils/OptionsUtils.h" +#include "clang/AST/Decl.h" #include "clang/Basic/Diagnostic.h" namespace clang { @@ -72,14 +73,14 @@ AST_MATCHER_FUNCTION(StatementMatcher, isConstRefReturningFunctionCall) { .bind(InitFunctionCallId); } -AST_MATCHER_FUNCTION(StatementMatcher, isInitializedFromReferenceToConst) { +AST_MATCHER_FUNCTION(StatementMatcher, initializerReturnsReferenceToConst) { auto OldVarDeclRef = declRefExpr(to(varDecl(hasLocalStorage()).bind(OldVarDeclId))); - return declStmt(has(varDecl(hasInitializer( + return expr( anyOf(isConstRefReturningFunctionCall(), isConstRefReturningMethodCall(), ignoringImpCasts(OldVarDeclRef), - ignoringImpCasts(unaryOperator( - hasOperatorName("&"), hasUnaryOperand(OldVarDeclRef)))))))); + ignoringImpCasts(unaryOperator(hasOperatorName("&"), + hasUnaryOperand(OldVarDeclRef))))); } // This checks that the variable itself is only used as const, and also makes @@ -106,18 +107,14 @@ static bool isInitializingVariableImmutable(const VarDecl &InitializingVar, if (!isa<ReferenceType, PointerType>(T)) return true; - auto Matches = - match(findAll(declStmt(has(varDecl(equalsNode(&InitializingVar)))) - .bind("declStmt")), - BlockStmt, Context); - // The reference or pointer is not initialized in the BlockStmt. We assume - // its pointee is not modified then. - if (Matches.empty()) + // The reference or pointer is not declared and hence not initialized anywhere + // in the function. We assume its pointee is not modified then. + if (!InitializingVar.isLocalVarDecl() || !InitializingVar.hasInit()) { return true; + } - const auto *Initialization = selectFirst<DeclStmt>("declStmt", Matches); - Matches = - match(isInitializedFromReferenceToConst(), *Initialization, Context); + auto Matches = match(initializerReturnsReferenceToConst(), + *InitializingVar.getInit(), Context); // The reference is initialized from a free function without arguments // returning a const reference. This is a global immutable object. if (selectFirst<CallExpr>(InitFunctionCallId, Matches) != nullptr) diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h index 8193672218932..c11be2d8885d6 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_COPY_INITIALIZATION_H #include "../ClangTidyCheck.h" +#include "clang/AST/Decl.h" namespace clang { namespace tidy { diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp index e894f84f11d8c..c2b3d8f9c7395 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp @@ -1,10 +1,20 @@ // RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t +template <typename T> +struct Iterator { + void operator++(); + const T &operator*() const; + bool operator!=(const Iterator &) const; + typedef const T &const_reference; +}; + struct ExpensiveToCopyType { ExpensiveToCopyType(); virtual ~ExpensiveToCopyType(); const ExpensiveToCopyType &reference() const; const ExpensiveToCopyType *pointer() const; + Iterator<ExpensiveToCopyType> begin() const; + Iterator<ExpensiveToCopyType> end() const; void nonConstMethod(); bool constMethod() const; }; @@ -589,3 +599,41 @@ void positiveUnusedReferenceIsRemoved() { // CHECK-FIXES-NOT: auto TrailingCommentRemoved = ExpensiveTypeReference(); // Trailing comment. // clang-format on } + +void negativeloopedOverObjectIsModified() { + ExpensiveToCopyType Orig; + for (const auto &Element : Orig) { + const auto Copy = Element; + Orig.nonConstMethod(); + Copy.constMethod(); + } + + auto Lambda = []() { + ExpensiveToCopyType Orig; + for (const auto &Element : Orig) { + const auto Copy = Element; + Orig.nonConstMethod(); + Copy.constMethod(); + } + }; +} + +void negativeReferenceIsInitializedOutsideOfBlock() { + ExpensiveToCopyType Orig; + const auto &E2 = Orig; + if (1 != 2 * 3) { + const auto C2 = E2; + Orig.nonConstMethod(); + C2.constMethod(); + } + + auto Lambda = []() { + ExpensiveToCopyType Orig; + const auto &E2 = Orig; + if (1 != 2 * 3) { + const auto C2 = E2; + Orig.nonConstMethod(); + C2.constMethod(); + } + }; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits