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 @@ -42,6 +42,7 @@ const bool CheckFunctionCalls; const std::string RawAssertList; SmallVector AssertMacros; + const std::vector 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,9 @@ namespace { -AST_MATCHER_P(Expr, hasSideEffect, bool, CheckFunctionCalls) { +AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls, + clang::ast_matchers::internal::Matcher, + IgnoredFunctionsMatcher) { const Expr *E = &Node; if (const auto *Op = dyn_cast(E)) { @@ -55,7 +59,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(FuncDecl)) Result &= !MethodDecl->isConst(); @@ -72,8 +77,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 +87,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( Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -156,7 +156,13 @@ Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- :doc:`bugprone-assert-side-effect ` + check now supports an ``IgnoredFunctions`` option to explicitly consider + the specified semicolon-separated functions list as not any having + 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 ` to match the current state of the C++ Core Guidelines. - Updated :doc:`google-readability-casting @@ -168,10 +174,10 @@ - Fixed a false positive in :doc:`bugprone-throw-keyword-missing ` when creating an exception object - using placement new + using placement new. - :doc:`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/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/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;