diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp index 514ce6f6e3b87..c1514aed88f70 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -83,13 +83,19 @@ AST_MATCHER_FUNCTION_P(StatementMatcher, isConstRefReturningMethodCall, // variable being declared. The assumption is that the const reference being // returned either points to a global static variable or to a member of the // called object. - return cxxMemberCallExpr( - callee(cxxMethodDecl( - returns(hasCanonicalType(matchers::isReferenceToConst()))) - .bind(MethodDeclId)), - on(declRefExpr(to(varDecl().bind(ObjectArgId)))), - thisPointerType(namedDecl( - unless(matchers::matchesAnyListedName(ExcludedContainerTypes))))); + const auto MethodDecl = + cxxMethodDecl(returns(hasCanonicalType(matchers::isReferenceToConst()))) + .bind(MethodDeclId); + const auto ReceiverExpr = declRefExpr(to(varDecl().bind(ObjectArgId))); + const auto ReceiverType = + hasCanonicalType(recordType(hasDeclaration(namedDecl( + unless(matchers::matchesAnyListedName(ExcludedContainerTypes)))))); + + return expr(anyOf( + cxxMemberCallExpr(callee(MethodDecl), on(ReceiverExpr), + thisPointerType(ReceiverType)), + cxxOperatorCallExpr(callee(MethodDecl), hasArgument(0, ReceiverExpr), + hasArgument(0, hasType(ReceiverType))))); } AST_MATCHER_FUNCTION(StatementMatcher, isConstRefReturningFunctionCall) { diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization-excluded-container-types.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization-excluded-container-types.cpp index 88b850fe2ff82..96c7ca8a460c2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization-excluded-container-types.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization-excluded-container-types.cpp @@ -21,6 +21,7 @@ struct ExpensiveToCopy { struct ConstInCorrectType { const ExpensiveToCopy &secretlyMutates() const; + const ExpensiveToCopy &operator[](int) const; }; using NSVTE = ns::ViewType; @@ -59,12 +60,28 @@ void excludedConstIncorrectType() { E.constMethod(); } +void excludedConstIncorrectTypeOperator() { + ConstInCorrectType C; + const auto E = C[42]; + E.constMethod(); +} + void excludedConstIncorrectTypeAsPointer(ConstInCorrectType *C) { const auto E = C->secretlyMutates(); E.constMethod(); } +void excludedConstIncorrectTypeAsPointerOperator(ConstInCorrectType *C) { + const auto E = (*C)[42]; + E.constMethod(); +} + void excludedConstIncorrectTypeAsReference(const ConstInCorrectType &C) { const auto E = C.secretlyMutates(); E.constMethod(); } + +void excludedConstIncorrectTypeAsReferenceOperator(const ConstInCorrectType &C) { + const auto E = C[42]; + E.constMethod(); +} 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 4c469c966860b..5eac571c2afa7 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 @@ -3,11 +3,19 @@ template struct Iterator { void operator++(); - const T &operator*() const; + T &operator*() const; bool operator!=(const Iterator &) const; typedef const T &const_reference; }; +template +struct ConstIterator { + void operator++(); + const T &operator*() const; + bool operator!=(const ConstIterator &) const; + typedef const T &const_reference; +}; + struct ExpensiveToCopyType { ExpensiveToCopyType(); virtual ~ExpensiveToCopyType(); @@ -15,8 +23,6 @@ struct ExpensiveToCopyType { using ConstRef = const ExpensiveToCopyType &; ConstRef referenceWithAlias() const; const ExpensiveToCopyType *pointer() const; - Iterator begin() const; - Iterator end() const; void nonConstMethod(); bool constMethod() const; template @@ -24,6 +30,21 @@ struct ExpensiveToCopyType { operator int() const; // Implicit conversion to int. }; +template +struct Container { + bool empty() const; + const T& operator[](int) const; + const T& operator[](int); + Iterator begin(); + Iterator end(); + ConstIterator begin() const; + ConstIterator end() const; + void nonConstMethod(); + bool constMethod() const; +}; + +using ExpensiveToCopyContainerAlias = Container; + struct TrivialToCopyType { const TrivialToCopyType &reference() const; }; @@ -138,6 +159,94 @@ void PositiveMethodCallConstPointerParam(const ExpensiveToCopyType *const Obj) { VarCopyConstructed.constMethod(); } +void PositiveOperatorCallConstReferenceParam(const Container &C) { + const auto AutoAssigned = C[42]; + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' + // CHECK-FIXES: const auto& AutoAssigned = C[42]; + AutoAssigned.constMethod(); + + const auto AutoCopyConstructed(C[42]); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed' + // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]); + AutoCopyConstructed.constMethod(); + + const ExpensiveToCopyType VarAssigned = C[42]; + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned' + // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42]; + VarAssigned.constMethod(); + + const ExpensiveToCopyType VarCopyConstructed(C[42]); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed' + // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]); + VarCopyConstructed.constMethod(); +} + +void PositiveOperatorCallConstValueParam(const Container C) { + const auto AutoAssigned = C[42]; + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' + // CHECK-FIXES: const auto& AutoAssigned = C[42]; + AutoAssigned.constMethod(); + + const auto AutoCopyConstructed(C[42]); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed' + // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]); + AutoCopyConstructed.constMethod(); + + const ExpensiveToCopyType VarAssigned = C[42]; + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned' + // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42]; + VarAssigned.constMethod(); + + const ExpensiveToCopyType VarCopyConstructed(C[42]); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed' + // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]); + VarCopyConstructed.constMethod(); +} + +void PositiveOperatorCallConstValueParamAlias(const ExpensiveToCopyContainerAlias C) { + const auto AutoAssigned = C[42]; + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' + // CHECK-FIXES: const auto& AutoAssigned = C[42]; + AutoAssigned.constMethod(); + + const auto AutoCopyConstructed(C[42]); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed' + // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]); + AutoCopyConstructed.constMethod(); + + const ExpensiveToCopyType VarAssigned = C[42]; + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned' + // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42]; + VarAssigned.constMethod(); + + const ExpensiveToCopyType VarCopyConstructed(C[42]); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed' + // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]); + VarCopyConstructed.constMethod(); +} + +void PositiveOperatorCallConstValueParam(const Container* C) { + const auto AutoAssigned = (*C)[42]; + // TODO-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' + // TODO-FIXES: const auto& AutoAssigned = (*C)[42]; + AutoAssigned.constMethod(); + + const auto AutoCopyConstructed((*C)[42]); + // TODO-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed' + // TODO-FIXES: const auto& AutoCopyConstructed((*C)[42]); + AutoCopyConstructed.constMethod(); + + const ExpensiveToCopyType VarAssigned = C->operator[](42); + // TODO-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned' + // TODO-FIXES: const ExpensiveToCopyType& VarAssigned = C->operator[](42); + VarAssigned.constMethod(); + + const ExpensiveToCopyType VarCopyConstructed(C->operator[](42)); + // TODO-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed' + // TODO-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C->operator[](42)); + VarCopyConstructed.constMethod(); +} + void PositiveLocalConstValue() { const ExpensiveToCopyType Obj; const auto UnnecessaryCopy = Obj.reference(); @@ -443,21 +552,9 @@ void WarningOnlyMultiDeclStmt() { } class Element {}; -class Container { -public: - class Iterator { - public: - void operator++(); - Element operator*(); - bool operator!=(const Iterator &); - WeirdCopyCtorType c; - }; - const Iterator &begin() const; - const Iterator &end() const; -}; void implicitVarFalsePositive() { - for (const Element &E : Container()) { + for (const Element &E : Container()) { } } @@ -621,8 +718,30 @@ void positiveUnusedReferenceIsRemoved() { // CHECK-FIXES: // Comments on a new line should not be deleted. } -void negativeloopedOverObjectIsModified() { - ExpensiveToCopyType Orig; +void positiveLoopedOverObjectIsConst() { + const Container Orig; + for (const auto &Element : Orig) { + const auto Copy = Element; + // CHECK-MESSAGES: [[@LINE-1]]:16: warning: local copy 'Copy' + // CHECK-FIXES: const auto& Copy = Element; + Orig.constMethod(); + Copy.constMethod(); + } + + auto Lambda = []() { + const Container Orig; + for (const auto &Element : Orig) { + const auto Copy = Element; + // CHECK-MESSAGES: [[@LINE-1]]:18: warning: local copy 'Copy' + // CHECK-FIXES: const auto& Copy = Element; + Orig.constMethod(); + Copy.constMethod(); + } + }; +} + +void negativeLoopedOverObjectIsModified() { + Container Orig; for (const auto &Element : Orig) { const auto Copy = Element; Orig.nonConstMethod(); @@ -630,7 +749,7 @@ void negativeloopedOverObjectIsModified() { } auto Lambda = []() { - ExpensiveToCopyType Orig; + Container Orig; for (const auto &Element : Orig) { const auto Copy = Element; Orig.nonConstMethod();