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

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

boga95 marked an inline comment as done.
boga95 added a comment.

Resolving the problems properly seemed quite difficult. I just finished 
university and started to work, therefore I don't have enough free time to 
finish the checker. Sorry about that.


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

https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The review says you abandoned it -- was that accidental?




Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:72
+
+const Expr *Obj = BinOp->getLHS();
+const std::string ObjName =

boga95 wrote:
> 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?
That is expected behavior -- the source location points to the insertion point 
of the token and if there's only one token in the source range, two two 
locations will point to the same place.


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

https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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(MFunctor->getCallee());
+const auto *BinOp = dyn_cast(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 
+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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:32
+  //   ::std::invoke(func, 1)
+  Finder->addMatcher(callExpr(has(declRefExpr())).bind("functor"), this);
+

I don't think this will work for calls wrapped in parens or with spurious 
dereferences.
```
void f(int) {}

int main() {
  void (*fp)(int) = f;

  fp(12);
  (fp)(12);
  (*fp)(12);
}
```
It seems like all of those can be replaced by `std::invoke()` directly.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:97
+ Functor->getSourceRange(),
+ (Twine("::std::invoke(") + Param +
+  (Functor->getNumArgs() == 0 ? "" : ", ") + OriginalParams)

You might be able to get rid of the explicit `Twine` construction here now that 
`Param` is a `Twine`. I could be remembering wrong though.


https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

boga95 updated this revision to Diff 167991.
boga95 marked 7 inline comments as done.

https://reviews.llvm.org/D52281

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp
  clang-tidy/modernize/ReplaceGenericFunctorCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
  test/clang-tidy/modernize-replace-generic-functor-call.cpp

Index: test/clang-tidy/modernize-replace-generic-functor-call.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-replace-generic-functor-call.cpp
@@ -0,0 +1,56 @@
+// RUN: %check_clang_tidy %s modernize-replace-generic-functor-call %t -- -- -std=c++17
+
+namespace std {
+
+template 
+T max(T a, T b) {
+  return a < b ? b : a;
+}
+
+struct function {
+  void operator()();
+};
+
+} // namespace std
+
+struct Foo {
+  static void print_() {}
+  void print() const {}
+  int num_;
+};
+
+// Something that triggers the check
+template 
+void func2(T func) {
+  func(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use ::std::invoke to invoke type dependent callable objects [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(func, 1);
+  func();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use ::std::invoke to invoke type dependent callable objects [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(func);
+
+  Foo foo;
+  (foo.*func)(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use ::std::invoke to invoke type dependent callable objects [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(foo, func, 1);
+  (foo.*func)();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use ::std::invoke to invoke type dependent callable objects [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(foo, func);
+  (Foo().*func)(1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use ::std::invoke to invoke type dependent callable objects [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(Foo(), func, 1, 2);
+}
+
+// Something that doesn't triggers the check
+void func3() {}
+
+void g(std::function func, int (*fp)(int), void (Foo::*mfp)(int)) {
+  func3();// Regular function call
+  std::max(1, 2); // Template function call
+  Foo::print_();  // Static function call
+  []() {}();  // Lambda call
+  func(); // Call through std::function
+  fp(3);  // Call through function pointer
+  Foo foo;
+  (foo.*(mfp))(1); // Call through member function pointer
+}
Index: docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
@@ -0,0 +1,39 @@
+.. title:: clang-tidy - modernize-replace-generic-functor-call
+
+modernize-replace-generic-functor-call
+==
+
+Replaces type dependent functor, function pointer and pointer to member call
+to the proper ``std::invoke()`` in C++17 or newer codebases.
+  
+Examples:
+
+.. code-block:: c++
+
+  template
+  void f(T1 functionPointer, T2 memberFunctionPointer) {
+functionPointer(2); // Replace to ::std::invoke(functionPointer, 2)
+
+Foo foo;
+(foo.*memberFunctionPointer)(1, 2); // Replace to ::std::invoke(foo, memberFunctionPointer, 1, 2)
+
+(Foo().*memberFunctionPointer)(3); // Replace to ::std::invoke(Foo(), memberFunctionPointer, 3)
+  }
+
+  // Neither of them is type dependent, no diagnose
+  void g(std::function func, int (*functionPointer)(int), void (Foo::*memberFunctionPointer)(int)) {
+freeFunction();
+
+std::max(1,2);
+
+Foo::print_();
+
+[](){}();
+
+func();
+
+functionPointer(3);
+
+Foo foo;
+(foo.*(memberFunctionPointer))(1);
+  }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -171,6 +171,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-generic-functor-call
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -142,6 +142,12 @@
 
   Checks on ``switch`` and ``if`` - ``else if`` constructs that do not cover all possible code paths.
 
+- New :doc:`modernize-replace-generic-functor-call
+  ` check.
+
+  Replaces type dependent functor, function pointer and pointer to member call
+  to the proper ``std::invoke()`` in C++17 or newer codebases.
+  
 - New :doc:`modernize-use-uncaught-exceptions
   ` check.
 
Index: 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:70
+const auto *Paren = dyn_cast(MFunctor->getCallee());
+const auto *BinOp = dyn_cast(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  = 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 
+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 
void f(Callable&& C, Args&&... As) {
  std::forward(C)(std::forward(As)...);
}
```
This could be converted into:
```
template 
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

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

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

boga95 updated this revision to Diff 166994.
boga95 marked 12 inline comments as done.

https://reviews.llvm.org/D52281

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp
  clang-tidy/modernize/ReplaceGenericFunctorCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
  test/clang-tidy/modernize-replace-generic-functor-call.cpp

Index: test/clang-tidy/modernize-replace-generic-functor-call.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-replace-generic-functor-call.cpp
@@ -0,0 +1,56 @@
+// RUN: %check_clang_tidy %s modernize-replace-generic-functor-call %t -- -- -std=c++17
+
+namespace std {
+
+template 
+T max(T a, T b) {
+  return a < b ? b : a;
+}
+
+struct function {
+  void operator()();
+};
+
+} // namespace std
+
+struct Foo {
+  static void print_() {}
+  void print() const {}
+  int num_;
+};
+
+// Something that triggers the check
+template 
+void func2(T func) {
+  func(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(func, 1);
+  func();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(func);
+
+  Foo foo;
+  (foo.*func)(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(foo, func, 1);
+  (foo.*func)();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(foo, func);
+  (Foo().*func)(1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(Foo(), func, 1, 2);
+}
+
+// Something that doesn't triggers the check
+void func3() {}
+
+void g(std::function func, int (*fp)(int), void (Foo::*mfp)(int)) {
+  func3();// Regular function call
+  std::max(1, 2); // Template function call
+  Foo::print_();  // Static function call
+  []() {}();  // Lambda call
+  func(); // Call through std::function
+  fp(3);  // Call through function pointer
+  Foo foo;
+  (foo.*(mfp))(1); // Call through member function pointer
+}
Index: docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
@@ -0,0 +1,39 @@
+.. title:: clang-tidy - modernize-replace-generic-functor-call
+
+modernize-replace-generic-functor-call
+==
+
+Replaces type dependent functor, function pointer and pointer to member call
+to the proper ``std::invoke()`` in C++17 or newer codebases.
+  
+Examples:
+
+.. code-block:: c++
+
+  template
+  void f(T1 functionPointer, T2 memberFunctionPointer) {
+functionPointer(2); // Replace to ::std::invoke(functionPointer, 2)
+
+Foo foo;
+(foo.*memberFunctionPointer)(1, 2); // Replace to ::std::invoke(foo, memberFunctionPointer, 1, 2)
+
+(Foo().*memberFunctionPointer)(3); // Replace to ::std::invoke(Foo(), memberFunctionPointer, 3)
+  }
+
+  // Neither of them is type dependent, no diagnose
+  void g(std::function func, int (*functionPointer)(int), void (Foo::*memberFunctionPointer)(int)) {
+freeFunction();
+
+std::max(1,2);
+
+Foo::print_();
+
+[](){}();
+
+func();
+
+functionPointer(3);
+
+Foo foo;
+(foo.*(memberFunctionPointer))(1);
+  }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -171,6 +171,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-generic-functor-call
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -142,6 +142,12 @@
 
   Checks on ``switch`` and ``if`` - ``else if`` constructs that do not cover all possible code paths.
 
+- New :doc:`modernize-replace-generic-functor-call
+  ` check.
+
+  Replaces type dependent functor, function pointer and pointer to member call
+  to the proper ``std::invoke()`` in C++17 or newer codebases.
+  
 - New :doc:`modernize-use-uncaught-exceptions
   ` check.
 
Index: 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:70
+  if (MFunctor && MFunctor->isTypeDependent()) {
+const auto *Paren = static_cast(MFunctor->getCallee());
+const auto *BinOp =

JonasToth wrote:
> Eugene.Zelenko wrote:
> > I think you should use LLVM's cast instead, but I'm not sure which one. 
> > Same for other places.
> in this case i think `const auto *BinOp = 
> dyn_cast(Paren->getSubExpr());` would match.
> 
> As a safety measure adding a `assert(BinOp && "No Binary Operator as 
> subexpression");` helps spotting bugs.
I agree; you'd want `dyn_cast` here. There's no need to add that assertion -- 
`dyn_cast` already asserts that the given value is non-null. If the value could 
be null and still be valid, you could use `dyn_cast_or_null` instead.


https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:70
+  if (MFunctor && MFunctor->isTypeDependent()) {
+const auto *Paren = static_cast(MFunctor->getCallee());
+const auto *BinOp =

Eugene.Zelenko wrote:
> I think you should use LLVM's cast instead, but I'm not sure which one. Same 
> for other places.
in this case i think `const auto *BinOp = 
dyn_cast(Paren->getSubExpr());` would match.

As a safety measure adding a `assert(BinOp && "No Binary Operator as 
subexpression");` helps spotting bugs.



Comment at: test/clang-tidy/modernize-replace-generic-functor-call.cpp:24
+template 
+void func2(T func) {
+  func(1);

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.


https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

boga95 updated this revision to Diff 166604.

https://reviews.llvm.org/D52281

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp
  clang-tidy/modernize/ReplaceGenericFunctorCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
  test/clang-tidy/modernize-replace-generic-functor-call.cpp

Index: test/clang-tidy/modernize-replace-generic-functor-call.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-replace-generic-functor-call.cpp
@@ -0,0 +1,56 @@
+// RUN: %check_clang_tidy %s modernize-replace-generic-functor-call %t -- -- -std=c++17
+
+namespace std {
+
+template 
+T max(T a, T b) {
+  return a < b ? b : a;
+}
+
+struct function {
+  void operator()();
+};
+
+} // namespace std
+
+struct Foo {
+  static void print_() {}
+  void print() const {}
+  int num_;
+};
+
+// Something that triggers the check
+template 
+void func2(T func) {
+  func(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(func, 1);
+  func();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(func);
+
+  Foo foo;
+  (foo.*func)(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(foo, func, 1);
+  (foo.*func)();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(foo, func);
+  (Foo().*func)(1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use ::std::invoke to invoke type dependent callable objects. [modernize-replace-generic-functor-call]
+  // CHECK-FIXES: ::std::invoke(Foo(), func, 1, 2);
+}
+
+// Something that doesn't triggers the check
+void func3() {}
+
+void g(std::function func, int (*fp)(int), void (Foo::*mfp)(int)) {
+  func3();// Regular function call
+  std::max(1, 2); // Template function call
+  Foo::print_();  // Static function call
+  []() {}();  // Lambda call
+  func(); // Call through std::function
+  fp(3);  // Call through function pointer
+  Foo foo;
+  (foo.*(mfp))(1); // Call through member function pointer
+}
Index: docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst
@@ -0,0 +1,39 @@
+.. title:: clang-tidy - modernize-replace-generic-functor-call
+
+modernize-replace-generic-functor-call
+==
+
+Replaces type dependent functor, function pointer and pointer to member call
+to the proper ``std::invoke()`` in C++17 or newer codebases.
+  
+Examples:
+
+.. code-block:: c++
+
+  template
+  void f(T1 functionPointer, T2 memberFunctionPointer) {
+functionPointer(2); // Replace to ::std::invoke(functionPointer, 2)
+
+Foo foo;
+(foo.*memberFunctionPointer)(1, 2); // Replace to ::std::invoke(foo, memberFunctionPointer, 1, 2)
+
+(Foo().*memberFunctionPointer)(3); // Replace to ::std::invoke(Foo(), memberFunctionPointer, 3)
+  }
+
+  // Neither of them is type dependent, no diagnose
+  void g(std::function func, int (*functionPointer)(int), void (Foo::*memberFunctionPointer)(int)) {
+freeFunction();
+
+std::max(1,2);
+
+Foo::print_();
+
+[](){}();
+
+func();
+
+functionPointer(3);
+
+Foo foo;
+(foo.*(memberFunctionPointer))(1);
+  }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -171,6 +171,7 @@
modernize-raw-string-literal
modernize-redundant-void-arg
modernize-replace-auto-ptr
+   modernize-replace-generic-functor-call
modernize-replace-random-shuffle
modernize-return-braced-init-list
modernize-shrink-to-fit
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -142,6 +142,12 @@
 
   Checks on ``switch`` and ``if`` - ``else if`` constructs that do not cover all possible code paths.
 
+- New :doc:`modernize-replace-generic-functor-call
+  ` check.
+
+  Replaces type dependent functor, function pointer and pointer to member call
+  to the proper ``std::invoke()`` in C++17 or newer codebases.
+  
 - New :doc:`modernize-use-uncaught-exceptions
   ` check.
 
Index: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.h

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:27-32
+  // template
+  // void f(T func) {
+  //   func();
+  //   ^~~
+  //   ::std::invoke(func, 1)
+  Finder->addMatcher(callExpr(has(declRefExpr())).bind("functor"), this);

lebedev.ri wrote:
> aaron.ballman wrote:
> > lebedev.ri wrote:
> > > Bikeshedding: i do not understand the idea behind `std::invoke`.
> > > Why is the new version better/preferred?
> > It generically handles all the various kinds of "callable" objects 
> > (lambdas, functors, function pointers, pointer to member functions, etc).
> Thank you for answering, but i still do not understand.
> E.g. on that example, what benefits does it give?
> Even after looking at cppreference, i'm not sure..
> Just curious.
Genericity is the primary benefit. Here's the original paper with its 
motivation:

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3727.html


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:27-32
+  // template
+  // void f(T func) {
+  //   func();
+  //   ^~~
+  //   ::std::invoke(func, 1)
+  Finder->addMatcher(callExpr(has(declRefExpr())).bind("functor"), this);

aaron.ballman wrote:
> lebedev.ri wrote:
> > Bikeshedding: i do not understand the idea behind `std::invoke`.
> > Why is the new version better/preferred?
> It generically handles all the various kinds of "callable" objects (lambdas, 
> functors, function pointers, pointer to member functions, etc).
Thank you for answering, but i still do not understand.
E.g. on that example, what benefits does it give?
Even after looking at cppreference, i'm not sure..
Just curious.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:27-32
+  // template
+  // void f(T func) {
+  //   func();
+  //   ^~~
+  //   ::std::invoke(func, 1)
+  Finder->addMatcher(callExpr(has(declRefExpr())).bind("functor"), this);

lebedev.ri wrote:
> Bikeshedding: i do not understand the idea behind `std::invoke`.
> Why is the new version better/preferred?
It generically handles all the various kinds of "callable" objects (lambdas, 
functors, function pointers, pointer to member functions, etc).



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:97-99
+  std::ostringstream Replace;
+  Replace << "::std::invoke(" << Param
+  << (Functor->getNumArgs() == 0 ? "" : ", ") << OriginalParams;

This should use a `Twine` instead.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.h:19
+
+/// Use std::invoke to call callable objects in generic code
+///

Missing full stop at the end of the comment.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added a comment.

This needs a test when calling in a `constexpr` function. I believe 
`std::invoke` is not `constepxr`, so a function object call in a `constexpr` 
function should not suggest `std::invoke`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:27-32
+  // template
+  // void f(T func) {
+  //   func();
+  //   ^~~
+  //   ::std::invoke(func, 1)
+  Finder->addMatcher(callExpr(has(declRefExpr())).bind("functor"), this);

Bikeshedding: i do not understand the idea behind `std::invoke`.
Why is the new version better/preferred?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Looks like you patch is not based on trunk. Please rebase.




Comment at: docs/ReleaseNotes.rst:60
 
+- New `modernize-replace-generic-functor-call
+  
`_
 check

Eugene.Zelenko wrote:
> Please sort new checks list in alphabetical order.
Please take a look on trunk Release Notes and use :doc: for link.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:114
+}
+} // namespace modernize
+} // namespace tidy

Please insert empty line above.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52281



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits