[PATCH] D89332: Always allow std::function to be copied.

[Felix Berger via Phabricator via cfe-commits]

flx created this revision.
flx added a reviewer: aaron.ballman.
Herald added a project: clang.
flx requested review of this revision.

Since its call operator is const but can modify the state of its underlying
functor we cannot tell whether the copy is necessary or not.

This avoids false positives.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89332

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -405,3 +405,23 @@
   ExpensiveToCopyType Orig;
   const ExpensiveToCopyType Copy = freeFunctionWithDefaultArg(&Orig);
 }
+
+namespace std {
+
+template <class>
+class function;
+template <class R, class... Args>
+class function<R(Args...)> {
+public:
+  function();
+  function(const function &other);
+  R operator()(Args... args) const;
+};
+
+} // namespace std
+
+void negativeStdFunction() {
+  std::function<int()> Orig;
+  std::function<int()> Copy = Orig;
+  int i = Orig();
+}
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -19,6 +19,10 @@
 namespace performance {
 namespace {
 
+using namespace ::clang::ast_matchers;
+using ::clang::ast_matchers::internal::Matcher;
+using utils::decl_ref_expr::isOnlyUsedAsConst;
+
 void recordFixes(const VarDecl &Var, ASTContext &Context,
                  DiagnosticBuilder &Diagnostic) {
   Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context);
@@ -29,10 +33,17 @@
   }
 }
 
-} // namespace
+// Always allow std::function to be copied. Since its call operator is const but
+// can modify the state of the underlying functor we cannot ell whether the copy
+// is unnecessary.
+AST_MATCHER(NamedDecl, isStdFunction) {
+  // First use the fast getName() method to avoid unnecessary calls to the
+  // slow getQualifiedNameAsString().
+  return Node.getName() == "function" &&
+         Node.getQualifiedNameAsString() == "std::function";
+}
 
-using namespace ::clang::ast_matchers;
-using utils::decl_ref_expr::isOnlyUsedAsConst;
+} // namespace
 
 UnnecessaryCopyInitialization::UnnecessaryCopyInitialization(
     StringRef Name, ClangTidyContext *Context)
@@ -57,7 +68,7 @@
                unless(callee(cxxMethodDecl())))
           .bind("initFunctionCall");
 
-  auto localVarCopiedFrom = [this](const internal::Matcher<Expr> &CopyCtorArg) {
+  auto localVarCopiedFrom = [this](const Matcher<Expr> &CopyCtorArg) {
     return compoundStmt(
                forEachDescendant(
                    declStmt(
@@ -66,8 +77,9 @@
                                        hasCanonicalType(
                                            matchers::isExpensiveToCopy()),
                                        unless(hasDeclaration(namedDecl(
-                                           matchers::matchesAnyListedName(
-                                               AllowedTypes)))))),
+                                           anyOf(matchers::matchesAnyListedName(
+                                                     AllowedTypes),
+                                                 isStdFunction())))))),
                                    unless(isImplicit()),
                                    hasInitializer(traverse(
                                        ast_type_traits::TK_AsIs,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89332.297905.patch
Type: text/x-patch
Size: 3374 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201013/04cafa77/attachment.bin>