diff options
5 files changed, 107 insertions, 15 deletions
diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp index e09f5148ba5..9f5f4daa1fe 100644 --- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp @@ -12,6 +12,7 @@ #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; @@ -29,11 +30,10 @@ bool isOverrideMethod(const FunctionDecl *Function) { } // namespace void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher(functionDecl(isDefinition(), - hasBody(stmt(hasDescendant(stmt()))), - hasAnyParameter(decl())) - .bind("function"), - this); + Finder->addMatcher( + functionDecl(isDefinition(), hasBody(stmt()), hasAnyParameter(decl())) + .bind("function"), + this); } template <typename T> @@ -122,7 +122,12 @@ UnusedParametersCheck::~UnusedParametersCheck() = default; UnusedParametersCheck::UnusedParametersCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + : ClangTidyCheck(Name, Context), + StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0) {} + +void UnusedParametersCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "StrictMode", StrictMode); +} void UnusedParametersCheck::warnOnUnusedParameter( const MatchFinder::MatchResult &Result, const FunctionDecl *Function, @@ -170,7 +175,15 @@ void UnusedParametersCheck::check(const MatchFinder::MatchResult &Result) { if (Param->isUsed() || Param->isReferenced() || !Param->getDeclName() || Param->hasAttr<UnusedAttr>()) continue; - warnOnUnusedParameter(Result, Function, i); + + // In non-strict mode ignore function definitions with empty bodies + // (constructor initializer counts for non-empty body). + if (StrictMode || + (Function->getBody()->child_begin() != + Function->getBody()->child_end()) || + (isa<CXXConstructorDecl>(Function) && + cast<CXXConstructorDecl>(Function)->getNumCtorInitializers() > 0)) + warnOnUnusedParameter(Result, Function, i); } } diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.h b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.h index 414ef1127dd..b9bae26ff49 100644 --- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.h +++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.h @@ -24,8 +24,10 @@ public: ~UnusedParametersCheck(); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; private: + const bool StrictMode; class IndexerVisitor; std::unique_ptr<IndexerVisitor> Indexer; diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst index de868b91858..3dfeb299de0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst @@ -3,24 +3,40 @@ misc-unused-parameters ====================== -Finds unused parameters and fixes them, so that `-Wunused-parameter` can be -turned on. +Finds unused function parameters. Unused parameters may signify a bug in the +code (e.g. when a different parameter is used instead). The suggested fixes +either comment parameter name out or remove the parameter completely, if all +callers of the function are in the same translation unit and can be updated. + +The check is similar to the `-Wunused-parameter` compiler diagnostic and can be +used to prepare a codebase to enabling of that diagnostic. By default the check +is more permissive (see :option:`StrictMode`). .. code-block:: c++ - void a(int i) {} + void a(int i) { /*some code that doesn't use `i`*/ } // becomes - void a(int /*i*/) {} - + void a(int /*i*/) { /*some code that doesn't use `i`*/ } .. code-block:: c++ static void staticFunctionA(int i); - static void staticFunctionA(int i) {} + static void staticFunctionA(int i) { /*some code that doesn't use `i`*/ } // becomes static void staticFunctionA() - static void staticFunctionA() {} + static void staticFunctionA() { /*some code that doesn't use `i`*/ } + +Options +------- + +.. option:: StrictMode + + When zero (default value), the check will ignore trivially unused parameters, + i.e. when the corresponding function has an empty body (and in case of + constructors - no constructor initializers). When the function body is empty, + an unused parameter is unlikely to be unnoticed by a human reader, and + there's basically no place for a bug to hide. diff --git a/clang-tools-extra/test/clang-tidy/misc-unused-parameters-strict.cpp b/clang-tools-extra/test/clang-tidy/misc-unused-parameters-strict.cpp new file mode 100644 index 00000000000..a334b45942e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/misc-unused-parameters-strict.cpp @@ -0,0 +1,25 @@ +// RUN: %check_clang_tidy %s misc-unused-parameters %t -- \ +// RUN: -config="{CheckOptions: [{key: StrictMode, value: 1}]}" -- + +// Warn on empty function bodies in StrictMode. +namespace strict_mode { +void f(int foo) {} +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'foo' is unused [misc-unused-parameters] +// CHECK-FIXES: {{^}}void f(int /*foo*/) {}{{$}} +class E { + int i; + +public: + E(int j) {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused +// CHECK-FIXES: {{^}} E(int /*j*/) {}{{$}} +}; +class F { + int i; + +public: + F(int j) : i() {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused +// CHECK-FIXES: {{^}} F(int /*j*/) : i() {}{{$}} +}; +} diff --git a/clang-tools-extra/test/clang-tidy/misc-unused-parameters.cpp b/clang-tools-extra/test/clang-tidy/misc-unused-parameters.cpp index 42554b958dd..feb3a142b66 100644 --- a/clang-tools-extra/test/clang-tidy/misc-unused-parameters.cpp +++ b/clang-tools-extra/test/clang-tidy/misc-unused-parameters.cpp @@ -222,5 +222,41 @@ static Function<void(int, int i)> dontGetConfusedByFunctionReturnTypes() { return Function<void(int, int)>(); } +namespace strict_mode_off { // Do not warn on empty function bodies. -void f(int foo) {} +void f1(int foo1) {} +void f2(int foo2) { + // "empty" in the AST sense, not in textual sense. +} +void f3(int foo3) {;} +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: parameter 'foo3' is unused +// CHECK-FIXES: {{^}}void f3(int /*foo3*/) {;}{{$}} + +class E { + int i; + +public: + E(int j) {} +}; +class F { + int i; + +public: + // Constructor initializer counts as a non-empty body. + F(int j) : i() {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused +// CHECK-FIXES: {{^}} F(int /*j*/) : i() {}{{$}} +}; + +class A { +public: + A(); + A(int); +}; +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 |
