This revision was automatically updated to reflect the committed changes.
Closed by commit rL335863: [clang-tidy] misc-unused-parameters - retain old
behavior under StrictMode (authored by alexfh, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D46951
Files:
clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst
clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters-strict.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst
===================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst
+++ clang-tools-extra/trunk/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.
Index: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
===================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
@@ -222,5 +222,41 @@
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
Index: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters-strict.cpp
===================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters-strict.cpp
+++ clang-tools-extra/trunk/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() {}{{$}}
+};
+}
Index: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h
===================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h
@@ -24,8 +24,10 @@
~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;
Index: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/trunk/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 @@
} // 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(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 @@
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);
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
