Index: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h =================================================================== --- clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h +++ clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.h @@ -23,8 +23,7 @@ /// 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) {} + AvoidBindCheck(StringRef Name, ClangTidyContext *Context); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; }; Index: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp +++ clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp @@ -14,11 +14,12 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Lex/Lexer.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Regex.h" #include "llvm/Support/raw_ostream.h" #include @@ -33,46 +34,282 @@ namespace { -enum BindArgumentKind { BK_Temporary, BK_Placeholder, BK_CallExpr, BK_Other }; +enum BindArgumentKind { + BK_Temporary, + BK_Placeholder, + BK_CallExpr, + BK_ObjectPtr, + BK_Other +}; +enum CaptureMode { CM_None, CM_ByRef, CM_ByValue, CM_InitExpression }; + +enum CallableType { + CT_Other, // unknown + CT_Function, // global or static function + CT_MemberFunction, // member function with implicit this + CT_Object, // object with operator() +}; + +enum CallableMaterializationKind { + CMK_Other, // unknown + CMK_Function, // callable is the name of a member or non-member function. + CMK_VariableRef, // callable is a simple expression involving a global or + // local variable. + CMK_CallExpression, // callable is obtained as the result of a call expression +}; struct BindArgument { - StringRef Tokens; + // A rough classification of the type of expression this argument was. BindArgumentKind Kind = BK_Other; + + // If this argument required a capture, a value indicating how it was + // captured. + CaptureMode CaptureMode = CM_None; + + // The exact spelling of this argument in the source code. + StringRef SourceTokens; + + // The identifier of the variable within the capture list. This may be + // different from UsageIdentifier for example in the expression *d, where the + // variable is captured as d, but referred to as *d. + std::string CaptureIdentifier; + + // If this is a placeholder or capture init expression, contains the tokens + // used to refer to this parameter from within the body of the lambda. + std::string UsageIdentifier; + + // If Kind == BK_Placeholder, the index of the placeholder. size_t PlaceHolderIndex = 0; + + // True if the argument is used inside the lambda, false otherwise. + bool IsUsed = false; + + // The actual Expr object representing this expression. + const Expr *E = nullptr; +}; + +struct CallableInfo { + CallableType Type = CT_Other; + CallableMaterializationKind Materialization = CMK_Other; + CaptureMode CaptureMode = CM_None; + StringRef SourceTokens; + std::string CaptureIdentifier; + std::string UsageIdentifier; + StringRef CaptureInitializer; + const FunctionDecl *Decl = nullptr; +}; + +struct LambdaProperties { + CallableInfo Callable; + SmallVector BindArguments; + StringRef BindNamespace; + bool IsFixitSupported = false; }; } // end namespace +static const Expr *ignoreTemporariesAndPointers(const Expr *E) { + if (const auto *T = dyn_cast(E)) + return ignoreTemporariesAndPointers(T->getSubExpr()); + + const Expr *F = E->IgnoreImplicit(); + if (E != F) + return ignoreTemporariesAndPointers(F); + + return E; +} + +static StringRef getSourceTextForExpr(const MatchFinder::MatchResult &Result, + const Expr *E) { + return Lexer::getSourceText( + CharSourceRange::getTokenRange(E->getBeginLoc(), E->getEndLoc()), + *Result.SourceManager, Result.Context->getLangOpts()); +} + +static bool isDeclNamed(const Decl *D, llvm::StringRef Name) { + ast_matchers::internal::HasNameMatcher HNM{{Name}}; + + return HNM.matchesNode(*cast(D)); +} + +static bool isCallExprNamed(const Expr *E, llvm::StringRef Name) { + if (const auto *CE = dyn_cast(E->IgnoreImplicit())) + return isDeclNamed(CE->getCalleeDecl(), Name); + + return false; +} + +static void +initializeBindArgumentForCallExpr(const MatchFinder::MatchResult &Result, + BindArgument &B, const CallExpr *CE, + unsigned &CaptureIndex) { + // std::ref(x) means to capture x by reference. + if (isCallExprNamed(CE, "::boost::ref") || + isCallExprNamed(CE, "::std::ref")) { + B.Kind = BK_Other; + B.CaptureMode = CM_ByRef; + B.UsageIdentifier = getSourceTextForExpr(Result, CE->getArg(0)); + } else { + B.Kind = BK_CallExpr; + B.CaptureMode = CM_InitExpression; + B.UsageIdentifier = "capture" + llvm::utostr(CaptureIndex++); + } + B.CaptureIdentifier = B.UsageIdentifier; +} + +static bool anyDescendantIsLocal(const Stmt *Statement) { + if (const auto *DeclRef = llvm::dyn_cast(Statement)) { + const ValueDecl *Decl = DeclRef->getDecl(); + if (const auto *Var = llvm::dyn_cast_or_null(Decl)) { + if (Var->isLocalVarDeclOrParm()) + return true; + } + } else if (const auto *ThisExpr = llvm::dyn_cast(Statement)) + return true; + + return llvm::any_of(Statement->children(), anyDescendantIsLocal); +} + +static bool tryCaptureAsLocalVariable(const MatchFinder::MatchResult &Result, + BindArgument &B, const Expr *E) { + if (const auto *BTE = llvm::dyn_cast(E)) { + if (const auto *CE = llvm::dyn_cast(BTE->getSubExpr())) + return tryCaptureAsLocalVariable(Result, B, CE->getArg(0)); + return false; + } + + const auto *DRE = dyn_cast(E->IgnoreImplicit()); + if (!DRE) + return false; + + const auto *VD = dyn_cast(DRE->getDecl()); + if (!VD || !VD->isLocalVarDeclOrParm()) + return false; + + B.CaptureMode = CM_ByValue; + B.UsageIdentifier = getSourceTextForExpr(Result, E); + B.CaptureIdentifier = B.UsageIdentifier; + return true; +} + +static bool tryCaptureAsMemberVariable(const MatchFinder::MatchResult &Result, + BindArgument &B, const Expr *E) { + if (const auto *BTE = llvm::dyn_cast(E)) { + if (const auto *CE = llvm::dyn_cast(BTE->getSubExpr())) + return tryCaptureAsMemberVariable(Result, B, CE->getArg(0)); + return false; + } + + E = E->IgnoreImplicit(); + if (isa(E)) { + B.CaptureMode = CM_ByValue; + B.UsageIdentifier = getSourceTextForExpr(Result, E); + B.CaptureIdentifier = "this"; + return true; + } + + const auto *ME = dyn_cast(E); + if (!ME) + return false; + + if (!ME->isLValue() || !llvm::isa(ME->getMemberDecl())) + return false; + + B.CaptureMode = CM_ByValue; + B.UsageIdentifier = getSourceTextForExpr(Result, E); + B.CaptureIdentifier = "this"; + return true; +} + static SmallVector -buildBindArguments(const MatchFinder::MatchResult &Result, const CallExpr *C) { +buildBindArguments(const MatchFinder::MatchResult &Result, + const CallableInfo &Callable) { SmallVector BindArguments; llvm::Regex MatchPlaceholder("^_([0-9]+)$"); + const auto *BindCall = Result.Nodes.getNodeAs("bind"); + // 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; - } + unsigned CaptureIndex = 0; + for (size_t I = 1, ArgCount = BindCall->getNumArgs(); I < ArgCount; ++I) { + + bool IsObjectPtrArg = (I == 1 && Callable.Type == CT_MemberFunction); + + const Expr *E = BindCall->getArg(I); + BindArgument &B = BindArguments.emplace_back(); + + size_t ArgIndex = I - 1; + if (Callable.Type == CT_MemberFunction) + --ArgIndex; - B.Tokens = Lexer::getSourceText( - CharSourceRange::getTokenRange(E->getBeginLoc(), E->getEndLoc()), - *Result.SourceManager, Result.Context->getLangOpts()); + if (!Callable.Decl || ArgIndex < Callable.Decl->getNumParams() || + IsObjectPtrArg) + B.IsUsed = true; + + B.E = E; + B.SourceTokens = getSourceTextForExpr(Result, E); + + if (IsObjectPtrArg) { + B.Kind = BK_ObjectPtr; + + if (tryCaptureAsLocalVariable(Result, B, B.E) || + tryCaptureAsMemberVariable(Result, B, B.E)) + continue; + + B.CaptureMode = CM_InitExpression; + B.UsageIdentifier = "ObjectPtr"; + B.CaptureIdentifier = B.UsageIdentifier; + continue; + } SmallVector Matches; - if (B.Kind == BK_Other && MatchPlaceholder.match(B.Tokens, &Matches)) { + if (MatchPlaceholder.match(B.SourceTokens, &Matches)) { B.Kind = BK_Placeholder; B.PlaceHolderIndex = std::stoi(Matches[1]); + llvm::raw_string_ostream OS(B.UsageIdentifier); + OS << "PH" << B.PlaceHolderIndex; + OS.flush(); + B.CaptureIdentifier = B.UsageIdentifier; + continue; + } + + if (const auto *CE = dyn_cast(E->IgnoreImplicit())) { + initializeBindArgumentForCallExpr(Result, B, CE, CaptureIndex); + continue; + } + + if (tryCaptureAsLocalVariable(Result, B, B.E) || + tryCaptureAsMemberVariable(Result, B, B.E)) + continue; + + // If it's not something we recognize, capture it by init expression to be + // safe. + B.Kind = BK_Other; + if (anyDescendantIsLocal(B.E)) { + B.CaptureMode = CM_InitExpression; + B.CaptureIdentifier = "capture" + llvm::utostr(CaptureIndex++); + B.UsageIdentifier = B.CaptureIdentifier; } - BindArguments.push_back(B); } return BindArguments; } -static void addPlaceholderArgs(const ArrayRef Args, +static size_t findPositionOfPlaceholderUse(ArrayRef Args, + size_t PlaceholderIndex) { + for (size_t I = 0; I < Args.size(); ++I) + if (Args[I].PlaceHolderIndex == PlaceholderIndex) + return I; + + return 0; +} + +static void addPlaceholderArgs(const LambdaProperties &LP, llvm::raw_ostream &Stream) { + + ArrayRef Args = LP.BindArguments; + if (LP.Callable.Type == CT_MemberFunction) + Args = Args.drop_front(); + auto MaxPlaceholderIt = std::max_element(Args.begin(), Args.end(), [](const BindArgument &B1, const BindArgument &B2) { @@ -87,20 +324,31 @@ Stream << "("; StringRef Delimiter = ""; for (size_t I = 1; I <= PlaceholderCount; ++I) { - Stream << Delimiter << "auto && arg" << I; + Stream << Delimiter << "auto &&"; + + size_t ArgIndex = findPositionOfPlaceholderUse(Args, I); + + if (Args[ArgIndex].IsUsed) + Stream << " " << Args[ArgIndex].CaptureIdentifier; Delimiter = ", "; } Stream << ")"; } -static void addFunctionCallArgs(const ArrayRef Args, +static void addFunctionCallArgs(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; + + for (int I = 0, Size = Args.size(); I < Size; ++I) { + const BindArgument &B = Args[I]; + + Stream << Delimiter; + + if (B.Kind == BK_Placeholder || B.CaptureMode != CM_None) + Stream << B.UsageIdentifier; + else if (B.CaptureMode == CM_None) + Stream << B.SourceTokens; + Delimiter = ", "; } } @@ -116,59 +364,304 @@ return false; } +static std::vector +findCandidateCallOperators(const CXXRecordDecl *RecordDecl, size_t NumArgs) { + std::vector Candidates; + + for (const clang::CXXMethodDecl *Method : RecordDecl->methods()) { + OverloadedOperatorKind OOK = Method->getOverloadedOperator(); + + if (OOK != OverloadedOperatorKind::OO_Call) + continue; + + if (Method->getNumParams() > NumArgs) + continue; + + Candidates.push_back(Method); + } + + return Candidates; +} + +static bool isFixitSupported(const CallableInfo &Callee, + ArrayRef Args) { + // Do not attempt to create fixits for nested std::bind or std::ref. + // Supporting nested std::bind will be more difficult due to placeholder + // sharing between outer and inner std::bind invocations, and std::ref + // requires us to capture some parameters by reference instead of by value. + if (llvm::any_of(Args, [](const BindArgument &B) { + return isCallExprNamed(B.E, "::boost::bind") || + isCallExprNamed(B.E, "::std::bind"); + })) { + return false; + } + + // 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 false; + + if (Callee.Type == CT_Other || Callee.Materialization == CMK_Other) + return false; + + return true; +} + +const FunctionDecl *getCallOperator(const CXXRecordDecl *Callable, + size_t NumArgs) { + std::vector Candidates = + findCandidateCallOperators(Callable, NumArgs); + if (Candidates.size() != 1) + return nullptr; + + return Candidates.front(); +} + +const FunctionDecl * +getCallMethodDecl(const MatchFinder::MatchResult &Result, CallableType Type, + CallableMaterializationKind Materialization) { + + const Expr *CallExpression = + ignoreTemporariesAndPointers(Result.Nodes.getNodeAs("ref")); + + if (Materialization == CMK_Function) { + if (const auto *DRE = dyn_cast(CallExpression)) { + if (const auto *FD = llvm::dyn_cast(DRE->getDecl())) + return FD; + } + return nullptr; + } + + if (Type == CT_Object) { + const auto *BindCall = Result.Nodes.getNodeAs("bind"); + size_t NumArgs = BindCall->getNumArgs() - 1; + return getCallOperator(BindCall->getType()->getAsCXXRecordDecl(), NumArgs); + } + + // Maybe this is an indirect call through a function pointer or something + // where we can't determine the exact decl. + return nullptr; +} + +static CallableType getCallableType(const MatchFinder::MatchResult &Result) { + const auto *CallableExpr = Result.Nodes.getNodeAs("ref"); + + QualType QT = CallableExpr->getType(); + if (QT->isMemberFunctionPointerType()) + return CT_MemberFunction; + + if (QT->isFunctionPointerType() || QT->isFunctionReferenceType() || + QT->isFunctionType()) + return CT_Function; + + if (QT->isRecordType()) { + const CXXRecordDecl *Decl = QT->getAsCXXRecordDecl(); + if (!Decl) + return CT_Other; + + return CT_Object; + } + + return CT_Other; +} + +static CallableMaterializationKind +getCallableMaterialization(const MatchFinder::MatchResult &Result) { + const auto *CallableExpr = Result.Nodes.getNodeAs("ref"); + + const auto *NoTemporaries = ignoreTemporariesAndPointers(CallableExpr); + + if (isa(NoTemporaries)) + return CMK_CallExpression; + + if (isa(NoTemporaries) || + isa(NoTemporaries)) + return CMK_Function; + + if (const auto *DRE = dyn_cast(NoTemporaries)) { + if (const auto *FD = dyn_cast(DRE->getDecl())) + return CMK_Function; + if (const auto *VD = dyn_cast(DRE->getDecl())) + return CMK_VariableRef; + } + + return CMK_Other; +} + +static LambdaProperties +getLambdaProperties(const MatchFinder::MatchResult &Result) { + const auto *CalleeExpr = Result.Nodes.getNodeAs("ref"); + + LambdaProperties LP; + + const auto *Bind = Result.Nodes.getNodeAs("bind"); + const auto *Decl = llvm::dyn_cast(Bind->getCalleeDecl()); + const auto *NS = + llvm::dyn_cast(Decl->getEnclosingNamespaceContext()); + while (NS->isInlineNamespace()) + NS = llvm::dyn_cast(NS->getDeclContext()); + LP.BindNamespace = NS->getName(); + + LP.Callable.Type = getCallableType(Result); + LP.Callable.Materialization = getCallableMaterialization(Result); + LP.Callable.Decl = + getCallMethodDecl(Result, LP.Callable.Type, LP.Callable.Materialization); + LP.Callable.SourceTokens = getSourceTextForExpr(Result, CalleeExpr); + if (LP.Callable.Materialization == CMK_VariableRef) { + LP.Callable.CaptureMode = CM_ByValue; + LP.Callable.UsageIdentifier = getSourceTextForExpr(Result, CalleeExpr); + LP.Callable.CaptureIdentifier = + getSourceTextForExpr(Result, ignoreTemporariesAndPointers(CalleeExpr)); + } else if (LP.Callable.Materialization == CMK_CallExpression) { + LP.Callable.CaptureMode = CM_InitExpression; + LP.Callable.UsageIdentifier = "Func"; + LP.Callable.CaptureIdentifier = "Func"; + LP.Callable.CaptureInitializer = getSourceTextForExpr(Result, CalleeExpr); + } + + LP.BindArguments = buildBindArguments(Result, LP.Callable); + + LP.IsFixitSupported = isFixitSupported(LP.Callable, LP.BindArguments); + + return LP; +} + +static bool emitCapture(llvm::StringSet<> &CaptureSet, StringRef Delimiter, + CaptureMode CM, StringRef Identifier, + StringRef InitExpression, raw_ostream &Stream) { + if (CM == CM_None) + return false; + + // This capture has already been emitted. + if (CaptureSet.count(Identifier) != 0) + return false; + + Stream << Delimiter; + + if (CM == CM_ByRef) + Stream << "&"; + Stream << Identifier; + if (CM == CM_InitExpression) + Stream << " = " << InitExpression; + + CaptureSet.insert(Identifier); + return true; +} + +static void emitCaptureList(const LambdaProperties &LP, + const MatchFinder::MatchResult &Result, + raw_ostream &Stream) { + llvm::StringSet<> CaptureSet; + bool AnyCapturesEmitted = false; + + AnyCapturesEmitted = emitCapture(CaptureSet, "", LP.Callable.CaptureMode, + LP.Callable.CaptureIdentifier, + LP.Callable.CaptureInitializer, Stream); + + for (const BindArgument &B : LP.BindArguments) { + if (B.CaptureMode == CM_None || !B.IsUsed) + continue; + + StringRef Delimiter = AnyCapturesEmitted ? ", " : ""; + + if (emitCapture(CaptureSet, Delimiter, B.CaptureMode, B.CaptureIdentifier, + B.SourceTokens, Stream)) + AnyCapturesEmitted = true; + } +} + +static ArrayRef +getForwardedArgumentList(const LambdaProperties &P) { + ArrayRef Args = llvm::makeArrayRef(P.BindArguments); + if (P.Callable.Type != CT_MemberFunction) + return Args; + + return Args.drop_front(); +} +AvoidBindCheck::AvoidBindCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + 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("ref"))) + callee(namedDecl( + anyOf(hasName("::boost::bind"), hasName("::std::bind")))), + hasArgument( + 0, anyOf(expr(hasType(memberPointerType())).bind("ref"), + expr(hasParent(materializeTemporaryExpr().bind("ref"))), + expr().bind("ref")))) .bind("bind"), this); } void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) { const auto *MatchedDecl = Result.Nodes.getNodeAs("bind"); - auto Diag = diag(MatchedDecl->getBeginLoc(), "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; })) + LambdaProperties LP = getLambdaProperties(Result); + auto Diag = diag( + MatchedDecl->getBeginLoc(), + llvm::formatv("prefer a lambda to {0}::bind", LP.BindNamespace).str()); + if (!LP.IsFixitSupported) 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; + const auto *Ref = Result.Nodes.getNodeAs("ref"); std::string Buffer; llvm::raw_string_ostream Stream(Buffer); - bool HasCapturedArgument = llvm::any_of( - Args, [](const BindArgument &B) { return B.Kind == BK_Other; }); - const auto *Ref = Result.Nodes.getNodeAs("ref"); - Stream << "[" << (HasCapturedArgument ? "=" : "") << "]"; - addPlaceholderArgs(Args, Stream); - Stream << " { return "; - Ref->printPretty(Stream, nullptr, Result.Context->getPrintingPolicy()); + Stream << "["; + emitCaptureList(LP, Result, Stream); + Stream << "]"; + + ArrayRef FunctionCallArgs = + llvm::makeArrayRef(LP.BindArguments); + + addPlaceholderArgs(LP, Stream); + + if (LP.Callable.Type == CT_Function) { + StringRef SourceTokens = LP.Callable.SourceTokens; + SourceTokens.consume_front("&"); + Stream << " { return " << SourceTokens; + } else if (LP.Callable.Type == CT_MemberFunction) { + const auto *MethodDecl = llvm::dyn_cast(LP.Callable.Decl); + const BindArgument &ObjPtr = FunctionCallArgs.front(); + assert(ObjPtr.Kind == BK_ObjectPtr); + + Stream << " { "; + if (!llvm::isa(ignoreTemporariesAndPointers(ObjPtr.E))) { + if (ObjPtr.CaptureMode != CM_None) + Stream << ObjPtr.UsageIdentifier; + else + Stream << ObjPtr.SourceTokens; + Stream << "->"; + } + + Stream << MethodDecl->getName(); + } else { + Stream << " { return "; + switch (LP.Callable.CaptureMode) { + case CM_ByValue: + case CM_ByRef: + if (LP.Callable.UsageIdentifier != LP.Callable.CaptureIdentifier) { + Stream << "(" << LP.Callable.UsageIdentifier << ")"; + break; + } + LLVM_FALLTHROUGH; + case CM_InitExpression: + Stream << LP.Callable.UsageIdentifier; + break; + default: + Ref->printPretty(Stream, nullptr, Result.Context->getPrintingPolicy()); + } + } + Stream << "("; - addFunctionCallArgs(Args, Stream); + + addFunctionCallArgs(getForwardedArgumentList(LP), Stream); Stream << "); }"; Diag << FixItHint::CreateReplacement(MatchedDecl->getSourceRange(), Index: clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp +++ clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp @@ -8,75 +8,51 @@ template bind_rt bind(Fp &&, Arguments &&...); } + +template +T ref(T &t); } -int add(int x, int y) { return x + y; } +namespace boost { +template +class bind_rt {}; -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); }; -} +template +bind_rt bind(const Fp &, Arguments...); +} // namespace boost -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); }; -} +namespace C { +int add(int x, int y) { return x + y; } +} // namespace C -struct placeholder {}; -placeholder _1; -placeholder _2; +struct Foo { + static int add(int x, int y) { return x + y; } +}; -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 D { + D() = default; + void operator()(int x, int y) const {} -struct A; -struct B; -bool ABTest(const A &, const B &); + void MemberFunction(int x) {} -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); }; -} + static D *create(); +}; -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); -} +struct F { + F(int x) {} + ~F() {} -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); -} + int get() { return 42; } +}; -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)); -} +void UseF(F); -namespace C { - int add(int x, int y){ return x + y; } -} +struct placeholder {}; +placeholder _1; +placeholder _2; -void n() { - auto clj = std::bind(C::add, 1, 1); - // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind - // CHECK-FIXES: auto clj = [] { return C::add(1, 1); }; -} +int add(int x, int y) { return x + y; } +int addThree(int x, int y, int z) { return x + y + z; } // Let's fake a minimal std::function-like facility. namespace std { @@ -114,10 +90,188 @@ void Reset(std::function); }; -void test(Thing *t) { +int GlobalVariable = 42; + +struct TestCaptureByValueStruct { + int MemberVariable; + static int StaticMemberVariable; + F MemberStruct; + + void testCaptureByValue(int Param, F f) { + int x = 3; + int y = 4; + auto AAA = std::bind(add, x, y); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind] + // CHECK-FIXES: auto AAA = [x, y] { return add(x, y); }; + + // When the captured variable is repeated, it should only appear in the capture list once. + auto BBB = std::bind(add, x, x); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind] + // CHECK-FIXES: auto BBB = [x] { return add(x, x); }; + + int LocalVariable; + // Global variables shouldn't be captured at all, and members should be captured through this. + auto CCC = std::bind(add, MemberVariable, GlobalVariable); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind] + // CHECK-FIXES: auto CCC = [this] { return add(MemberVariable, GlobalVariable); }; + + // Static member variables shouldn't be captured, but locals should + auto DDD = std::bind(add, TestCaptureByValueStruct::StaticMemberVariable, LocalVariable); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind] + // CHECK-FIXES: auto DDD = [LocalVariable] { return add(TestCaptureByValueStruct::StaticMemberVariable, LocalVariable); }; + + auto EEE = std::bind(add, Param, Param); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind [modernize-avoid-bind] + // CHECK-FIXES: auto EEE = [Param] { return add(Param, Param); }; + + // The signature of boost::bind() is different, and causes + // CXXBindTemporaryExprs to be created in certain cases. So let's test + // those here. + auto FFF = boost::bind(UseF, f); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to boost::bind [modernize-avoid-bind] + // CHECK-FIXES: auto FFF = [f] { return UseF(f); }; + + auto GGG = boost::bind(UseF, MemberStruct); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to boost::bind [modernize-avoid-bind] + // CHECK-FIXES: auto GGG = [this] { return UseF(MemberStruct); }; + } +}; + +void testLiteralParameters() { + auto AAA = std::bind(add, 2, 2); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind] + // CHECK-FIXES: auto AAA = [] { return add(2, 2); }; + + auto BBB = std::bind(addThree, 2, 3, 4); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind [modernize-avoid-bind] + // CHECK-FIXES: auto BBB = [] { return addThree(2, 3, 4); }; +} + +void testCaptureByReference() { + int x = 2; + int y = 2; + auto AAA = std::bind(add, std::ref(x), std::ref(y)); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto AAA = [&x, &y] { return add(x, y); }; + + auto BBB = std::bind(add, std::ref(x), y); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto BBB = [&x, y] { return add(x, y); }; + + auto CCC = std::bind(add, y, std::ref(x)); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto CCC = [y, &x] { return add(y, x); }; +} + +void testCaptureByInitExpression() { + int x = 42; + auto AAA = std::bind(add, x, F(x).get()); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto AAA = [x, capture0 = F(x).get()] { return add(x, capture0); }; +} + +void testPlaceholders() { + int x = 2; + auto AAA = std::bind(add, x, _1); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto AAA = [x](auto && PH1) { return add(x, PH1); }; + + auto BBB = std::bind(add, _2, _1); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto BBB = [](auto && PH1, auto && PH2) { return add(PH2, PH1); }; + + // No fix is applied for reused placeholders. + auto CCC = std::bind(add, _1, _1); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto CCC = std::bind(add, _1, _1); +} + +void testGlobalFunctions() { + auto AAA = std::bind(C::add, 1, 1); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto AAA = [] { return C::add(1, 1); }; + + auto BBB = std::bind(Foo::add, 1, 1); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto BBB = [] { return Foo::add(1, 1); }; + + // The & should get removed inside of the lambda body. + auto CCC = std::bind(&C::add, 1, 1); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto CCC = [] { return C::add(1, 1); }; + + auto DDD = std::bind(&Foo::add, 1, 1); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto DDD = [] { return Foo::add(1, 1); }; + + auto EEE = std::bind(&add, 1, 1); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto EEE = [] { return add(1, 1); }; +} + +void testCapturedSubexpressions() { + int x = 3; + int y = 3; + + auto AAA = std::bind(add, 1, add(2, 5)); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // Results of nested calls are captured by value. + // CHECK-FIXES: auto AAA = [capture0 = add(2, 5)] { return add(1, capture0); }; + + auto BBB = std::bind(add, x, add(y, 5)); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // Results of nested calls are captured by value. + // CHECK-FIXES: auto BBB = [x, capture0 = add(y, 5)] { return add(x, capture0); }; +} + +void testFunctionObjects() { + D d; + D *e = nullptr; + auto AAA = std::bind(d, 1, 2); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto AAA = [d] { return d(1, 2); } + + auto BBB = std::bind(*e, 1, 2); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto BBB = [e] { return (*e)(1, 2); } + + auto CCC = std::bind(D{}, 1, 2); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto CCC = [] { return D{}(1, 2); } + + auto DDD = std::bind(D(), 1, 2); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto DDD = [] { return D()(1, 2); } + + auto EEE = std::bind(*D::create(), 1, 2); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto EEE = [Func = *D::create()] { return Func(1, 2); }; +} + +struct E { + void MemberFunction(int x) {} + + void testMemberFunctions() { + auto CCC = std::bind(&E::MemberFunction, this, 1); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto CCC = [this] { MemberFunction(1); }; + + D *d; + D dd; + auto AAA = std::bind(&D::MemberFunction, d, 1); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto AAA = [d] { d->MemberFunction(1); }; + + auto BBB = std::bind(&D::MemberFunction, &dd, 1); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind + // CHECK-FIXES: auto BBB = [ObjectPtr = &dd] { ObjectPtr->MemberFunction(1); }; + } +}; + +void testStdFunction(Thing *t) { Callback cb; if (t) cb.Reset(std::bind(UseThing, t)); // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind - // CHECK-FIXES: cb.Reset([=] { return UseThing(t); }); + // CHECK-FIXES: cb.Reset([t] { return UseThing(t); }); }