[PATCH] D52281: 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:1
+//===--- ReplaceGenericFunctorCallCheck.cpp - 
clang-tidy---===//
+//

Please add space after clang-tidy.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:48
+const MatchFinder::MatchResult &Result) {
+
+  const auto *Source = Result.SourceManager;

Please remove empty line.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:49
+
+  const auto *Source = Result.SourceManager;
+

Please don't use auto when type could not be deduced from same statement.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:53
+  if (Functor && Functor->isTypeDependent()) {
+const auto EndLoc = Functor->getNumArgs() == 0
+? Functor->getRParenLoc()

Please don't use auto when type could not be deduced from same statement.



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

I think you should use LLVM's cast instead, but I'm not sure which one. Same 
for other places.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:78
+
+const auto *MFunc = BinOp->getRHS();
+const char *MFuncEnd = Source->getCharacterData(Paren->getRParen());

Please don't use auto when type could not be deduced from same statement.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:96
+const std::string &OriginalParams) {
+
+  std::ostringstream Replace;

Please remove empty line.



Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.h:1
+//===--- ReplaceGenericFunctorCallCheck.h - clang-tidy---*- C++ 
-*-===//
+//

Please add space after clang-tidy.



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

Please sort new checks list in alphabetical order.



Comment at: docs/ReleaseNotes.rst:63
+
+  Replace type dependent functor, function pointer and pointer to member call
+  to the proper ``std::invoke()`` in C++17 or newer codebases.

Replace -> Replaces.



Comment at: docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst:6
+
+This check finds and replace the functor, function pointer and pointer to 
member
+calls to the proper ``std::invoke()`` call. It works only in C++17 or newer.

Please make first statement same as in Release Notes.



Comment at: docs/clang-tidy/checks/modernize-replace-generic-functor-call.rst:15
+  void f(T1 func, T2 mfp) {
+func(2); // Replace to ::std::invoke(func, 2)
+Foo foo;

Please use more descriptive identifiers and separate statements with empty 
lines. Same for other places.


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: Use std::invoke in generic code

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

boga95 created this revision.
boga95 added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgorny.

In generic code (e.g.: in a type dependent expression), the `std::invoke` 
should be used, in order to support functors, function pointers, pointers to 
members, regular functions, and other C++ and STL features: link 

The check replace the appropriate calls with calls to std::invoke. This should 
work only in C++17 and newer codebases.


Repository:
  rCTE Clang Tools Extra

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,31 @@
+.. title:: clang-tidy - modernize-replace-generic-functor-call
+
+modernize-replace-generic-functor-call
+==
+
+This check finds and replace the functor, function pointer and pointer to member
+calls to the proper ``std::invoke()`` call. It works only in C++17 or newer.
+
+Examples:
+
+.. code-block:: c++
+
+  template
+  void f(T1 func, T2 mfp) {
+func(2); // Replace to ::std::invoke(func, 2)
+Foo foo;
+(foo.*mfp)(1, 2); // Replace to ::std::invoke(foo, mfp, 1, 2)
+(Foo().*mfp)(3); // Replace to ::std::invoke(Foo(), mfp, 3)
+  }
+
+  // Neither of them is type dependent, no diagnose
+  void g(std::function func, int (*fp)(int), void (Foo::*mfp)(int)) {
+func3();
+std::max(1,2);
+Foo::print_();
+[](){}();
+func();
+fp(3);
+Foo foo;
+(foo.*(mfp))(1);
+  }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -163,6 +163,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
@@ -57,6 +57,12 @@
 Improvements to clang-tidy
 --
 
+- New `modernize-replace-generic-functor-call
+