aaron.ballman added a comment. This is a great re-write, thank you for working on it!
================ Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:105 + CallableInfo Callable; + + SmallVector<BindArgument, 4> BindArguments; ---------------- You can remove some newlines here. ================ Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:116 +static const Expr *ignoreTemporariesAndImplicitCasts(const Expr *E) { + if (const auto *T = dyn_cast<MaterializeTemporaryExpr>(E)) + return ignoreTemporariesAndImplicitCasts(T->GetTemporaryExpr()); ---------------- What about `CXXBindTemporaryExpr`? Would `Expr::IgnoreImplicit()` do too much stripping because it removes `FullExpr` as well? ================ Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:124 +static const Expr *ignoreTemporariesAndPointers(const Expr *E) { + + if (const auto *T = dyn_cast<UnaryOperator>(E)) ---------------- Spurious newline ================ Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:130 + + if (const auto *T = dyn_cast<MaterializeTemporaryExpr>(E)) + return ignoreTemporariesAndPointers(T->GetTemporaryExpr()); ---------------- Similar question here about bound temporaries as above. ================ Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:143-147 + std::string S; + llvm::raw_string_ostream OS(S); + OS << "capture" << CaptureIndex++; + OS.flush(); + return S; ---------------- `llvm::utostr()` instead of using a streaming interface? ================ Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:144 + std::string S; + llvm::raw_string_ostream OS(S); + OS << "capture" << CaptureIndex++; ---------------- I think you can drop the `llvm::` here and elsewhere in the file. ================ Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:153 + + return HNM.matchesNode(*dyn_cast<NamedDecl>(D)); +} ---------------- This should probably use `cast<>` if it's going to assume the returned value is never null. ================ Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:196-199 + for (const auto *Child : Statement->children()) { + if (anyDescendantIsLocal(Child)) + return true; + } ---------------- `return llvm::any_of(Statement->children(), anyDescendantIsLocal);`? ================ Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:431-434 + if (!Candidates.empty()) + return Candidates; + + return Candidates; ---------------- It seems like `return Candidates;` will suffice. :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70368/new/ https://reviews.llvm.org/D70368 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits