[PATCH] D77680: [clang-tidy] misc-unused-parameters: Don't remove parameter from lambda

[Matthias Gehre via Phabricator via cfe-commits]

mgehre created this revision.
mgehre added reviewers: aaron.ballman, alexfh, hokein, njames93.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.
mgehre added a project: clang-tools-extra.
mgehre edited the summary of this revision.

Previously, the check would fix

  using fn = void(int);
  void f(fn *);
  void test() {
    // CHECK-MESSAGES: :[[@LINE+2]]:12: warning: parameter 'I' is unused
    // CHECK-FIXES: {{^}}  f([](int  /*I*/) {
    f([](int I) { return; });
  }

into
`f([]() { return; });` which breaks compilation. Now the check is disabled for lambdas.

The AST is not so easy to use. For

  auto l = [](int) {  return;  };
  f(l);

one gets

  `-CallExpr <line:7:5, col:8> 'void'
       |-ImplicitCastExpr <col:5> 'void (*)(fn *)' <FunctionToPointerDecay>
       | `-DeclRefExpr <col:5> 'void (fn *)' lvalue Function 0x55a91a545e28 'f' 'void (fn *)'
       `-ImplicitCastExpr <col:7> 'void (*)(int)' <UserDefinedConversion>
         `-CXXMemberCallExpr <col:7> 'void (*)(int)'
           `-MemberExpr <col:7> '<bound member function type>' .operator void (*)(int) 0x55a91a546850
             `-ImplicitCastExpr <col:7> 'const (lambda at line:6:14)' lvalue <NoOp>
               `-DeclRefExpr <col:7> '(lambda at line:6:14)':'(lambda at line:6:14)' lvalue Var 0x55a91a5461c0 'l' '(lambda at line:6:14)':'(lambda at line:6:14)'

There is no direct use of the `operator()(int I)` of the lambda, so the `!Indexer->getOtherRefs(Function).empty()`
does not fire. In the future, we might be able to use the conversion operator `operator void (*)(int)` to mark
the call operator as having an "other ref".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77680

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -276,3 +276,13 @@
 // CHECK-FIXES: {{^}}  B(int  /*i*/) : A() {}{{$}}
 };
 } // namespace strict_mode_off
+
+namespace lambda {
+using fn = void(int);
+void f(fn *);
+void test() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:12: warning: parameter 'I' is unused
+  // CHECK-FIXES: {{^}}  f([](int  /*I*/) {
+  f([](int I) { return; });
+}
+} // namespace lambda
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "UnusedParametersCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTLambda.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
@@ -141,7 +142,8 @@
   // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
       !Result.SourceManager->isInMainFile(Function->getLocation()) ||
-      !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
+      !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function) ||
+      isLambdaCallOperator(Function)) {
 
     // It is illegal to omit parameter name here in C code, so early-out.
     if (!Result.Context->getLangOpts().CPlusPlus)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77680.255818.patch
Type: text/x-patch
Size: 1675 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200407/3544846d/attachment.bin>