baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: flx, alexfh, aaron.ballman, lebedev.ri.
baloghadamsoftware added a project: clang-tools-extra.

New option added to these three checks to be able to silence false positives on 
types that are intentionally passed by value or copied. Such types are e.g. 
intrusive reference counting pointer types like `llvm::IntrusiveRefCntPtr`. The 
new option is named `WhiteListTypes` and can contain a semicolon-separated list 
of names of these types. Regular expressions are allowed. Default is empty.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52727

Files:
  clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tidy/performance/ForRangeCopyCheck.h
  clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tidy/performance/UnnecessaryCopyInitialization.h
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tidy/performance/UnnecessaryValueParamCheck.h
  docs/clang-tidy/checks/performance-for-range-copy.rst
  docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  docs/clang-tidy/checks/performance-unnecessary-value-param.rst
  test/clang-tidy/performance-for-range-copy.cpp
  test/clang-tidy/performance-unnecessary-copy-initialization.cpp
  test/clang-tidy/performance-unnecessary-value-param.cpp

Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===================================================================
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t
+// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t -- -config="{CheckOptions: [{key: performance-unnecessary-value-param.WhiteListTypes, value: '[Pp]ointer$|[Pp]tr$|[Rr]ef(erence)?$'}]}" --
 
 // CHECK-FIXES: #include <utility>
 
@@ -123,6 +123,8 @@
 void negativeArray(const ExpensiveToCopyType[]) {
 }
 
+// Test white list
+
 void negativePointer(ExpensiveToCopyType* Obj) {
 }
 
@@ -381,3 +383,59 @@
   // CHECK-FIXES: void templateFunction<ExpensiveToCopyType>(ExpensiveToCopyType E) {
   E.constReference();
 }
+
+struct SmartPointer {
+  ~SmartPointer();
+};
+
+struct smart_pointer {
+  ~smart_pointer();
+};
+
+struct SmartPtr {
+  ~SmartPtr();
+};
+
+struct smart_ptr {
+  ~smart_ptr();
+};
+
+struct SmartReference {
+  ~SmartReference();
+};
+
+struct smart_reference {
+  ~smart_reference();
+};
+
+struct SmartRef {
+  ~SmartRef();
+};
+
+struct smart_ref {
+  ~smart_ref();
+};
+
+void negativeSmartPointer(SmartPointer P) {
+}
+
+void negative_smart_pointer(smart_pointer p) {
+}
+
+void negativeSmartPtr(SmartPtr P) {
+}
+
+void negative_smart_ptr(smart_ptr p) {
+}
+
+void negativeSmartReference(SmartReference R) {
+}
+
+void negative_smart_reference(smart_reference r) {
+}
+
+void negativeSmartRef(SmartRef R) {
+}
+
+void negative_smart_ref(smart_ref r) {
+}
Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp
===================================================================
--- test/clang-tidy/performance-unnecessary-copy-initialization.cpp
+++ test/clang-tidy/performance-unnecessary-copy-initialization.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
+// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t -- -config="{CheckOptions: [{key: performance-unnecessary-copy-initialization.WhiteListTypes, value: '[Pp]ointer$|[Pp]tr$|[Rr]ef(erence)?$'}]}" --
 
 struct ExpensiveToCopyType {
   ExpensiveToCopyType();
@@ -387,3 +387,78 @@
   for (const Element &E : Container()) {
   }
 }
