ngg created this revision.
ngg added reviewers: clang-tools-extra, njames93.
Herald added subscribers: carlosgalvezp, lebedev.ri, xazax.hun.
Herald added a reviewer: lebedev.ri.
Herald added a project: All.
ngg requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
If there is a function call that calls a non-throwing function,
this check should not report a warning, even if there is a throw
statement within that function.
The dependent_throw() test case was misleading and suggested that this
warning is not triggered when a throw exception is in unreachable code,
but in reality that warning did not trigger because it could not
determine whether the function should be noexcept without specializing
the T template argument. That test case is fixed to show non-specialized
and specialized cases.
Fixes https://github.com/llvm/llvm-project/issues/54668 and
https://github.com/llvm/llvm-project/issues/54956.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D134588
Files:
clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
clang-tools-extra/clang-tidy/openmp/ExceptionEscapeCheck.cpp
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -31,11 +31,21 @@
throw 1;
}
+void indirect_noexcept() noexcept {
+ // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_noexcept' which should not throw exceptions
+ throwing_noexcept();
+}
+
void throwing_throw_nothing() throw() {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_throw_nothing' which should not throw exceptions
throw 1;
}
+void indirect_throw_nothing() noexcept {
+ // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_throw_nothing' which should not throw exceptions
+ throwing_throw_nothing();
+}
+
void throw_and_catch() noexcept {
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch' which should not throw exceptions
try {
@@ -200,8 +210,19 @@
template<typename T>
void dependent_throw() noexcept(sizeof(T)<4) {
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'dependent_throw' which should not throw exceptions
- if (sizeof(T) > 4)
- throw 1;
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:6: warning: an exception may be thrown in function 'dependent_throw<int>' which should not throw exceptions
+ // CHECK-MESSAGES: :[[@LINE-3]]:6: warning: an exception may be thrown in function 'dependent_throw<char>' which should not throw exceptions
+ throw 1;
+}
+
+void indirect_dependent_noexcept_true() noexcept {
+ // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_dependent_noexcept_true' which should not throw exceptions
+ dependent_throw<char>();
+}
+
+void indirect_dependent_noexcept_false() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_dependent_noexcept_false' which should not throw exceptions
+ dependent_throw<int>();
}
void swap(int&, int&) {
Index: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
+++ clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
@@ -128,21 +128,24 @@
IgnoredExceptions = std::move(ExceptionNames);
}
- ExceptionInfo analyze(const FunctionDecl *Func);
- ExceptionInfo analyze(const Stmt *Stmt);
+ ExceptionInfo analyze(const FunctionDecl *Func, ASTContext &Context);
+ ExceptionInfo analyze(const Stmt *Stmt, ASTContext &Context);
private:
ExceptionInfo
throwsException(const FunctionDecl *Func,
- llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
+ llvm::SmallSet<const FunctionDecl *, 32> &CallStack,
+ ASTContext &Context);
ExceptionInfo
throwsException(const Stmt *St, const ExceptionInfo::Throwables &Caught,
- llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
+ llvm::SmallSet<const FunctionDecl *, 32> &CallStack,
+ ASTContext &Context);
- ExceptionInfo analyzeImpl(const FunctionDecl *Func);
- ExceptionInfo analyzeImpl(const Stmt *Stmt);
+ ExceptionInfo analyzeImpl(const FunctionDecl *Func, ASTContext &Context);
+ ExceptionInfo analyzeImpl(const Stmt *Stmt, ASTContext &Context);
- template <typename T> ExceptionInfo analyzeDispatch(const T *Node);
+ template <typename T>
+ ExceptionInfo analyzeDispatch(const T *Node, ASTContext &Context);
bool IgnoreBadAlloc = true;
llvm::StringSet<> IgnoredExceptions;
Index: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -12,6 +12,8 @@
namespace tidy {
namespace utils {
+using namespace ast_matchers;
+
void ExceptionAnalyzer::ExceptionInfo::registerException(
const Type *ExceptionType) {
assert(ExceptionType != nullptr && "Only valid types are accepted");
@@ -111,20 +113,22 @@
ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
const FunctionDecl *Func,
- llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
- if (CallStack.count(Func))
+ llvm::SmallSet<const FunctionDecl *, 32> &CallStack, ASTContext &Context) {
+ if (CallStack.count(Func) ||
+ (!CallStack.empty() &&
+ !match(functionDecl(isNoThrow()), *Func, Context).empty()))
return ExceptionInfo::createNonThrowing();
if (const Stmt *Body = Func->getBody()) {
CallStack.insert(Func);
ExceptionInfo Result =
- throwsException(Body, ExceptionInfo::Throwables(), CallStack);
+ throwsException(Body, ExceptionInfo::Throwables(), CallStack, Context);
// For a constructor, we also have to check the initializers.
if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(Func)) {
for (const CXXCtorInitializer *Init : Ctor->inits()) {
ExceptionInfo Excs = throwsException(
- Init->getInit(), ExceptionInfo::Throwables(), CallStack);
+ Init->getInit(), ExceptionInfo::Throwables(), CallStack, Context);
Result.merge(Excs);
}
}
@@ -145,7 +149,7 @@
/// possible except some 'Unknown' functions are called.
ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
const Stmt *St, const ExceptionInfo::Throwables &Caught,
- llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
+ llvm::SmallSet<const FunctionDecl *, 32> &CallStack, ASTContext &Context) {
auto Results = ExceptionInfo::createNonThrowing();
if (!St)
return Results;
@@ -167,14 +171,15 @@
Results.registerExceptions(Caught);
} else if (const auto *Try = dyn_cast<CXXTryStmt>(St)) {
ExceptionInfo Uncaught =
- throwsException(Try->getTryBlock(), Caught, CallStack);
+ throwsException(Try->getTryBlock(), Caught, CallStack, Context);
for (unsigned I = 0; I < Try->getNumHandlers(); ++I) {
const CXXCatchStmt *Catch = Try->getHandler(I);
// Everything is catched through 'catch(...)'.
if (!Catch->getExceptionDecl()) {
- ExceptionInfo Rethrown = throwsException(
- Catch->getHandlerBlock(), Uncaught.getExceptionTypes(), CallStack);
+ ExceptionInfo Rethrown =
+ throwsException(Catch->getHandlerBlock(),
+ Uncaught.getExceptionTypes(), CallStack, Context);
Results.merge(Rethrown);
Uncaught.clear();
} else {
@@ -193,8 +198,8 @@
if (Uncaught.filterByCatch(CaughtType)) {
ExceptionInfo::Throwables CaughtExceptions;
CaughtExceptions.insert(CaughtType);
- ExceptionInfo Rethrown = throwsException(Catch->getHandlerBlock(),
- CaughtExceptions, CallStack);
+ ExceptionInfo Rethrown = throwsException(
+ Catch->getHandlerBlock(), CaughtExceptions, CallStack, Context);
Results.merge(Rethrown);
}
}
@@ -202,20 +207,20 @@
Results.merge(Uncaught);
} else if (const auto *Call = dyn_cast<CallExpr>(St)) {
if (const FunctionDecl *Func = Call->getDirectCallee()) {
- ExceptionInfo Excs = throwsException(Func, CallStack);
+ ExceptionInfo Excs = throwsException(Func, CallStack, Context);
Results.merge(Excs);
}
} else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) {
ExceptionInfo Excs =
- throwsException(Construct->getConstructor(), CallStack);
+ throwsException(Construct->getConstructor(), CallStack, Context);
Results.merge(Excs);
} else if (const auto *DefaultInit = dyn_cast<CXXDefaultInitExpr>(St)) {
ExceptionInfo Excs =
- throwsException(DefaultInit->getExpr(), Caught, CallStack);
+ throwsException(DefaultInit->getExpr(), Caught, CallStack, Context);
Results.merge(Excs);
} else {
for (const Stmt *Child : St->children()) {
- ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
+ ExceptionInfo Excs = throwsException(Child, Caught, CallStack, Context);
Results.merge(Excs);
}
}
@@ -223,13 +228,13 @@
}
ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyzeImpl(const FunctionDecl *Func) {
+ExceptionAnalyzer::analyzeImpl(const FunctionDecl *Func, ASTContext &Context) {
ExceptionInfo ExceptionList;
// Check if the function has already been analyzed and reuse that result.
if (FunctionCache.count(Func) == 0) {
llvm::SmallSet<const FunctionDecl *, 32> CallStack;
- ExceptionList = throwsException(Func, CallStack);
+ ExceptionList = throwsException(Func, CallStack, Context);
// Cache the result of the analysis. This is done prior to filtering
// because it is best to keep as much information as possible.
@@ -243,15 +248,15 @@
}
ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyzeImpl(const Stmt *Stmt) {
+ExceptionAnalyzer::analyzeImpl(const Stmt *Stmt, ASTContext &Context) {
llvm::SmallSet<const FunctionDecl *, 32> CallStack;
- return throwsException(Stmt, ExceptionInfo::Throwables(), CallStack);
+ return throwsException(Stmt, ExceptionInfo::Throwables(), CallStack, Context);
}
template <typename T>
ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyzeDispatch(const T *Node) {
- ExceptionInfo ExceptionList = analyzeImpl(Node);
+ExceptionAnalyzer::analyzeDispatch(const T *Node, ASTContext &Context) {
+ ExceptionInfo ExceptionList = analyzeImpl(Node, Context);
if (ExceptionList.getBehaviour() == State::NotThrowing ||
ExceptionList.getBehaviour() == State::Unknown)
@@ -265,13 +270,13 @@
}
ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
- return analyzeDispatch(Func);
+ExceptionAnalyzer::analyze(const FunctionDecl *Func, ASTContext &Context) {
+ return analyzeDispatch(Func, Context);
}
ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyze(const Stmt *Stmt) {
- return analyzeDispatch(Stmt);
+ExceptionAnalyzer::analyze(const Stmt *Stmt, ASTContext &Context) {
+ return analyzeDispatch(Stmt, Context);
}
} // namespace utils
Index: clang-tools-extra/clang-tidy/openmp/ExceptionEscapeCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/openmp/ExceptionEscapeCheck.cpp
+++ clang-tools-extra/clang-tidy/openmp/ExceptionEscapeCheck.cpp
@@ -58,7 +58,7 @@
Result.Nodes.getNodeAs<Stmt>("structured-block");
assert(StructuredBlock && "Expected to get some OpenMP Structured Block.");
- if (Tracer.analyze(StructuredBlock).getBehaviour() !=
+ if (Tracer.analyze(StructuredBlock, *Result.Context).getBehaviour() !=
utils::ExceptionAnalyzer::State::Throwing)
return; // No exceptions have been proven to escape out of the struc. block.
Index: clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -68,7 +68,7 @@
if (!MatchedDecl)
return;
- if (Tracer.analyze(MatchedDecl).getBehaviour() ==
+ if (Tracer.analyze(MatchedDecl, *Result.Context).getBehaviour() ==
utils::ExceptionAnalyzer::State::Throwing)
// FIXME: We should provide more information about the exact location where
// the exception is thrown, maybe the full path the exception escapes
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
