Index: clang-tidy/readability/AvoidStdBindCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/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_READABILITY_AVOID_STD_BIND_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_STD_BIND_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// 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/readability-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 readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOID_STD_BIND_H Index: clang-tidy/readability/AvoidStdBindCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/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 +#include "AvoidStdBindCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +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 +buildBindArguments(const MatchFinder::MatchResult &Result, const CallExpr *C) { + std::vector 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(E)) { + auto TE = M->GetTemporaryExpr(); + if (dyn_cast(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 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 &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 &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 &Args) { + std::unordered_map 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("bind"); + auto DiagnosticBuilder = + diag(MatchedDecl->getLocStart(), "avoid using std::bind"); + + if (!getLangOpts().CPlusPlus14) // Need C++14 for generic lambdas + return; + + const std::vector 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("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 readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyReadabilityModule + AvoidStdBindCheck.cpp BracesAroundStatementsCheck.cpp ContainerSizeEmptyCheck.cpp ElseAfterReturnCheck.cpp Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "AvoidStdBindCheck.h" #include "BracesAroundStatementsCheck.h" #include "ContainerSizeEmptyCheck.h" #include "ElseAfterReturnCheck.h" @@ -31,6 +32,8 @@ class ReadabilityModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "readability-avoid-std-bind"); CheckFactories.registerCheck( "readability-braces-around-statements"); CheckFactories.registerCheck( Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -79,6 +79,7 @@ modernize-use-override performance-for-range-copy performance-implicit-cast-in-loop + readability-avoid-std-bind readability-braces-around-statements readability-container-size-empty readability-else-after-return Index: docs/clang-tidy/checks/readability-avoid-std-bind.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-avoid-std-bind.rst @@ -0,0 +1,16 @@ +.. title:: clang-tidy - readability-avoid-std-bind + +readability-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 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: test/clang-tidy/readability-avoid-std-bind.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-avoid-std-bind.cpp @@ -0,0 +1,81 @@ +// RUN: %check_clang_tidy %s readability-avoid-std-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: avoid using std::bind [readability-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 [readability-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 [readability-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 [readability-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 [readability-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 [readability-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 [readability-avoid-std-bind] +} +// No fix is applied for nested calls. +// CHECK-FIXES: auto clj = std::bind(add, 1, add(2, 5)); +