+
+// Test white list
+
+struct SmartPointer {
+  ~SmartPointer();
+};
+
+struct smart_pointer {
+  ~smart_pointer();
+};
+
+struct SmartPtr {
+  ~SmartPtr();
+};
+
+struct smart_ptr {
+  ~smart_ptr();
+};
+
+struct SmartReference {
+  ~SmartReference();
+};
+
+struct smart_reference {
+  ~smart_reference();
+};
+
+struct SmartRef {
+  ~SmartRef();
+};
+
+struct smart_ref {
+  ~smart_ref();
+};
+
+const SmartPointer &getSmartPointer();
+const smart_pointer &get_smart_pointer();
+const SmartPtr &getSmartPtr();
+const smart_ptr &get_smart_ptr();
+const SmartReference &getSmartReference();
+const smart_reference &get_smart_reference();
+const SmartRef &getSmartRef();
+const smart_ref &get_smart_ref();
+
+void negativeSmartPointer() {
+  const auto P = getSmartPointer();
+}
+
+void negative_smart_pointer() {
+  const auto p = get_smart_pointer();
+}
+
+void negativeSmartPtr() {
+  const auto P = getSmartPtr();
+}
+
+void negative_smart_ptr() {
+  const auto p = get_smart_ptr();
+}
+
+void negativeSmartReference() {
+  const auto R = getSmartReference();
+}
+
+void negative_smart_reference() {
+  const auto r = get_smart_reference();
+}
+
+void negativeSmartRef() {
+  const auto R = getSmartRef();
+}
+
+void negative_smart_ref() {
+  const auto r = get_smart_ref();
+}
Index: test/clang-tidy/performance-for-range-copy.cpp
===================================================================
--- test/clang-tidy/performance-for-range-copy.cpp
+++ test/clang-tidy/performance-for-range-copy.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s performance-for-range-copy %t -- -- -std=c++11 -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s performance-for-range-copy %t -- -config="{CheckOptions: [{key: performance-for-range-copy.WhiteListTypes, value: '[Pp]ointer$|[Pp]tr$|[Rr]ef(erence)?$'}]}" -- -std=c++11 -fno-delayed-template-parsing
 
 namespace std {
 
@@ -270,3 +270,85 @@
   for (auto _ : View<Iterator<S>>()) {
   }
 }
+
+// Test white list
+
+struct SmartPointer {
+  ~SmartPointer();
+};
+
+struct smart_pointer {
+  ~smart_pointer();
+};
+
+struct SmartPtr {
+  ~SmartPtr();
+};
+
+struct smart_ptr {
+  ~smart_ptr();
+};
+
+struct SmartReference {
+  ~SmartReference();
+};
+
+struct smart_reference {
+  ~smart_reference();
+};
+
+struct SmartRef {
+  ~SmartRef();
+};
+
+struct smart_ref {
+  ~smart_ref();
+};
+
+void negativeSmartPointer() {
+  for (auto P : View<Iterator<SmartPointer>>()) {
+    auto P2 = P;
+  }
+}
+
+void negative_smart_pointer() {
+  for (auto p : View<Iterator<smart_pointer>>()) {
+    auto p2 = p;
+  }
+}
+
+void negativeSmartPtr() {
+  for (auto P : View<Iterator<SmartPtr>>()) {
+    auto P2 = P;
+  }
+}
+
+void negative_smart_ptr() {
+  for (auto p : View<Iterator<smart_ptr>>()) {
+    auto p2 = p;
+  }
+}
+
+void negativeSmartReference() {
+  for (auto R : View<Iterator<SmartReference>>()) {
+    auto R2 = R;
+  }
+}
+
+void negative_smart_reference() {
+  for (auto r : View<Iterator<smart_reference>>()) {
+    auto r2 = r;
+  }
+}
+
+void negativeSmartRef() {
+  for (auto R : View<Iterator<SmartRef>>()) {
+    auto R2 = R;
+  }
+}
+
+void negative_smart_ref() {
+  for (auto r : View<Iterator<smart_ref>>()) {
+    auto r2 = r;
+  }
+}
Index: docs/clang-tidy/checks/performance-unnecessary-value-param.rst
===================================================================
--- docs/clang-tidy/checks/performance-unnecessary-value-param.rst
+++ docs/clang-tidy/checks/performance-unnecessary-value-param.rst
@@ -61,3 +61,8 @@
 
    A string specifying which include-style is used, `llvm` or `google`. Default
    is `llvm`.
