Index: clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.h +++ clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.h @@ -0,0 +1,36 @@ +//===--- AvoidBindCheck.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_MODERNIZE_AVOID_BIND_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOID_BIND_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace modernize { + +/// 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/modernize-avoid-std-bind.html +class AvoidBindCheck : public ClangTidyCheck { +public: + AvoidBindCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; +} // namespace modernize +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOID_BIND_H Index: clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/modernize/AvoidBindCheck.cpp @@ -0,0 +1,163 @@ +//===--- AvoidBindCheck.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 "AvoidBindCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace modernize { + +namespace { +enum BindArgumentKind { BK_Temporary, BK_Placeholder, BK_CallExpr, BK_Other }; + +struct BindArgument { + StringRef Tokens; + BindArgumentKind Kind = BK_Other; + size_t PlaceHolderIndex = 0; +}; + +} // end namespace + +static SmallVector +buildBindArguments(const MatchFinder::MatchResult &Result, const CallExpr *C) { + SmallVector BindArguments; + llvm::Regex MatchPlaceholder("^_([0-9]+)$"); + + // Start at index 1 as first argument to bind is the function name. + for (size_t I = 1, ArgCount = C->getNumArgs(); I < ArgCount; ++I) { + const Expr *E = C->getArg(I); + BindArgument B; + if (const auto *M = dyn_cast(E)) { + const auto *TE = M->GetTemporaryExpr(); + B.Kind = isa(TE) ? BK_CallExpr : BK_Temporary; + } + + B.Tokens = Lexer::getSourceText( + CharSourceRange::getTokenRange(E->getLocStart(), E->getLocEnd()), + *Result.SourceManager, Result.Context->getLangOpts()); + + SmallVector 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; +} + +static void addPlaceholderArgs(const ArrayRef 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 << ")"; +} + +static void addFunctionCallArgs(const ArrayRef 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 = ", "; + } +} + +static bool isPlaceHolderIndexRepeated(const ArrayRef Args) { + llvm::SmallSet PlaceHolderIndices; + for (const BindArgument &B : Args) { + if (B.PlaceHolderIndex) { + if (!PlaceHolderIndices.insert(B.PlaceHolderIndex).second) + return true; + } + } + return false; +} + +void AvoidBindCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus14) // Need C++14 for generic lambdas. + return; + + Finder->addMatcher( + callExpr(callee(namedDecl(hasName("::std::bind"))), + hasArgument(0, declRefExpr(to(functionDecl().bind("f"))))) + .bind("bind"), + this); +} + +void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedDecl = Result.Nodes.getNodeAs("bind"); + auto Diag = diag(MatchedDecl->getLocStart(), "prefer a lambda to std::bind"); + + const auto 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 (llvm::any_of(Args, + [](const BindArgument &B) { return B.Kind == BK_CallExpr; })) + 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("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 = llvm::any_of( + Args, [](const BindArgument &B) { return B.Kind == BK_Other; }); + + Stream << "[" << (HasCapturedArgument ? "=" : "") << "]"; + addPlaceholderArgs(Args, Stream); + Stream << " { return " << F->getName() << "("; + addFunctionCallArgs(Args, Stream); + Stream << "); };"; + + Diag << FixItHint::CreateReplacement(MatchedDecl->getSourceRange(), Stream.str()); +} + +} // namespace modernize +} // namespace tidy +} // namespace clang Index: clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyModernizeModule + AvoidBindCheck.cpp DeprecatedHeadersCheck.cpp LoopConvertCheck.cpp LoopConvertUtils.cpp Index: clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "AvoidBindCheck.h" #include "DeprecatedHeadersCheck.h" #include "LoopConvertCheck.h" #include "MakeSharedCheck.h" @@ -34,6 +35,8 @@ class ModernizeModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "modernize-avoid-bind"); CheckFactories.registerCheck( "modernize-deprecated-headers"); CheckFactories.registerCheck("modernize-loop-convert"); Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -91,6 +91,7 @@ misc-unused-raii misc-unused-using-decls misc-virtual-near-miss + modernize-avoid-bind modernize-deprecated-headers modernize-loop-convert modernize-make-shared Index: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-bind.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-bind.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-avoid-bind.rst @@ -0,0 +1,38 @@ +.. title:: clang-tidy - modernize-avoid-std-bind + +modernize-avoid-bind +========================== + +The check finds uses of ``std::bind`` and replaces simple uses with lambdas. +Lambdas will use value-capture where required. + +Right now it only handles free functions, not member functions. + +Given: + +.. code:: C++ + + int add(int x, int y) { return x + y; } + +Then: + +.. code:: C++ + + void f() { + int x = 2; + auto clj = std::bind(add, x, _1); + } + +is replaced by: + +.. code:: C++ + + void f() { + int x = 2; + auto clj = [=](auto && arg1) { return add(x, arg1); }; + } + +``std::bind`` can be hard to read and can result in larger object files and +binaries due to type information that will not be produced by equivalent +lambdas. + Index: clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-bind.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-bind.cpp +++ clang-tools-extra/trunk/test/clang-tidy/modernize-avoid-bind.cpp @@ -0,0 +1,70 @@ +// RUN: %check_clang_tidy %s modernize-avoid-bind %t -- -- -std=c++14 + +namespace std { +inline namespace impl { +template +class bind_rt {}; + +template +bind_rt 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: prefer a lambda to std::bind [modernize-avoid-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: prefer a lambda to 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: prefer a lambda to 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: prefer a lambda to 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: prefer a lambda to 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: prefer a lambda to 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: prefer a lambda to std::bind + // No fix is applied for nested calls. + // CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5)); +} +