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

[mgehre]

Summary

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".

[njames93]

Outright disabling for lambdas probably isn't the ideal solution, however it's at least a good stepping stone til a better solution can be found.

[mgehre]

In D77680#1968860, @njames93 wrote:

Outright disabling for lambdas probably isn't the ideal solution, however it's at least a good stepping stone til a better solution can be found.

I fully agree!

[aaron.ballman]

LGTM as a temporary workaround until we find a better solution.

(I just noticed that we don't need a similar fix for blocks because this check doesn't seem to support blocks: https://godbolt.org/z/HZM2t2)

clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp

 //===--- UnusedParametersCheck.cpp - clang-tidy----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #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"
 #include "llvm/ADT/STLExtras.h"
 #include <unordered_set>
 
 using namespace clang::ast_matchers;
 
void UnusedParametersCheck::warnOnUnusedParameter(
 
   if (!Indexer) {
     Indexer = std::make_unique<IndexerVisitor>(*Result.Context);
   }
 
   // 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)
       return;
 
     SourceRange RemovalRange(Param->getLocation());
     // Note: We always add a space before the '/*' to not accidentally create
     // a '*/*' for pointer types, which doesn't start a comment. clang-format

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

 // RUN: echo "static void staticFunctionHeader(int i) {;}" > %T/header.h
 // RUN: echo "static void staticFunctionHeader(int  /*i*/) {;}" > %T/header-fixed.h
 // RUN: %check_clang_tidy -std=c++11 %s misc-unused-parameters %t -- -header-filter='.*' -- -fno-delayed-template-parsing
 // RUN: diff %T/header.h %T/header-fixed.h
 // FIXME: Make the test work in all language modes.
 
 #include "header.h"
 // CHECK-MESSAGES: header.h:1:38: warning

 };
 class B : public A {
 public:
   B(int i) : A() {}
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is unused
 // 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