[clang-tools-extra] 2724507 - [clang-tidy] Model noexcept more properly in bugprone-exception-escape
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 17 08:59:51 PDT 2023
Author: Piotr Zegar
Date: 2023-07-17T15:59:34Z
New Revision: 27245077643ae8b94a0511ee1c3a39d6f4ca8076
URL: https://github.com/llvm/llvm-project/commit/27245077643ae8b94a0511ee1c3a39d6f4ca8076
DIFF: https://github.com/llvm/llvm-project/commit/27245077643ae8b94a0511ee1c3a39d6f4ca8076.diff
LOG: [clang-tidy] Model noexcept more properly in bugprone-exception-escape
During call stack analysis skip called noexcept functions
as they wont throw exceptions, they will crash.
Check will emit warnings for those functions separately.
Fixes: #43667, #49151, #51596, #54668, #54956
Reviewed By: carlosgalvezp
Differential Revision: https://reviews.llvm.org/D153458
Added:
Modified:
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
index c3e9a13bb1ba24..f1f1cc3b294d19 100644
--- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -316,6 +316,34 @@ bool isQualificationConvertiblePointer(QualType From, QualType To,
}
} // namespace
+static bool canThrow(const FunctionDecl *Func) {
+ const auto *FunProto = Func->getType()->getAs<FunctionProtoType>();
+ if (!FunProto)
+ return true;
+
+ switch (FunProto->canThrow()) {
+ case CT_Cannot:
+ return false;
+ case CT_Dependent: {
+ const Expr *NoexceptExpr = FunProto->getNoexceptExpr();
+ if (!NoexceptExpr)
+ return true; // no noexept - can throw
+
+ if (NoexceptExpr->isValueDependent())
+ return true; // depend on template - some instance can throw
+
+ bool Result = false;
+ if (!NoexceptExpr->EvaluateAsBooleanCondition(Result, Func->getASTContext(),
+ /*InConstantContext=*/true))
+ return true; // complex X condition in noexcept(X), cannot validate,
+ // assume that may throw
+ return !Result; // noexcept(false) - can throw
+ }
+ default:
+ return true;
+ };
+}
+
bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(
const Type *HandlerTy, const ASTContext &Context) {
llvm::SmallVector<const Type *, 8> TypesToDelete;
@@ -421,7 +449,7 @@ void ExceptionAnalyzer::ExceptionInfo::reevaluateBehaviour() {
ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
const FunctionDecl *Func, const ExceptionInfo::Throwables &Caught,
llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
- if (CallStack.count(Func))
+ if (!Func || CallStack.count(Func) || (!CallStack.empty() && !canThrow(Func)))
return ExceptionInfo::createNonThrowing();
if (const Stmt *Body = Func->getBody()) {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 820a76c66d9862..abe815e7cba756 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -275,7 +275,8 @@ Changes in existing checks
- Improved :doc:`bugprone-exception-escape
<clang-tidy/checks/bugprone/exception-escape>` to not emit warnings for
- forward declarations of functions.
+ 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
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 f575a185897c20..39cf5476eb6ea9 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
@@ -152,7 +152,7 @@ void throw_catch_multi_ptr_4() noexcept {
}
// FIXME: In this case 'a' is convertible to the handler and should be caught
-// but in reality it's thrown. Note that clang doesn't report a warning for
+// but in reality it's thrown. Note that clang doesn't report a warning for
// this either.
void throw_catch_multi_ptr_5() noexcept {
try {
@@ -249,7 +249,7 @@ void throw_derived_catch_base_alias() noexcept {
void throw_derived_catch_base_ptr_c() noexcept {
try {
derived d;
- throw &d;
+ throw &d;
} catch(const base *) {
}
}
@@ -259,7 +259,7 @@ void throw_derived_catch_base_ptr() noexcept {
try {
derived d;
const derived *p = &d;
- throw p;
+ throw p;
} catch(base *) {
}
}
@@ -282,7 +282,7 @@ void throw_derived_catch_base_private() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private' which should not throw exceptions
try {
B b;
- throw b;
+ throw b;
} catch(A) {
}
}
@@ -291,7 +291,7 @@ void throw_derived_catch_base_private_ptr() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private_ptr' which should not throw exceptions
try {
B b;
- throw &b;
+ throw &b;
} catch(A *) {
}
}
@@ -300,7 +300,7 @@ void throw_derived_catch_base_protected() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected' which should not throw exceptions
try {
C c;
- throw c;
+ throw c;
} catch(A) {
}
}
@@ -309,7 +309,7 @@ void throw_derived_catch_base_protected_ptr() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected_ptr' which should not throw exceptions
try {
C c;
- throw &c;
+ throw &c;
} catch(A *) {
}
}
@@ -318,7 +318,7 @@ void throw_derived_catch_base_ambiguous() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous' which should not throw exceptions
try {
E e;
- throw e;
+ throw e;
} catch(A) {
}
}
@@ -327,7 +327,7 @@ void throw_derived_catch_base_ambiguous_ptr() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous_ptr' which should not throw exceptions
try {
E e;
- throw e;
+ throw e;
} catch(A) {
}
}
@@ -703,3 +703,22 @@ int main() {
throw 1;
return 0;
}
+
+// The following function all incorrectly throw exceptions, *but* calling them
+// should not yield a warning because they are marked as noexcept.
+
+void test_basic_no_throw() noexcept { throw 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'test_basic_no_throw' which should not throw exceptions
+
+void test_basic_throw() noexcept(false) { throw 42; }
+
+void only_calls_non_throwing() noexcept {
+ test_basic_no_throw();
+}
+
+void calls_non_and_throwing() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'calls_non_and_throwing' which should not throw exceptions
+ test_basic_no_throw();
+ test_basic_throw();
+}
+
More information about the cfe-commits
mailing list