Author: Piotr Zegar Date: 2023-07-18T21:11:08Z New Revision: ff40c34881772e0641027327cd1791f4acacf6ff
URL: https://github.com/llvm/llvm-project/commit/ff40c34881772e0641027327cd1791f4acacf6ff DIFF: https://github.com/llvm/llvm-project/commit/ff40c34881772e0641027327cd1791f4acacf6ff.diff LOG: [clang-tidy] Allow explicit throwing in bugprone-exception-escape for special functions Functions declared explicitly with noexcept(false) or throw(exception) will be excluded from the analysis, as even though it is not recommended for functions like swap, main, move constructors and assignment operators, and destructors, it is a clear indication of the developer's intention and should be respected. Fixes: #40583, #55143 Reviewed By: carlosgalvezp Differential Revision: https://reviews.llvm.org/D153423 Added: Modified: clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp clang/include/clang/Basic/ExceptionSpecificationType.h Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp index 635cc2ca05d120..90bf523ffb00b6 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp @@ -15,15 +15,21 @@ using namespace clang::ast_matchers; -namespace clang { +namespace clang::tidy::bugprone { namespace { + AST_MATCHER_P(FunctionDecl, isEnabled, llvm::StringSet<>, FunctionsThatShouldNotThrow) { return FunctionsThatShouldNotThrow.count(Node.getNameAsString()) > 0; } + +AST_MATCHER(FunctionDecl, isExplicitThrow) { + return isExplicitThrowExceptionSpec(Node.getExceptionSpecType()) && + Node.getExceptionSpecSourceRange().isValid(); +} + } // namespace -namespace tidy::bugprone { ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), RawFunctionsThatShouldNotThrow(Options.get( @@ -53,10 +59,12 @@ void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( functionDecl(isDefinition(), - anyOf(isNoThrow(), cxxDestructorDecl(), - cxxConstructorDecl(isMoveConstructor()), - cxxMethodDecl(isMoveAssignmentOperator()), - hasName("main"), hasName("swap"), + anyOf(isNoThrow(), + allOf(anyOf(cxxDestructorDecl(), + cxxConstructorDecl(isMoveConstructor()), + cxxMethodDecl(isMoveAssignmentOperator()), + isMain(), hasName("swap")), + unless(isExplicitThrow())), isEnabled(FunctionsThatShouldNotThrow))) .bind("thrower"), this); @@ -77,5 +85,4 @@ void ExceptionEscapeCheck::check(const MatchFinder::MatchResult &Result) { << MatchedDecl; } -} // namespace tidy::bugprone -} // namespace clang +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index abe815e7cba756..1c542d4c9f2f30 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -274,13 +274,10 @@ Changes in existing checks Global options of the same name should be used instead. - Improved :doc:`bugprone-exception-escape - <clang-tidy/checks/bugprone/exception-escape>` to not emit warnings for - forward declarations of functions and additionally modified it to skip - ``noexcept`` functions during call stack analysis. - -- Fixed :doc:`bugprone-exception-escape<clang-tidy/checks/bugprone/exception-escape>` - for coroutines where previously a warning would be emitted with coroutines - throwing exceptions in their bodies. + <clang-tidy/checks/bugprone/exception-escape>` check to not emit warnings for + forward declarations of functions, explicitly declared throwing functions, + coroutines throwing exceptions in their bodies and skip ``noexcept`` + functions during call stack analysis. - Improved :doc:`bugprone-fold-init-type <clang-tidy/checks/bugprone/fold-init-type>` to handle iterators that do not 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 52f3ceff281494..e6aa8e001492a6 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 @@ -22,6 +22,12 @@ are always possible to implement in a non throwing way. Non throwing ``swap()`` operations are also used to create move operations. A throwing ``main()`` function also results in unexpected termination. +Functions declared explicitly with ``noexcept(false)`` or ``throw(exception)`` +will be excluded from the analysis, as even though it is not recommended for +functions like ``swap()``, ``main()``, move constructors, move assignment operators +and destructors, it is a clear indication of the developer's intention and +should be respected. + WARNING! This check may be expensive on large source files. Options diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp index 2e748a2374bf9e..222577b124dced 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp @@ -183,7 +183,6 @@ struct Promise<Task, void, ThrowInPromiseConstructor, ThrowInInitialSuspend, struct Evil { ~Evil() noexcept(false) { - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function '~Evil' which should not throw exceptions throw 42; } }; diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp index 39cf5476eb6ea9..7099545fce15bb 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp @@ -722,3 +722,27 @@ void calls_non_and_throwing() noexcept { test_basic_throw(); } +namespace PR55143 { namespace PR40583 { + +struct test_explicit_throw { + test_explicit_throw() throw(int) { throw 42; } + test_explicit_throw(const test_explicit_throw&) throw(int) { throw 42; } + test_explicit_throw(test_explicit_throw&&) throw(int) { throw 42; } + test_explicit_throw& operator=(const test_explicit_throw&) throw(int) { throw 42; } + test_explicit_throw& operator=(test_explicit_throw&&) throw(int) { throw 42; } + ~test_explicit_throw() throw(int) { throw 42; } +}; + +struct test_implicit_throw { + test_implicit_throw() { throw 42; } + test_implicit_throw(const test_implicit_throw&) { throw 42; } + test_implicit_throw(test_implicit_throw&&) { throw 42; } + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'test_implicit_throw' which should not throw exceptions + test_implicit_throw& operator=(const test_implicit_throw&) { throw 42; } + test_implicit_throw& operator=(test_implicit_throw&&) { throw 42; } + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: an exception may be thrown in function 'operator=' which should not throw exceptions + ~test_implicit_throw() { throw 42; } + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function '~test_implicit_throw' which should not throw exceptions +}; + +}} diff --git a/clang/include/clang/Basic/ExceptionSpecificationType.h b/clang/include/clang/Basic/ExceptionSpecificationType.h index 5616860555c8a4..d3c9e9cd063b42 100644 --- a/clang/include/clang/Basic/ExceptionSpecificationType.h +++ b/clang/include/clang/Basic/ExceptionSpecificationType.h @@ -50,6 +50,11 @@ inline bool isUnresolvedExceptionSpec(ExceptionSpecificationType ESpecType) { return ESpecType == EST_Unevaluated || ESpecType == EST_Uninstantiated; } +inline bool isExplicitThrowExceptionSpec(ExceptionSpecificationType ESpecType) { + return ESpecType == EST_Dynamic || ESpecType == EST_MSAny || + ESpecType == EST_NoexceptFalse; +} + /// Possible results from evaluation of a noexcept expression. enum CanThrowResult { CT_Cannot, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
