jbcoe removed rL LLVM as the repository for this revision.
jbcoe updated this revision to Diff 47247.
jbcoe added a comment.

Require C++14 for improved fixits.

Do not attempt to generate fixits for more complicated uses of bind.


http://reviews.llvm.org/D16962

Files:
  clang-tidy/misc/AvoidStdBindCheck.cpp
  clang-tidy/misc/AvoidStdBindCheck.h
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-avoid-std-bind.rst
  test/clang-tidy/misc-avoid-std-bind.cpp

Index: test/clang-tidy/misc-avoid-std-bind.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-avoid-std-bind.cpp
@@ -0,0 +1,81 @@
+// RUN: %check_clang_tidy %s misc-avoid-std-bind %t -- -- -std=c++14
+
+namespace std {
+inline namespace impl {
+template <class Fp, class... Arguments>
+class bind_rt {};
+
+template <class Fp, class... Arguments>
+bind_rt<Fp, Arguments...> bind(Fp&&, Arguments&& ...);
+}
+}
+
+int add(int x, int y) { return x + y; }
+
+void f()
+{
+  auto clj = std::bind(add,2,2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [] { return add(2, 2); };
+
+void g()
+{
+  int x = 2;
+  int y = 2;
+  auto clj = std::bind(add,x,y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [=] { return add(x, y); };
+
+struct placeholder {};
+placeholder _1;
+placeholder _2;
+
+void h()
+{
+  int x = 2;
+  auto clj = std::bind(add,x,_1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto clj = [=](auto && arg1) { return add(x, arg1); };
+
+struct A;
+struct B;
+bool ABTest(const A&, const B&);
+
+void i()
+{
+  auto BATest = std::bind(ABTest, _2, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+
+// CHECK-FIXES: auto BATest = [](auto && arg1, auto && arg2) { return ABTest(arg2, arg1); };
+
+void j()
+{
+  auto clj = std::bind(add, 2, 2, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+// No fix is applied for argument mismatches.
+// CHECK-FIXES: auto clj = std::bind(add, 2, 2, 2);
+
+void k()
+{
+  auto clj = std::bind(add, _1, _1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+// No fix is applied for reused placeholders.
+// CHECK-FIXES: auto clj = std::bind(add, _1, _1);
+
+void m()
+{
+  auto clj = std::bind(add, 1, add(2, 5));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using std::bind [misc-avoid-std-bind]
+}
+// No fix is applied for nested calls.
+// CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5));
+
Index: docs/clang-tidy/checks/misc-avoid-std-bind.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-avoid-std-bind.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - misc-avoid-std-bind
+
+misc-avoid-std-bind
+===================
+
+Find uses of std::bind. Replace simple uses of std::bind with lambdas. Lambdas
+will use value-capture where required.
+
+Right now it only handles free functions not member functions.
+
+Fixits are only generated for simple uses of std::bind.
+
+std::bind can result in larger object files and binaries due to type
+information that will not be produced by equivalent lambdas.
+
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -46,6 +46,7 @@
    misc-argument-comment
    misc-assert-side-effect
    misc-assign-operator-signature
+   misc-avoid-std-bind
    misc-bool-pointer-implicit-conversion
    misc-definitions-in-headers
    misc-inaccurate-erase
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -13,6 +13,7 @@
 #include "ArgumentCommentCheck.h"
 #include "AssertSideEffectCheck.h"
 #include "AssignOperatorSignatureCheck.h"
+#include "AvoidStdBindCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "InaccurateEraseCheck.h"
@@ -48,6 +49,8 @@
         "misc-assert-side-effect");
     CheckFactories.registerCheck<AssignOperatorSignatureCheck>(
         "misc-assign-operator-signature");
+    CheckFactories.registerCheck<AvoidStdBindCheck>(
+        "misc-avoid-std-bind");
     CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
         "misc-bool-pointer-implicit-conversion");
     CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -4,6 +4,7 @@
   ArgumentCommentCheck.cpp
   AssertSideEffectCheck.cpp
   AssignOperatorSignatureCheck.cpp
+  AvoidStdBindCheck.cpp
   BoolPointerImplicitConversionCheck.cpp
   DefinitionsInHeadersCheck.cpp
   InaccurateEraseCheck.cpp
Index: clang-tidy/misc/AvoidStdBindCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/AvoidStdBindCheck.h
@@ -0,0 +1,37 @@
+//===--- AvoidStdBindCheck.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_MISC_AVOID_STD_BIND_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_AVOID_STD_BIND_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Replace simple uses of std::bind with a lambda.
+///
+/// FIXME: Add support for function references and member function references.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-avoid-std-bind.html
+class AvoidStdBindCheck : public ClangTidyCheck {
+public:
+  AvoidStdBindCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_AVOID_STD_BIND_H
Index: clang-tidy/misc/AvoidStdBindCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/AvoidStdBindCheck.cpp
@@ -0,0 +1,184 @@
+//===--- AvoidStdBindCheck.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 <cassert>
+#include "AvoidStdBindCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include <unordered_map>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+AST_MATCHER(NamedDecl, isStdBind) {
+  return Node.isInStdNamespace() && (Node.getName() == "bind");
+}
+
+enum BindArgumentKind { BK_Temporary, BK_Placeholder, BK_CallExpr, BK_Other };
+
+struct BindArgument {
+  StringRef Tokens;
+  BindArgumentKind Kind = BK_Other;
+  size_t PlaceHolderIndex = 0;
+};
+
+} // end namespace
+
+std::vector<BindArgument>
+buildBindArguments(const MatchFinder::MatchResult &Result, const CallExpr *C) {
+  std::vector<BindArgument> BindArguments;
+  llvm::Regex MatchPlaceholder("^_([0-9]+)$");
+
+  for (size_t I = 1, ArgCount = C->getNumArgs(); I < ArgCount; ++I) {
+    const Expr *E = C->getArg(I);
+    BindArgument B;
+    if (auto M = dyn_cast<MaterializeTemporaryExpr>(E)) {
+      auto TE = M->GetTemporaryExpr();
+      if (dyn_cast<CallExpr>(TE))
+        B.Kind = BK_CallExpr;
+      else
+        B.Kind = BK_Temporary;
+    }
+
+    B.Tokens = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(E->getLocStart(), E->getLocEnd()),
+        *Result.SourceManager, Result.Context->getLangOpts());
+
+    SmallVector<StringRef, 2> Matches;
+    if (B.Kind == BK_Other && MatchPlaceholder.match(B.Tokens, &Matches)) {
+      B.Kind = BK_Placeholder;
+      B.PlaceHolderIndex = std::stoi(Matches[1]);
+    }
+    BindArguments.push_back(B);
+  }
+  return BindArguments;
+}
+
+void addPlaceholderArgs(const std::vector<BindArgument> &Args,
+                        llvm::raw_ostream &Stream) {
+  auto MaxPlaceholderIt =
+      std::max_element(Args.begin(), Args.end(),
+                       [](const BindArgument &B1, const BindArgument &B2) {
+                         return B1.PlaceHolderIndex < B2.PlaceHolderIndex;
+                       });
+
+  // Placeholders (if present) have index 1 or greater.
+  if (MaxPlaceholderIt == Args.end() || MaxPlaceholderIt->PlaceHolderIndex == 0)
+    return;
+
+  size_t PlaceholderCount = MaxPlaceholderIt->PlaceHolderIndex;
+  Stream << "(";
+  StringRef Delimiter = "";
+  for (size_t I = 1; I <= PlaceholderCount; ++I) {
+    Stream << Delimiter << "auto &&"
+           << " arg" << I;
+    Delimiter = ", ";
+  }
+  Stream << ")";
+}
+
+void addFunctionCallArgs(const std::vector<BindArgument> &Args,
+                         llvm::raw_ostream &Stream) {
+  StringRef Delimiter = "";
+  for (const auto &B : Args) {
+    if (B.PlaceHolderIndex)
+      Stream << Delimiter << "arg" << B.PlaceHolderIndex;
+    else
+      Stream << Delimiter << B.Tokens;
+    Delimiter = ", ";
+  }
+  Stream << "); };";
+}
+
+bool isPlaceHolderIndexRepeated(const std::vector<BindArgument> &Args) {
+  std::unordered_map<size_t, size_t> PlaceHolderIndexCounts;
+  for (const BindArgument &B : Args) {
+    if (B.PlaceHolderIndex) {
+      size_t &Count = PlaceHolderIndexCounts[B.PlaceHolderIndex];
+      if (Count)
+        return true;
+      ++Count;
+    }
+  }
+  return false;
+}
+
+void AvoidStdBindCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(callee(namedDecl(isStdBind())),
+               hasArgument(0, declRefExpr(to(functionDecl().bind("f")))))
+          .bind("bind"),
+      this);
+}
+
+void AvoidStdBindCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<CallExpr>("bind");
+  auto DiagnosticBuilder =
+      diag(MatchedDecl->getLocStart(), "avoid using std::bind");
+
+  if (!getLangOpts().CPlusPlus14) // Need C++14 for generic lambdas
+    return;
+
+  const std::vector<BindArgument> Args =
+      buildBindArguments(Result, MatchedDecl);
+
+  // Do not attempt to create fixits for nested call expressions.
+  // FIXME: Create lambda capture variables to capture output of calls.
+  // NOTE: Supporting nested std::bind will be more difficult due to placeholder
+  // sharing between outer and inner std:bind invocations.
+  if (std::find_if(Args.begin(), Args.end(), [](const BindArgument &B) {
+        return B.Kind == BK_CallExpr;
+      }) != Args.end())
+    return;
+
+  // Do not attempt to create fixits when placeholders are reused.
+  // Unused placeholders are supported by requiring C++14 generic lambdas.
+  // FIXME: Support this case by deducing the common type.
+  if (isPlaceHolderIndexRepeated(Args))
+    return;
+
+  const auto *F = Result.Nodes.getNodeAs<FunctionDecl>("f");
+
+  // std::bind can support argument count mismatch between its arguments and the
+  // bound function's arguments. Do not attempt to generate a fixit for such
+  // cases.
+  // FIXME: Support this case by creating unused lambda capture variables.
+  if (F->getNumParams() != Args.size())
+    return;
+
+  std::string Buffer;
+  llvm::raw_string_ostream Stream(Buffer);
+  
+  bool HasCapturedArgument =
+      std::find_if(Args.begin(), Args.end(), [](const BindArgument &B) {
+        return B.Kind == BK_Other;
+      }) != Args.end();
+
+  Stream << "[" << (HasCapturedArgument ? "=" : "") << "]";
+  addPlaceholderArgs(Args, Stream);
+  Stream << " { return " << F->getName() << "(";
+  addFunctionCallArgs(Args, Stream);
+
+  SourceRange ReplacedRange(
+      MatchedDecl->getLocStart(),
+      Lexer::getLocForEndOfToken(MatchedDecl->getLocEnd(), 0,
+                                 *Result.SourceManager,
+                                 Result.Context->getLangOpts()));
+
+  DiagnosticBuilder << FixItHint::CreateReplacement(ReplacedRange,
+                                                    Stream.str());
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to