diff options
3 files changed, 40 insertions, 9 deletions
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp index 034894c..dc2e8a3 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -104,15 +104,18 @@ AST_MATCHER_FUNCTION_P(StatementMatcher, hasArgument(0, hasType(ReceiverType))))); } +AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); } + AST_MATCHER_FUNCTION(StatementMatcher, isConstRefReturningFunctionCall) { - // Only allow initialization of a const reference from a free function if it - // has no arguments. Otherwise it could return an alias to one of its - // arguments and the arguments need to be checked for const use as well. - return callExpr(callee(functionDecl(returns(hasCanonicalType( - matchers::isReferenceToConst()))) - .bind(FunctionDeclId)), - argumentCountIs(0), unless(callee(cxxMethodDecl()))) - .bind(InitFunctionCallId); + // Only allow initialization of a const reference from a free function or + // static member function if it has no arguments. Otherwise it could return + // an alias to one of its arguments and the arguments need to be checked + // for const use as well. + return callExpr(argumentCountIs(0), + callee(functionDecl(returns(hasCanonicalType(matchers::isReferenceToConst())), + unless(cxxMethodDecl(unless(isStatic())))) + .bind(FunctionDeclId))) + .bind(InitFunctionCallId); } AST_MATCHER_FUNCTION_P(StatementMatcher, initializerReturnsReferenceToConst, @@ -232,7 +235,7 @@ UnnecessaryCopyInitialization::UnnecessaryCopyInitialization( Options.get("ExcludedContainerTypes", ""))) {} void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) { - auto LocalVarCopiedFrom = [this](const internal::Matcher<Expr> &CopyCtorArg) { + auto LocalVarCopiedFrom = [this](const ast_matchers::internal::Matcher<Expr> &CopyCtorArg) { return compoundStmt( forEachDescendant( declStmt( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 5c7ba43..0815632 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -339,6 +339,10 @@ Changes in existing checks <clang-tidy/checks/performance/move-const-arg>` check to fix a crash when an argument type is declared but not defined. +- Improved :doc:`performance-unnecessary-copy-initialization` + <clang-tidy/checks/performance/unnecessary-copy-initialization> check + to consider static member functions the same way as free functions. + - Improved :doc:`readability-container-contains <clang-tidy/checks/readability/container-contains>` check to let it work on any class that has a ``contains`` method. Fix some false negatives in the 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 d02bb98..b532577 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 @@ -28,6 +28,8 @@ struct ExpensiveToCopyType { template <typename A> const A &templatedAccessor() const; operator int() const; // Implicit conversion to int. + + static const ExpensiveToCopyType &instance(); }; template <typename T> @@ -100,6 +102,28 @@ void PositiveFunctionCall() { VarCopyConstructed.constMethod(); } +void PositiveStaticMethodCall() { + const auto AutoAssigned = ExpensiveToCopyType::instance(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization] + // CHECK-FIXES: const auto& AutoAssigned = ExpensiveToCopyType::instance(); + AutoAssigned.constMethod(); + + const auto AutoCopyConstructed(ExpensiveToCopyType::instance()); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed' + // CHECK-FIXES: const auto& AutoCopyConstructed(ExpensiveToCopyType::instance()); + AutoCopyConstructed.constMethod(); + + const ExpensiveToCopyType VarAssigned = ExpensiveToCopyType::instance(); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned' + // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = ExpensiveToCopyType::instance(); + VarAssigned.constMethod(); + + const ExpensiveToCopyType VarCopyConstructed(ExpensiveToCopyType::instance()); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed' + // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(ExpensiveToCopyType::instance()); + VarCopyConstructed.constMethod(); +} + void PositiveMethodCallConstReferenceParam(const ExpensiveToCopyType &Obj) { const auto AutoAssigned = Obj.reference(); // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' |