[PATCH] D52281: [clang-tidy] Add modernize check to use std::invoke in generic code

[Borsik Gábor via Phabricator via cfe-commits]

boga95 abandoned this revision.
boga95 marked 4 inline comments as done.
boga95 added inline comments.


================
Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:72
+
+    const Expr *Obj = BinOp->getLHS();
+    const std::string ObjName =
----------------
I found an interesting behavior with source location:

```
const SourceManager *Source = Result.SourceManager;

const char *StartPos = Source->getCharacterData(Obj->getLocStart());
const char *EndPos = Source->getCharacterData(Obj->getLocEnd());
```
`StartPos` and `EndPos` point to the exact same location. Is this the expected behavior or it is a bug?


================
Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:70
+    const auto *Paren = dyn_cast<ParenExpr>(MFunctor->getCallee());
+    const auto *BinOp = dyn_cast<BinaryOperator>(Paren->getSubExpr());
+
----------------
aaron.ballman wrote:
> 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.
It cannot be null because of the matcher.


================
Comment at: test/clang-tidy/modernize-replace-generic-functor-call.cpp:24
+template <class T>
+void func2(T func) {
+  func(1);
----------------
JonasToth wrote:
> Please add tests that include the usage of macros to do the function call.
> In my opinions these should be excluded from the FIXITs and only the warning should be emitted.
Please show me some example of that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52281/new/

https://reviews.llvm.org/D52281