zinovy.nis updated this revision to Diff 400301.
zinovy.nis added a comment.
- Support regexp for list of ignored functions.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116478/new/
https://reviews.llvm.org/D116478
Files:
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
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
+++ 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 @@
//===----------------------------------------------------------------------===//
+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 @@
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 @@
// 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;
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
@@ -21,3 +21,13 @@
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`).
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -153,7 +153,12 @@
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- :doc:`bugprone-assert-side-effect <clang-tidy/checks/bugprone-assert-side-effect>`
+ check now supports a ``IgnoredFunctions`` option to explicitly consider the specified
+ functions or methods as not any having side-effects.
+
- 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.
- Updated :doc:`google-readability-casting
@@ -165,10 +170,10 @@
- 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.
Index: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
@@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ASSERTSIDEEFFECTCHECK_H
#include "../ClangTidyCheck.h"
+#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include <string>
@@ -42,6 +43,7 @@
const bool CheckFunctionCalls;
const std::string RawAssertList;
SmallVector<StringRef, 5> AssertMacros;
+ const std::vector<std::string> IgnoredFunctions;
};
} // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ 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,10 @@
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 +60,8 @@
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 +78,9 @@
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 +88,17 @@
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(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
