[clang-tidy] Add fine-graded configuration for 'bugprone-exception-escape'

[vbvictor]

Need these options to complete #160825, but I think it's generally beneficial to fine-tune this check.

[llvmbot]

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Baranov Victor (vbvictor)

Changes

Need these options to complete #160825.

---

Full diff: https://github.com/llvm/llvm-project/pull/164081.diff

5 Files Affected:

• (modified) clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp (+23-7)
• (modified) clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.h (+7)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3-1)
• (modified) clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst (+25)
• (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-options.cpp (+49)

diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp index 837a86ff8655e..02baa1f72a6d2 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp @@ -36,13 +36,22 @@ ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), RawFunctionsThatShouldNotThrow(Options.get( "FunctionsThatShouldNotThrow", "")), - RawIgnoredExceptions(Options.get("IgnoredExceptions", "")) { + RawIgnoredExceptions(Options.get("IgnoredExceptions", "")), + RawCheckedSwapFunctions( + Options.get("CheckedSwapFunctions", "swap,iter_swap,iter_move")), + CheckDestructors(Options.get("CheckDestructors", true)), + CheckMoveMemberFunctions(Options.get("CheckMoveMemberFunctions", true)), + CheckMain(Options.get("CheckMain", true)), + CheckNothrowFunctions(Options.get("CheckNothrowFunctions", true)) { llvm::SmallVector<StringRef, 8> FunctionsThatShouldNotThrowVec, - IgnoredExceptionsVec; + IgnoredExceptionsVec, CheckedSwapFunctionsVec; RawFunctionsThatShouldNotThrow.split(FunctionsThatShouldNotThrowVec, ",", -1, false); FunctionsThatShouldNotThrow.insert_range(FunctionsThatShouldNotThrowVec); + RawCheckedSwapFunctions.split(CheckedSwapFunctionsVec, ",", -1, false); + CheckedSwapFunctions.insert_range(CheckedSwapFunctionsVec); + llvm::StringSet<> IgnoredExceptions; RawIgnoredExceptions.split(IgnoredExceptionsVec, ",", -1, false); IgnoredExceptions.insert_range(IgnoredExceptionsVec); @@ -54,17 +63,24 @@ void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "FunctionsThatShouldNotThrow", RawFunctionsThatShouldNotThrow); Options.store(Opts, "IgnoredExceptions", RawIgnoredExceptions); + Options.store(Opts, "CheckedSwapFunctions", RawCheckedSwapFunctions); + Options.store(Opts, "CheckDestructors", CheckDestructors); + Options.store(Opts, "CheckMoveMemberFunctions", CheckMoveMemberFunctions); + Options.store(Opts, "CheckMain", CheckMain); + Options.store(Opts, "CheckNothrowFunctions", CheckNothrowFunctions); } void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) { + ast_matchers::internal::Matcher<FunctionDecl> Nothing = unless(anything()); Finder->addMatcher( functionDecl( isDefinition(), - anyOf(isNoThrow(), - allOf(anyOf(cxxDestructorDecl(), - cxxConstructorDecl(isMoveConstructor()), - cxxMethodDecl(isMoveAssignmentOperator()), isMain(), - allOf(hasAnyName("swap", "iter_swap", "iter_move"), + anyOf(CheckNothrowFunctions ? isNoThrow() : Nothing, + allOf(anyOf(CheckDestructors ? cxxDestructorDecl() : Nothing, + CheckMoveMemberFunctions ? anyOf(cxxConstructorDecl(isMoveConstructor()), + cxxMethodDecl(isMoveAssignmentOperator())) : Nothing,  + CheckMain ? isMain() : Nothing, + allOf(isEnabled(CheckedSwapFunctions), hasAtLeastOneParameter())), unless(isExplicitThrow())), isEnabled(FunctionsThatShouldNotThrow))) diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.h b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.h index bd1e7bae57f5d..db5e71901962e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.h @@ -35,8 +35,15 @@ class ExceptionEscapeCheck : public ClangTidyCheck { private: StringRef RawFunctionsThatShouldNotThrow; StringRef RawIgnoredExceptions; + StringRef RawCheckedSwapFunctions; + + const bool CheckDestructors; + const bool CheckMoveMemberFunctions; + const bool CheckMain; + const bool CheckNothrowFunctions; llvm::StringSet<> FunctionsThatShouldNotThrow; + llvm::StringSet<> CheckedSwapFunctions; utils::ExceptionAnalyzer Tracer; }; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a94dd9737468c..c5247074244d2 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -260,7 +260,9 @@ Changes in existing checks - Improved :doc:`bugprone-exception-escape <clang-tidy/checks/bugprone/exception-escape>` check's handling of lambdas: exceptions from captures are now diagnosed, exceptions in the bodies of - lambdas that aren't actually invoked are not. + lambdas that aren't actually invoked are not. Added fine-grained configuration + via options `CheckDestructors`, `CheckMoveMemberFunctions`, `CheckMain`, + `CheckedSwapFunctions`, and `CheckNothrowFunctions`. - Improved :doc:`bugprone-infinite-loop <clang-tidy/checks/bugprone/infinite-loop>` check by adding detection for diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst index 182fade7f47a0..07abbedd109b0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst @@ -35,6 +35,31 @@ WARNING! This check may be expensive on large source files. Options ------- +.. option:: CheckDestructors + + When `true`, destructors are analyzed to not throw exceptions. + Default value is `true`. + +.. option:: CheckMoveMemberFunctions + + When `true`, move constructors and move assignment operators are analyzed + to not throw exceptions. Default value is `true`. + +.. option:: CheckMain + + When `true`, ``main()`` function is analyzed to not throw exceptions. + Default value is `true`. + +.. option:: CheckNothrowFunctions + + When `true`, functions marked with ``noexcept`` or ``throw()`` exception + specifications are analyzed to not throw exceptions. Default value is `true`. + +.. option:: CheckedSwapFunctions + + Comma separated list of swap function names which should not throw exceptions. + Default value is `swap,iter_swap,iter_move`. + .. option:: FunctionsThatShouldNotThrow Comma separated list containing function names which should not throw. An diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-options.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-options.cpp new file mode 100644 index 0000000000000..b448770561dbb --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-options.cpp @@ -0,0 +1,49 @@ +// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-exception-escape %t -- \ +// RUN: -config="{CheckOptions: { \ +// RUN: bugprone-exception-escape.CheckDestructors: false, \ +// RUN: bugprone-exception-escape.CheckMoveMemberFunctions: false, \ +// RUN: bugprone-exception-escape.CheckMain: false, \ +// RUN: bugprone-exception-escape.CheckedSwapFunctions: '', \ +// RUN: bugprone-exception-escape.CheckNothrowFunctions: false \ +// RUN: }}" \ +// RUN: -- -fexceptions + +// CHECK-MESSAGES-NOT: warning: + +struct destructor { + ~destructor() { + throw 1; + } +}; + +struct move { + move(const move&) { throw 42; } + move(move&&) { throw 42; } + move& operator=(const move&) { throw 42; } + move& operator=(move&&) { throw 42; } +}; + +void swap(int&, int&) { + throw 1; +} + +void iter_swap(int&, int&) { + throw 1; +} + +void iter_move(int&) { + throw 1; +} + +void nothrow_func() throw() { + throw 1; +} + +void noexcept_func() noexcept { + throw 1; +} + +int main() { + throw 1; + return 0; +} 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[vbvictor]

This is not ideal, we better use hasAnyName with ArrayRef<StringRef> but for now I keep uniform with existing style.

Will refactor later.

[vbvictor]

Ping, should be easy NFC-ish patch

[zwuis]

LGTM modulo one comment.