From eaa55590945a130131a47a4d2b89e3bbdfced79e Mon Sep 17 00:00:00 2001 From: Matthias Gehre Date: Tue, 7 Apr 2020 23:36:53 +0200 Subject: [PATCH] [clang-tidy] misc-unused-parameters: Don't remove parameter from lambda 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 from Lambdas. The AST is not so easy to use. For ``` auto l = [](int) { return; }; f(l); ``` one gets ``` `-CallExpr 'void' |-ImplicitCastExpr 'void (*)(fn *)' | `-DeclRefExpr 'void (fn *)' lvalue Function 0x55a91a545e28 'f' 'void (fn *)' `-ImplicitCastExpr 'void (*)(int)' `-CXXMemberCallExpr 'void (*)(int)' `-MemberExpr '' .operator void (*)(int) 0x55a91a546850 `-ImplicitCastExpr 'const (lambda at line:6:14)' lvalue `-DeclRefExpr '(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". Reviewers: aaron.ballman, alexfh, hokein, njames93 Subscribers: xazax.hun, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D77680 --- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp | 4 +++- .../test/clang-tidy/checkers/misc-unused-parameters.cpp | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp index 9514c044..b1507ed 100644 --- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp +++ b/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 @@ void UnusedParametersCheck::warnOnUnusedParameter( // 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) diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp index 8e546b4..52f9675 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp @@ -276,3 +276,13 @@ public: // 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 -- 2.7.4