aaron.ballman added inline comments.
================ Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:70 + const auto *Paren = dyn_cast<ParenExpr>(MFunctor->getCallee()); + const auto *BinOp = dyn_cast<BinaryOperator>(Paren->getSubExpr()); + ---------------- How do you know `Paren` won't be null? If it cannot be null, please use `cast<>` instead, otherwise, you should be checking for null before dereferencing. ================ Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:85 + + const std::string Param = ObjName + ", " + FuncName; + diagnose(MFunctor, Param, OriginalParams); ---------------- Perhaps use a `Twine` in the `diagnose()` call for this. ================ Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:94-96 + const Twine &Replace = Twine("::std::invoke(") + Param + + (Functor->getNumArgs() == 0 ? "" : ", ") + + OriginalParams; ---------------- This is dangerous (the intermediary temps will be destroyed and you will be referencing dangling memory) -- you should lower it into the call to `CreateReplacement()`. ================ Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:98 + diag(Functor->getExprLoc(), + "Use ::std::invoke to invoke type dependent callable objects.") + << FixItHint::CreateReplacement(Functor->getSourceRange(), Replace.str()); ---------------- Diagnostics should not be complete sentences, so Use -> use and drop the full stop. ================ Comment at: test/clang-tidy/modernize-replace-generic-functor-call.cpp:23-25 +template <class T> +void func2(T func) { + func(1); ---------------- Can you add a test case that demonstrates this works well in the presence of std::forward? This is a common pattern: ``` template <typename Callable, typename... Args> void f(Callable&& C, Args&&... As) { std::forward<Callable>(C)(std::forward<Args>(As)...); } ``` This could be converted into: ``` template <typename Callable, typename... Args> void f(Callable&& C, Args&&... As) { std::invoke(C, As...); } ``` https://reviews.llvm.org/D52281 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits