zturner updated this revision to Diff 228093.
zturner added a comment.
Minor cleanup -- moved `isFixitSupported` logic to its own function.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69872/new/
https://reviews.llvm.org/D69872
Files:
clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp
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
@@ -78,6 +78,39 @@
// CHECK-FIXES: auto clj = [] { return C::add(1, 1); };
}
+struct D {
+ void operator()(int x, int y) {}
+
+ void MemberFunction(int x) {}
+};
+
+void o() {
+ D d;
+ auto clj = std::bind(d, 1, 2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+ // CHECK-FIXES: auto clj = std::bind(d, 1, 2);
+}
+
+void p() {
+ auto clj = std::bind(D{}, 1, 2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+ // CHECK-FIXES: auto clj = std::bind(D{}, 1, 2);
+}
+
+void q() {
+ D *d;
+ auto clj = std::bind(*d, 1, 2);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+ // CHECK-FIXES: auto clj = std::bind(*d, 1, 2);
+}
+
+void r() {
+ D *d;
+ auto clj = std::bind(&D::MemberFunction, d, 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: prefer a lambda to std::bind
+ // CHECK-FIXES: auto clj = std::bind(&D::MemberFunction, d, 1);
+}
+
// Let's fake a minimal std::function-like facility.
namespace std {
template <typename _Tp>
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
@@ -120,41 +120,49 @@
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")))
- .bind("bind"),
- this);
+ Finder->addMatcher(callExpr(callee(namedDecl(hasName("::std::bind"))),
+ hasArgument(0, expr().bind("ref")))
+ .bind("bind"),
+ this);
}
-void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *MatchedDecl = Result.Nodes.getNodeAs<CallExpr>("bind");
- auto Diag = diag(MatchedDecl->getBeginLoc(), "prefer a lambda to std::bind");
-
- const auto Args = buildBindArguments(Result, MatchedDecl);
-
+static bool isFixitSupported(const Expr *Expr, ArrayRef<BindArgument> Args) {
// 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;
+ 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;
+ return false;
+
+ if (const auto *DRE = llvm::dyn_cast<DeclRefExpr>(Expr)) {
+ const auto *FD = llvm::dyn_cast_or_null<FunctionDecl>(DRE->getDecl());
+ // 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 (FD && (FD->getNumParams() == Args.size()))
+ return true;
+ }
- const auto *F = Result.Nodes.getNodeAs<FunctionDecl>("f");
+ // Do not attempt to create fixits for generalized call expressions.
+ return false;
+}
+
+void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedDecl = Result.Nodes.getNodeAs<CallExpr>("bind");
+ auto Diag = diag(MatchedDecl->getBeginLoc(), "prefer a lambda to std::bind");
+
+ const auto Args = buildBindArguments(Result, MatchedDecl);
- // 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())
+ const auto *Ref = Result.Nodes.getNodeAs<Expr>("ref");
+ if (!isFixitSupported(Ref, Args))
return;
std::string Buffer;
@@ -162,7 +170,6 @@
bool HasCapturedArgument = llvm::any_of(
Args, [](const BindArgument &B) { return B.Kind == BK_Other; });
- const auto *Ref = Result.Nodes.getNodeAs<DeclRefExpr>("ref");
Stream << "[" << (HasCapturedArgument ? "=" : "") << "]";
addPlaceholderArgs(Args, Stream);
Stream << " { return ";
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