+
+.. option:: WhiteListTypes
+
+   A semicolon-separated list of names of whitelist types. Regular expressions
+   are allowed. Default is empty.
Index: docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
===================================================================
--- docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
+++ docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
@@ -35,3 +35,11 @@
     string UnnecessaryCopy2 = UnnecessaryCopy1;
     UnnecessaryCopy2.find("bar");
   }
+
+Options
+-------
+
+.. option:: WhiteListTypes
+
+   A semicolon-separated list of names of whitelist types. Regular expressions
+   are allowed. Default is empty.
Index: docs/clang-tidy/checks/performance-for-range-copy.rst
===================================================================
--- docs/clang-tidy/checks/performance-for-range-copy.rst
+++ docs/clang-tidy/checks/performance-for-range-copy.rst
@@ -25,3 +25,9 @@
 
    When non-zero, warns on any use of `auto` as the type of the range-based for
    loop variable. Default is `0`.
+
+.. option:: WhiteListTypes
+
+   A semicolon-separated list of names of whitelist types. Regular expressions
+   are allowed. Default is empty.
+
Index: clang-tidy/performance/UnnecessaryValueParamCheck.h
===================================================================
--- clang-tidy/performance/UnnecessaryValueParamCheck.h
+++ clang-tidy/performance/UnnecessaryValueParamCheck.h
@@ -40,6 +40,7 @@
       MutationAnalyzers;
   std::unique_ptr<utils::IncludeInserter> Inserter;
   const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  const std::vector<std::string> WhiteListTypes;
 };
 
 } // namespace performance
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -12,6 +12,7 @@
 #include "../utils/DeclRefExprUtils.h"
 #include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include "../utils/TypeTraits.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Lexer.h"
@@ -68,7 +69,8 @@
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
-          Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+                       Options.getLocalOrGlobal("IncludeStyle", "llvm"))),
+      WhiteListTypes(utils::options::parseStringList(Options.get("WhiteListTypes", ""))) {}
 
 void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
   // This check is specific to C++ and doesn't apply to languages like
@@ -97,8 +99,15 @@
   if (Analyzer.isMutated(Param))
     return;
 
-  const bool IsConstQualified =
-      Param->getType().getCanonicalType().isConstQualified();
+  // Skip whitelisted types
+  const auto ParamType = Param->getType();
+  if (std::find_if(WhiteListTypes.begin(), WhiteListTypes.end(),
+                   [&](llvm::StringRef WhiteListType) {
+                     return llvm::Regex(WhiteListType).match(ParamType.getCanonicalType().getAsString(Result.Context->getPrintingPolicy()));
+                   }) != WhiteListTypes.end())
+    return;
+
+  const bool IsConstQualified = ParamType.getCanonicalType().isConstQualified();
 
   // If the parameter is non-const, check if it has a move constructor and is
   // only referenced once to copy-construct another object or whether it has a
@@ -173,6 +182,7 @@
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IncludeStyle",
                 utils::IncludeSorter::toString(IncludeStyle));
+  Options.store(Opts, "WhiteListTypes", utils::options::serializeStringList(WhiteListTypes));
 }
 
 void UnnecessaryValueParamCheck::onEndOfTranslationUnit() {
Index: clang-tidy/performance/UnnecessaryCopyInitialization.h
===================================================================
--- clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -26,18 +26,19 @@
 // const references, and const pointers to const.
 class UnnecessaryCopyInitialization : public ClangTidyCheck {
 public:
-  UnnecessaryCopyInitialization(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  UnnecessaryCopyInitialization(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
 
 private:
   void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt,
                                   bool IssueFix, const VarDecl *ObjectArg,
                                   ASTContext &Context);
   void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar,
                               const Stmt &BlockStmt, bool IssueFix,
                               ASTContext &Context);
+  const std::vector<std::string> WhiteListTypes;
 };
 
 } // namespace performance
