[clang-tidy] performance-unnecessary-copy-initialization: Consider st…

…atic functions (llvm#119974) Static member functions can be considered the same way as free functions are, so do that.
Mel-Chen · Jan 13, 2025 · 3125531 · 3125531
1 parent d92926b
commit 3125531
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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'