[clang-tidy] Move checks from misc- to performance-

Summary:
rename_check.py misc-move-constructor-init performance-move-constructor-init
rename_check.py misc-inefficient-algorithm performance-inefficient-algorithm

Reviewers: hokein, aaron.ballman

Reviewed By: hokein, aaron.ballman

Subscribers: aaron.ballman, mgorny, xazax.hun, cfe-commits

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

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@319023 91177308-0d34-0410-b5e6-96231b3b80d8

6 files changed, 361 insertions, 0 deletions

diff --git a/clang-tidy/performance/CMakeLists.txt b/clang-tidy/performance/CMakeLists.txt
index 4b5541fd..67594332 100644
--- a/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tidy/performance/CMakeLists.txt
@@ -4,8 +4,10 @@ add_clang_library(clangTidyPerformanceModule
   FasterStringFindCheck.cpp
   ForRangeCopyCheck.cpp
   ImplicitConversionInLoopCheck.cpp
+  InefficientAlgorithmCheck.cpp
   InefficientStringConcatenationCheck.cpp
   InefficientVectorOperationCheck.cpp
+  MoveConstructorInitCheck.cpp
   PerformanceTidyModule.cpp
   TypePromotionInMathFnCheck.cpp
   UnnecessaryCopyInitialization.cpp
diff --git a/clang-tidy/performance/InefficientAlgorithmCheck.cpp b/clang-tidy/performance/InefficientAlgorithmCheck.cpp
new file mode 100644
index 00000000..4471d077
--- /dev/null
+++ b/clang-tidy/performance/InefficientAlgorithmCheck.cpp
@@ -0,0 +1,163 @@
+//===--- InefficientAlgorithmCheck.cpp - clang-tidy------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "InefficientAlgorithmCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+static bool areTypesCompatible(QualType Left, QualType Right) {
+  if (const auto *LeftRefType = Left->getAs<ReferenceType>())
+    Left = LeftRefType->getPointeeType();
+  if (const auto *RightRefType = Right->getAs<ReferenceType>())
+    Right = RightRefType->getPointeeType();
+  return Left->getCanonicalTypeUnqualified() ==
+         Right->getCanonicalTypeUnqualified();
+}
+
+void InefficientAlgorithmCheck::registerMatchers(MatchFinder *Finder) {
+  // Only register the matchers for C++; the functionality currently does not
+  // provide any benefit to other languages, despite being benign.
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  const auto Algorithms =
+      hasAnyName("::std::find", "::std::count", "::std::equal_range",
+                 "::std::lower_bound", "::std::upper_bound");
+  const auto ContainerMatcher = classTemplateSpecializationDecl(hasAnyName(
+      "::std::set", "::std::map", "::std::multiset", "::std::multimap",
+      "::std::unordered_set", "::std::unordered_map",
+      "::std::unordered_multiset", "::std::unordered_multimap"));
+
+  const auto Matcher =
+      callExpr(
+          callee(functionDecl(Algorithms)),
+          hasArgument(
+              0, cxxConstructExpr(has(ignoringParenImpCasts(cxxMemberCallExpr(
+                     callee(cxxMethodDecl(hasName("begin"))),
+                     on(declRefExpr(
+                            hasDeclaration(decl().bind("IneffContObj")),
+                            anyOf(hasType(ContainerMatcher.bind("IneffCont")),
+                                  hasType(pointsTo(
+                                      ContainerMatcher.bind("IneffContPtr")))))
+                            .bind("IneffContExpr"))))))),
+          hasArgument(
+              1, cxxConstructExpr(has(ignoringParenImpCasts(cxxMemberCallExpr(
+                     callee(cxxMethodDecl(hasName("end"))),
+                     on(declRefExpr(
+                         hasDeclaration(equalsBoundNode("IneffContObj"))))))))),
+          hasArgument(2, expr().bind("AlgParam")),
+          unless(isInTemplateInstantiation()))
+          .bind("IneffAlg");
+
+  Finder->addMatcher(Matcher, this);
+}
+
+void InefficientAlgorithmCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *AlgCall = Result.Nodes.getNodeAs<CallExpr>("IneffAlg");
+  const auto *IneffCont =
+      Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("IneffCont");
+  bool PtrToContainer = false;
+  if (!IneffCont) {
+    IneffCont =
+        Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("IneffContPtr");
+    PtrToContainer = true;
+  }
+  const llvm::StringRef IneffContName = IneffCont->getName();
+  const bool Unordered =
+      IneffContName.find("unordered") != llvm::StringRef::npos;
+  const bool Maplike = IneffContName.find("map") != llvm::StringRef::npos;
+
+  // Store if the key type of the container is compatible with the value
+  // that is searched for.
+  QualType ValueType = AlgCall->getArg(2)->getType();
+  QualType KeyType =
+      IneffCont->getTemplateArgs()[0].getAsType().getCanonicalType();
+  const bool CompatibleTypes = areTypesCompatible(KeyType, ValueType);
+
+  // Check if the comparison type for the algorithm and the container matches.
+  if (AlgCall->getNumArgs() == 4 && !Unordered) {
+    const Expr *Arg = AlgCall->getArg(3);
+    const QualType AlgCmp =
+        Arg->getType().getUnqualifiedType().getCanonicalType();
+    const unsigned CmpPosition =
+        (IneffContName.find("map") == llvm::StringRef::npos) ? 1 : 2;
+    const QualType ContainerCmp = IneffCont->getTemplateArgs()[CmpPosition]
+                                      .getAsType()
+                                      .getUnqualifiedType()
+                                      .getCanonicalType();
+    if (AlgCmp != ContainerCmp) {
+      diag(Arg->getLocStart(),
+           "different comparers used in the algorithm and the container");
+      return;
+    }
+  }
+
+  const auto *AlgDecl = AlgCall->getDirectCallee();
+  if (!AlgDecl)
+    return;
+
+  if (Unordered && AlgDecl->getName().find("bound") != llvm::StringRef::npos)
+    return;
+
+  const auto *AlgParam = Result.Nodes.getNodeAs<Expr>("AlgParam");
+  const auto *IneffContExpr = Result.Nodes.getNodeAs<Expr>("IneffContExpr");
+  FixItHint Hint;
+
+  SourceManager &SM = *Result.SourceManager;
+  LangOptions LangOpts = getLangOpts();
+
+  CharSourceRange CallRange =
+      CharSourceRange::getTokenRange(AlgCall->getSourceRange());
+
+  // FIXME: Create a common utility to extract a file range that the given token
+  // sequence is exactly spelled at (without macro argument expansions etc.).
+  // We can't use Lexer::makeFileCharRange here, because for
+  //
+  //   #define F(x) x
+  //   x(a b c);
+  //
+  // it will return "x(a b c)", when given the range "a"-"c". It makes sense for
+  // removals, but not for replacements.
+  //
+  // This code is over-simplified, but works for many real cases.
+  if (SM.isMacroArgExpansion(CallRange.getBegin()) &&
+      SM.isMacroArgExpansion(CallRange.getEnd())) {
+    CallRange.setBegin(SM.getSpellingLoc(CallRange.getBegin()));
+    CallRange.setEnd(SM.getSpellingLoc(CallRange.getEnd()));
+  }
+
+  if (!CallRange.getBegin().isMacroID() && !Maplike && CompatibleTypes) {
+    StringRef ContainerText = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(IneffContExpr->getSourceRange()), SM,
+        LangOpts);
+    StringRef ParamText = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(AlgParam->getSourceRange()), SM,
+        LangOpts);
+    std::string ReplacementText =
+        (llvm::Twine(ContainerText) + (PtrToContainer ? "->" : ".") +
+         AlgDecl->getName() + "(" + ParamText + ")")
+            .str();
+    Hint = FixItHint::CreateReplacement(CallRange, ReplacementText);
+  }
+
+  diag(AlgCall->getLocStart(),
+       "this STL algorithm call should be replaced with a container method")
+      << Hint;
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tidy/performance/InefficientAlgorithmCheck.h b/clang-tidy/performance/InefficientAlgorithmCheck.h
new file mode 100644
index 00000000..72506cf7
--- /dev/null
+++ b/clang-tidy/performance/InefficientAlgorithmCheck.h
@@ -0,0 +1,36 @@
+//===--- InefficientAlgorithmCheck.h - clang-tidy----------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTALGORITHMCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTALGORITHMCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// Warns on inefficient use of STL algorithms on associative containers.
+///
+/// Associative containers implements some of the algorithms as methods which
+/// should be preferred to the algorithms in the algorithm header. The methods
+/// can take advanatage of the order of the elements.
+class InefficientAlgorithmCheck : public ClangTidyCheck {
+public:
+  InefficientAlgorithmCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTALGORITHMCHECK_H
diff --git a/clang-tidy/performance/MoveConstructorInitCheck.cpp b/clang-tidy/performance/MoveConstructorInitCheck.cpp
new file mode 100644
index 00000000..055e4f0a
--- /dev/null
+++ b/clang-tidy/performance/MoveConstructorInitCheck.cpp
@@ -0,0 +1,110 @@
+//===--- MoveConstructorInitCheck.cpp - clang-tidy-------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MoveConstructorInitCheck.h"
+#include "../utils/Matchers.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+MoveConstructorInitCheck::MoveConstructorInitCheck(StringRef Name,
+                                                   ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+          Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+
+void MoveConstructorInitCheck::registerMatchers(MatchFinder *Finder) {
+  // Only register the matchers for C++11; the functionality currently does not
+  // provide any benefit to other languages, despite being benign.
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  Finder->addMatcher(
+      cxxConstructorDecl(
+          unless(isImplicit()),
+          allOf(isMoveConstructor(),
+                hasAnyConstructorInitializer(
+                    cxxCtorInitializer(
+                        withInitializer(cxxConstructExpr(hasDeclaration(
+                            cxxConstructorDecl(isCopyConstructor())
+                                .bind("ctor")))))
+                        .bind("move-init")))),
+      this);
+}
+
+void MoveConstructorInitCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *CopyCtor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
+  const auto *Initializer =
+      Result.Nodes.getNodeAs<CXXCtorInitializer>("move-init");
+
+  // Do not diagnose if the expression used to perform the initialization is a
+  // trivially-copyable type.
+  QualType QT = Initializer->getInit()->getType();
+  if (QT.isTriviallyCopyableType(*Result.Context))
+    return;
+
+  if (QT.isConstQualified())
+    return;
+
+  const auto *RD = QT->getAsCXXRecordDecl();
+  if (RD && RD->isTriviallyCopyable())
+    return;
+
+  // Diagnose when the class type has a move constructor available, but the
+  // ctor-initializer uses the copy constructor instead.
+  const CXXConstructorDecl *Candidate = nullptr;
+  for (const auto *Ctor : CopyCtor->getParent()->ctors()) {
+    if (Ctor->isMoveConstructor() && Ctor->getAccess() <= AS_protected &&
+        !Ctor->isDeleted()) {
+      // The type has a move constructor that is at least accessible to the
+      // initializer.
+      //
+      // FIXME: Determine whether the move constructor is a viable candidate
+      // for the ctor-initializer, perhaps provide a fixit that suggests
+      // using std::move().
+      Candidate = Ctor;
+      break;
+    }
+  }
+
+  if (Candidate) {
+    // There's a move constructor candidate that the caller probably intended
+    // to call instead.
+    diag(Initializer->getSourceLocation(),
+         "move constructor initializes %0 by calling a copy constructor")
+        << (Initializer->isBaseInitializer() ? "base class" : "class member");
+    diag(CopyCtor->getLocation(), "copy constructor being called",
+         DiagnosticIDs::Note);
+    diag(Candidate->getLocation(), "candidate move constructor here",
+         DiagnosticIDs::Note);
+  }
+}
+
+void MoveConstructorInitCheck::registerPPCallbacks(CompilerInstance &Compiler) {
+  Inserter.reset(new utils::IncludeInserter(
+      Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle));
+  Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void MoveConstructorInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IncludeStyle",
+                utils::IncludeSorter::toString(IncludeStyle));
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tidy/performance/MoveConstructorInitCheck.h b/clang-tidy/performance/MoveConstructorInitCheck.h
new file mode 100644
index 00000000..3b7dda06
--- /dev/null
+++ b/clang-tidy/performance/MoveConstructorInitCheck.h
@@ -0,0 +1,44 @@
+//===--- MoveConstructorInitCheck.h - clang-tidy-----------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_MOVECONSTRUCTORINITCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_MOVECONSTRUCTORINITCHECK_H
+
+#include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
+
+#include <memory>
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// The check flags user-defined move constructors that have a ctor-initializer
+/// initializing a member or base class through a copy constructor instead of a
+/// move constructor.
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-move-constructor-init.html
+class MoveConstructorInitCheck : public ClangTidyCheck {
+public:
+  MoveConstructorInitCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void registerPPCallbacks(clang::CompilerInstance &Compiler) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  std::unique_ptr<utils::IncludeInserter> Inserter;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_MOVECONSTRUCTORINITCHECK_H
diff --git a/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tidy/performance/PerformanceTidyModule.cpp
index b4652509..51a8728b 100644
--- a/clang-tidy/performance/PerformanceTidyModule.cpp
+++ b/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -13,8 +13,10 @@
 #include "FasterStringFindCheck.h"
 #include "ForRangeCopyCheck.h"
 #include "ImplicitConversionInLoopCheck.h"
+#include "InefficientAlgorithmCheck.h"
 #include "InefficientStringConcatenationCheck.h"
 #include "InefficientVectorOperationCheck.h"
+#include "MoveConstructorInitCheck.h"
 #include "TypePromotionInMathFnCheck.h"
 #include "UnnecessaryCopyInitialization.h"
 #include "UnnecessaryValueParamCheck.h"
@@ -32,10 +34,14 @@ public:
         "performance-for-range-copy");
     CheckFactories.registerCheck<ImplicitConversionInLoopCheck>(
         "performance-implicit-conversion-in-loop");
+    CheckFactories.registerCheck<InefficientAlgorithmCheck>(
+        "performance-inefficient-algorithm");
     CheckFactories.registerCheck<InefficientStringConcatenationCheck>(
         "performance-inefficient-string-concatenation");
     CheckFactories.registerCheck<InefficientVectorOperationCheck>(
         "performance-inefficient-vector-operation");
+    CheckFactories.registerCheck<MoveConstructorInitCheck>(
+        "performance-move-constructor-init");
     CheckFactories.registerCheck<TypePromotionInMathFnCheck>(
         "performance-type-promotion-in-math-fn");
     CheckFactories.registerCheck<UnnecessaryCopyInitialization>(