[clang-tools-extra] 19d7a0b - [clang-tidy] [bugprone-assert-side-effect] Ignore list for functions/methods
Zinovy Nis via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 25 10:07:32 PST 2022
Author: Zinovy Nis
Date: 2022-01-25T21:04:07+03:00
New Revision: 19d7a0b47b6849923576845f87a3278e012e049b
URL: https://github.com/llvm/llvm-project/commit/19d7a0b47b6849923576845f87a3278e012e049b
DIFF: https://github.com/llvm/llvm-project/commit/19d7a0b47b6849923576845f87a3278e012e049b.diff
LOG: [clang-tidy] [bugprone-assert-side-effect] Ignore list for functions/methods
A semicolon-separated list of the names of functions or methods to be considered as not having side-effects was added for bugprone-assert-side-effect. It can be used to exclude methods like iterator::begin/end from being considered as having side-effects.
Differential Revision: https://reviews.llvm.org/D116478
Added:
Modified:
clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
index 4e2359ff4f67b..eba6b29f56af9 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -7,6 +7,8 @@
//===----------------------------------------------------------------------===//
#include "AssertSideEffectCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Frontend/CompilerInstance.h"
@@ -25,7 +27,9 @@ namespace bugprone {
namespace {
-AST_MATCHER_P(Expr, hasSideEffect, bool, CheckFunctionCalls) {
+AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
+ clang::ast_matchers::internal::Matcher<NamedDecl>,
+ IgnoredFunctionsMatcher) {
const Expr *E = &Node;
if (const auto *Op = dyn_cast<UnaryOperator>(E)) {
@@ -55,7 +59,8 @@ AST_MATCHER_P(Expr, hasSideEffect, bool, CheckFunctionCalls) {
bool Result = CheckFunctionCalls;
if (const auto *FuncDecl = CExpr->getDirectCallee()) {
if (FuncDecl->getDeclName().isIdentifier() &&
- FuncDecl->getName() == "__builtin_expect") // exceptions come here
+ IgnoredFunctionsMatcher.matches(*FuncDecl, Finder,
+ Builder)) // exceptions come here
Result = false;
else if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl))
Result &= !MethodDecl->isConst();
@@ -72,8 +77,9 @@ AssertSideEffectCheck::AssertSideEffectCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
CheckFunctionCalls(Options.get("CheckFunctionCalls", false)),
- RawAssertList(Options.get("AssertMacros",
- "assert,NSAssert,NSCAssert")) {
+ RawAssertList(Options.get("AssertMacros", "assert,NSAssert,NSCAssert")),
+ IgnoredFunctions(utils::options::parseStringList(
+ "__builtin_expect;" + Options.get("IgnoredFunctions", ""))) {
StringRef(RawAssertList).split(AssertMacros, ",", -1, false);
}
@@ -81,11 +87,17 @@ AssertSideEffectCheck::AssertSideEffectCheck(StringRef Name,
void AssertSideEffectCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "CheckFunctionCalls", CheckFunctionCalls);
Options.store(Opts, "AssertMacros", RawAssertList);
+ Options.store(Opts, "IgnoredFunctions",
+ utils::options::serializeStringList(IgnoredFunctions));
}
void AssertSideEffectCheck::registerMatchers(MatchFinder *Finder) {
+ auto IgnoredFunctionsMatcher =
+ matchers::matchesAnyListedName(IgnoredFunctions);
+
auto DescendantWithSideEffect =
- traverse(TK_AsIs, hasDescendant(expr(hasSideEffect(CheckFunctionCalls))));
+ traverse(TK_AsIs, hasDescendant(expr(hasSideEffect(
+ CheckFunctionCalls, IgnoredFunctionsMatcher))));
auto ConditionWithSideEffect = hasCondition(DescendantWithSideEffect);
Finder->addMatcher(
stmt(
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
index 15d1a69cb8cd0..c240f362e71e6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
@@ -42,6 +42,7 @@ class AssertSideEffectCheck : public ClangTidyCheck {
const bool CheckFunctionCalls;
const std::string RawAssertList;
SmallVector<StringRef, 5> AssertMacros;
+ const std::vector<std::string> IgnoredFunctions;
};
} // namespace bugprone
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index cb622f9b09606..0e9491eab9f6a 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -133,7 +133,7 @@ New checks
- New :doc:`readability-duplicate-include
<clang-tidy/checks/readability-duplicate-include>` check.
- Looks for duplicate includes and removes them.
+ Looks for duplicate includes and removes them.
- New :doc:`readability-identifier-length
<clang-tidy/checks/readability-identifier-length>` check.
@@ -167,7 +167,13 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- :doc:`bugprone-assert-side-effect <clang-tidy/checks/bugprone-assert-side-effect>`
+ check now supports an ``IgnoredFunctions`` option to explicitly consider
+ the specified semicolon-separated functions list as not having any
+ side-effects. Regular expressions for the list items are also accepted.
+
- Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
+ from :doc:`cppcoreguidelines-explicit-virtual-functions <clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions>`
to match the current state of the C++ Core Guidelines.
- Removed suggestion ``use gsl::at`` from warning message in the
@@ -185,10 +191,10 @@ Changes in existing checks
- Fixed a false positive in :doc:`bugprone-throw-keyword-missing
<clang-tidy/checks/bugprone-throw-keyword-missing>` when creating an exception object
- using placement new
+ using placement new.
- :doc:`cppcoreguidelines-narrowing-conversions <clang-tidy/checks/cppcoreguidelines-narrowing-conversions>`
- check now supports a `WarnOnIntegerToFloatingPointNarrowingConversion`
+ check now supports a ``WarnOnIntegerToFloatingPointNarrowingConversion``
option to control whether to warn on narrowing integer to floating-point
conversions.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
index dc7a3c9a4bd6c..8ba84ff61c6a9 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
@@ -21,3 +21,13 @@ Options
Whether to treat non-const member and non-member functions as they produce
side effects. Disabled by default because it can increase the number of false
positive warnings.
+
+.. option:: IgnoredFunctions
+
+ A semicolon-separated list of the names of functions or methods to be
+ considered as not having side-effects. Regular expressions are accepted,
+ e.g. `[Rr]ef(erence)?$` matches every type with suffix `Ref`, `ref`,
+ `Reference` and `reference`. The default is empty. If a name in the list
+ contains the sequence `::` it is matched against the qualified typename
+ (i.e. `namespace::Type`, otherwise it is matched against only
+ the type name (i.e. `Type`).
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
index 85f471f6e9eb1..c327007651d4c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}]}" -- -fexceptions
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: bugprone-assert-side-effect.IgnoredFunctions, value: 'MyClass::badButIgnoredFunc'}]}" -- -fexceptions
//===--- assert definition block ------------------------------------------===//
int abort() { return 0; }
@@ -43,9 +43,12 @@ void print(...);
//===----------------------------------------------------------------------===//
+bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
+
class MyClass {
public:
bool badFunc(int a, int b) { return a * b > 0; }
+ bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
bool goodFunc(int a, int b) const { return a * b > 0; }
MyClass &operator=(const MyClass &rhs) { return *this; }
@@ -57,6 +60,11 @@ class MyClass {
void operator delete(void *p) {}
};
+class SomeoneElseClass {
+public:
+ bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
+};
+
bool freeFunction() {
return true;
}
@@ -85,8 +93,16 @@ int main() {
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
MyClass mc;
+ SomeoneElseClass sec;
assert(mc.badFunc(0, 1));
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+ assert(mc.badButIgnoredFunc(0, 1));
+ // badButIgnoredFunc is not ignored as only class members are ignored by the config
+ assert(badButIgnoredFunc(0, 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+ // sec.badButIgnoredFunc is not ignored as only MyClass members are ignored by the config
+ assert(sec.badButIgnoredFunc(0, 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
assert(mc.goodFunc(0, 1));
MyClass mc2;
More information about the cfe-commits
mailing list