[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

[Felix Berger via Phabricator via cfe-commits]

flx updated this revision to Diff 298745.
flx added a comment.

Add more complete fake version of std::function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89332/new/

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,36 @@
   ExpensiveToCopyType Orig;
   const ExpensiveToCopyType Copy = freeFunctionWithDefaultArg(&Orig);
 }
+
+// Let's fake a minimal std::function-like facility.
+namespace std {
+template <typename _Tp>
+_Tp declval();
+
+template <typename _Functor, typename... _ArgTypes>
+struct __res {
+  template <typename... _Args>
+  static decltype(declval<_Functor>()(_Args()...)) _S_test(int);
+
+  template <typename...>
+  static void _S_test(...);
+
+  using type = decltype(_S_test<_ArgTypes...>(0));
+};
+
+template <typename>
+struct function;
+
+template <typename... _ArgTypes>
+struct function<void(_ArgTypes...)> {
+  template <typename _Functor,
+            typename = typename __res<_Functor, _ArgTypes...>::type>
+  function(_Functor) {}
+};
+} // 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.298745.patch
Type: text/x-patch
Size: 3795 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201016/e3534e8b/attachment-0001.bin>