[clang-tools-extra] 1c1f794 - Always allow std::function to be copied.

[Felix Berger via cfe-commits]

Author: Felix Berger
Date: 2020-10-21T17:20:35-04:00
New Revision: 1c1f794c2b645ba33a98f0359bc9d30bbef89920

URL: https://github.com/llvm/llvm-project/commit/1c1f794c2b645ba33a98f0359bc9d30bbef89920
DIFF: https://github.com/llvm/llvm-project/commit/1c1f794c2b645ba33a98f0359bc9d30bbef89920.diff

LOG: Always allow std::function to be copied.

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.

Reviewed-by: aaron.ballman, gribozavr2

Differential Revision: https://reviews.llvm.org/D89332

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index 03b4450d8ca8..24847d80657c 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -63,8 +63,10 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
                    declStmt(
                        has(varDecl(hasLocalStorage(),
                                    hasType(qualType(
-                                       hasCanonicalType(
-                                           matchers::isExpensiveToCopy()),
+                                       hasCanonicalType(allOf(
+                                           matchers::isExpensiveToCopy(),
+                                           unless(hasDeclaration(namedDecl(
+                                               hasName("::std::function")))))),
                                        unless(hasDeclaration(namedDecl(
                                            matchers::matchesAnyListedName(
                                                AllowedTypes)))))),

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 7a70bc18a28c..9055529c7a3f 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
@@ -405,3 +405,58 @@ void negativeInitialzedFromFreeFunctionWithNonDefaultArg() {
   ExpensiveToCopyType Orig;
   const ExpensiveToCopyType Copy = freeFunctionWithDefaultArg(&Orig);
 }
+
+namespace std {
+inline namespace __1 {
+
+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 __1
+} // namespace std
+
+void negativeStdFunction() {
+  std::function<int()> Orig;
+  std::function<int()> Copy = Orig;
+  int i = Orig();
+}
+
+using Functor = std::function<int()>;
+
+void negativeAliasedStdFunction() {
+  Functor Orig;
+  Functor Copy = Orig;
+  int i = Orig();
+}
+
+typedef std::function<int()> TypedefFunc;
+
+void negativeTypedefedStdFunction() {
+  TypedefFunc Orig;
+  TypedefFunc Copy = Orig;
+  int i = Orig();
+}
+
+namespace fake {
+namespace std {
+template <class R, class... Args>
+struct function {
+  // Custom copy constructor makes it expensive to copy;
+  function(const function &);
+};
+} // namespace std
+
+void positiveFakeStdFunction(std::function<void(int)> F) {
+  auto Copy = F;
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: local copy 'Copy' of the variable 'F' is never modified;
+  // CHECK-FIXES: const auto& Copy = F;
+}
+
+} // namespace fake