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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