Index: clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -12,6 +12,7 @@
 #include "../utils/DeclRefExprUtils.h"
 #include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 
 namespace clang {
 namespace tidy {
@@ -30,6 +31,11 @@
 using namespace ::clang::ast_matchers;
 using utils::decl_ref_expr::isOnlyUsedAsConst;
 
+UnnecessaryCopyInitialization::UnnecessaryCopyInitialization(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      WhiteListTypes(utils::options::parseStringList(Options.get("WhiteListTypes", ""))) {}
+
 void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
   auto ConstReference = referenceType(pointee(qualType(isConstQualified())));
   auto ConstOrConstReference =
@@ -84,6 +90,15 @@
   const auto *ObjectArg = Result.Nodes.getNodeAs<VarDecl>("objectArg");
   const auto *BlockStmt = Result.Nodes.getNodeAs<Stmt>("blockStmt");
   const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall");
+
+  // Skip whitelisted types
+  const auto VarType = NewVar->getType();
+  if (std::find_if(WhiteListTypes.begin(), WhiteListTypes.end(),
+                   [&](llvm::StringRef WhiteListType) {
+                     return llvm::Regex(WhiteListType).match(VarType.getCanonicalType().getAsString(Result.Context->getPrintingPolicy()));
+                   }) != WhiteListTypes.end())
+    return;
+
   // Do not propose fixes if the DeclStmt has multiple VarDecls or in macros
   // since we cannot place them correctly.
   bool IssueFix =
@@ -144,6 +159,11 @@
     recordFixes(NewVar, Context, Diagnostic);
 }
 
+void UnnecessaryCopyInitialization::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "WhiteListTypes", utils::options::serializeStringList(WhiteListTypes));
+}
+
 } // namespace performance
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/performance/ForRangeCopyCheck.h
===================================================================
--- clang-tidy/performance/ForRangeCopyCheck.h
+++ clang-tidy/performance/ForRangeCopyCheck.h
@@ -40,6 +40,7 @@
                                        ASTContext &Context);
 
   const bool WarnOnAllAutoCopies;
+  const std::vector<std::string> WhiteListTypes;
 };
 
 } // namespace performance
Index: clang-tidy/performance/ForRangeCopyCheck.cpp
===================================================================
--- clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -10,6 +10,7 @@
 #include "ForRangeCopyCheck.h"
 #include "../utils/DeclRefExprUtils.h"
 #include "../utils/FixItHintUtils.h"
+#include "../utils/OptionsUtils.h"
 #include "../utils/TypeTraits.h"
 #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 
@@ -21,10 +22,12 @@
 
 ForRangeCopyCheck::ForRangeCopyCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      WarnOnAllAutoCopies(Options.get("WarnOnAllAutoCopies", 0)) {}
+      WarnOnAllAutoCopies(Options.get("WarnOnAllAutoCopies", 0)),
+      WhiteListTypes(utils::options::parseStringList(Options.get("WhiteListTypes", ""))) {}
 
 void ForRangeCopyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "WarnOnAllAutoCopies", WarnOnAllAutoCopies);
+  Options.store(Opts, "WhiteListTypes", utils::options::serializeStringList(WhiteListTypes));
 }
 
 void ForRangeCopyCheck::registerMatchers(MatchFinder *Finder) {
@@ -41,6 +44,15 @@
 
 void ForRangeCopyCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Var = Result.Nodes.getNodeAs<VarDecl>("loopVar");
+
+  // Skip whitelisted types
+  const auto VarType = Var->getType();
+  if (std::find_if(WhiteListTypes.begin(), WhiteListTypes.end(),
+                   [&](llvm::StringRef WhiteListType) {
+                     return llvm::Regex(WhiteListType).match(VarType.getCanonicalType().getAsString(Result.Context->getPrintingPolicy()));
+                   }) != WhiteListTypes.end())
+    return;
+
   // Ignore code in macros since we can't place the fixes correctly.
   if (Var->getBeginLoc().isMacroID())
     return;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to