[clang-tidy] Add options to throw unannotated functions in bugprone-exception-escape
#168324
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…exception-escape`
Member
|
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: mitchell (zeyi2) ChangesCloses #164795 Full diff: https://github.com/llvm/llvm-project/pull/168324.diff 8 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
index b7de8395ffa05..b2415b59b135d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -42,7 +42,10 @@ ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name,
CheckDestructors(Options.get("CheckDestructors", true)),
CheckMoveMemberFunctions(Options.get("CheckMoveMemberFunctions", true)),
CheckMain(Options.get("CheckMain", true)),
- CheckNothrowFunctions(Options.get("CheckNothrowFunctions", true)) {
+ CheckNothrowFunctions(Options.get("CheckNothrowFunctions", true)),
+ KnownUnannotatedAsThrowing(
+ Options.get("KnownUnannotatedAsThrowing", false)),
+ UnknownAsThrowing(Options.get("UnknownAsThrowing", false)) {
llvm::SmallVector<StringRef, 8> FunctionsThatShouldNotThrowVec,
IgnoredExceptionsVec, CheckedSwapFunctionsVec;
RawFunctionsThatShouldNotThrow.split(FunctionsThatShouldNotThrowVec, ",", -1,
@@ -57,6 +60,7 @@ ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name,
IgnoredExceptions.insert_range(IgnoredExceptionsVec);
Tracer.ignoreExceptions(std::move(IgnoredExceptions));
Tracer.ignoreBadAlloc(true);
+ Tracer.assumeUnannotatedFunctionsThrow(KnownUnannotatedAsThrowing);
}
void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
@@ -68,6 +72,8 @@ void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "CheckMoveMemberFunctions", CheckMoveMemberFunctions);
Options.store(Opts, "CheckMain", CheckMain);
Options.store(Opts, "CheckNothrowFunctions", CheckNothrowFunctions);
+ Options.store(Opts, "KnownUnannotatedAsThrowing", KnownUnannotatedAsThrowing);
+ Options.store(Opts, "UnknownAsThrowing", UnknownAsThrowing);
}
void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) {
@@ -103,41 +109,53 @@ void ExceptionEscapeCheck::check(const MatchFinder::MatchResult &Result) {
const utils::ExceptionAnalyzer::ExceptionInfo Info =
Tracer.analyze(MatchedDecl);
- if (Info.getBehaviour() != utils::ExceptionAnalyzer::State::Throwing)
- return;
-
- diag(MatchedDecl->getLocation(), "an exception may be thrown in function "
- "%0 which should not throw exceptions")
- << MatchedDecl;
+ const auto Behaviour = Info.getBehaviour();
+ const bool IsThrowing =
+ Behaviour == utils::ExceptionAnalyzer::State::Throwing;
+ const bool IsUnknown = Behaviour == utils::ExceptionAnalyzer::State::Unknown;
- const auto &[ThrowType, ThrowInfo] = *Info.getExceptions().begin();
+ const bool ReportUnknown =
+ IsUnknown &&
+ ((KnownUnannotatedAsThrowing && Info.hasUnknownFromKnownUnannotated()) ||
+ (UnknownAsThrowing && Info.hasUnknownFromMissingDefinition()));
- if (ThrowInfo.Loc.isInvalid())
+ if (!(IsThrowing || ReportUnknown))
return;
- const utils::ExceptionAnalyzer::CallStack &Stack = ThrowInfo.Stack;
- diag(ThrowInfo.Loc,
- "frame #0: unhandled exception of type %0 may be thrown in function %1 "
- "here",
- DiagnosticIDs::Note)
- << QualType(ThrowType, 0U) << Stack.back().first;
-
- size_t FrameNo = 1;
- for (auto CurrIt = ++Stack.rbegin(), PrevIt = Stack.rbegin();
- CurrIt != Stack.rend(); ++CurrIt, ++PrevIt) {
- const FunctionDecl *CurrFunction = CurrIt->first;
- const FunctionDecl *PrevFunction = PrevIt->first;
- const SourceLocation PrevLocation = PrevIt->second;
- if (PrevLocation.isValid()) {
- diag(PrevLocation, "frame #%0: function %1 calls function %2 here",
- DiagnosticIDs::Note)
- << FrameNo << CurrFunction << PrevFunction;
- } else {
- diag(CurrFunction->getLocation(),
- "frame #%0: function %1 calls function %2", DiagnosticIDs::Note)
- << FrameNo << CurrFunction << PrevFunction;
+ diag(MatchedDecl->getLocation(), "an exception may be thrown in function %0 "
+ "which should not throw exceptions")
+ << MatchedDecl;
+
+ if (!Info.getExceptions().empty()) {
+ const auto &[ThrowType, ThrowInfo] = *Info.getExceptions().begin();
+
+ if (ThrowInfo.Loc.isInvalid())
+ return;
+
+ const utils::ExceptionAnalyzer::CallStack &Stack = ThrowInfo.Stack;
+ diag(ThrowInfo.Loc,
+ "frame #0: unhandled exception of type %0 may be thrown in function "
+ "%1 here",
+ DiagnosticIDs::Note)
+ << QualType(ThrowType, 0U) << Stack.back().first;
+
+ size_t FrameNo = 1;
+ for (auto CurrIt = ++Stack.rbegin(), PrevIt = Stack.rbegin();
+ CurrIt != Stack.rend(); ++CurrIt, ++PrevIt) {
+ const FunctionDecl *CurrFunction = CurrIt->first;
+ const FunctionDecl *PrevFunction = PrevIt->first;
+ const SourceLocation PrevLocation = PrevIt->second;
+ if (PrevLocation.isValid()) {
+ diag(PrevLocation, "frame #%0: function %1 calls function %2 here",
+ DiagnosticIDs::Note)
+ << FrameNo << CurrFunction << PrevFunction;
+ } else {
+ diag(CurrFunction->getLocation(),
+ "frame #%0: function %1 calls function %2", DiagnosticIDs::Note)
+ << FrameNo << CurrFunction << PrevFunction;
+ }
+ ++FrameNo;
}
- ++FrameNo;
}
}
diff --git a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.h b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.h
index c3bf4a4335273..ba65640435368 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.h
@@ -42,6 +42,9 @@ class ExceptionEscapeCheck : public ClangTidyCheck {
const bool CheckMain;
const bool CheckNothrowFunctions;
+ const bool KnownUnannotatedAsThrowing;
+ const bool UnknownAsThrowing;
+
llvm::StringSet<> FunctionsThatShouldNotThrow;
llvm::StringSet<> CheckedSwapFunctions;
utils::ExceptionAnalyzer Tracer;
diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
index c774f54b1da5a..221f7711786de 100644
--- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -39,6 +39,10 @@ ExceptionAnalyzer::ExceptionInfo &ExceptionAnalyzer::ExceptionInfo::merge(
Behaviour = State::Unknown;
ContainsUnknown = ContainsUnknown || Other.ContainsUnknown;
+ UnknownFromMissingDefinition =
+ UnknownFromMissingDefinition || Other.UnknownFromMissingDefinition;
+ UnknownFromKnownUnannotated =
+ UnknownFromKnownUnannotated || Other.UnknownFromKnownUnannotated;
ThrownExceptions.insert_range(Other.ThrownExceptions);
return *this;
}
@@ -484,10 +488,19 @@ ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
}
CallStack.erase(Func);
+ // Optionally treat unannotated functions as potentially throwing if they
+ // are not explicitly non-throwing and no throw was discovered.
+ if (AssumeUnannotatedThrowing &&
+ Result.getBehaviour() == State::NotThrowing && canThrow(Func)) {
+ auto Unknown = ExceptionInfo::createUnknown();
+ Unknown.markUnknownFromKnownUnannotated();
+ return Unknown;
+ }
return Result;
}
auto Result = ExceptionInfo::createUnknown();
+ Result.markUnknownFromMissingDefinition();
if (const auto *FPT = Func->getType()->getAs<FunctionProtoType>()) {
for (const QualType &Ex : FPT->exceptions()) {
CallStack.insert({Func, CallLoc});
diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
index 1a277c8a6d3b2..c0356b71383fb 100644
--- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
@@ -52,7 +52,7 @@ class ExceptionAnalyzer {
using Throwables = llvm::SmallDenseMap<const Type *, ThrowInfo, 2>;
static ExceptionInfo createUnknown() { return {State::Unknown}; }
- static ExceptionInfo createNonThrowing() { return {State::Throwing}; }
+ static ExceptionInfo createNonThrowing() { return {State::NotThrowing}; }
/// By default the exception situation is unknown and must be
/// clarified step-wise.
@@ -67,6 +67,22 @@ class ExceptionAnalyzer {
State getBehaviour() const { return Behaviour; }
+ /// Unknown cause tracking.
+ void markUnknownFromMissingDefinition() {
+ UnknownFromMissingDefinition = true;
+ ContainsUnknown = true;
+ }
+ void markUnknownFromKnownUnannotated() {
+ UnknownFromKnownUnannotated = true;
+ ContainsUnknown = true;
+ }
+ bool hasUnknownFromMissingDefinition() const {
+ return UnknownFromMissingDefinition;
+ }
+ bool hasUnknownFromKnownUnannotated() const {
+ return UnknownFromKnownUnannotated;
+ }
+
/// Register a single exception type as recognized potential exception to be
/// thrown.
void registerException(const Type *ExceptionType,
@@ -124,12 +140,20 @@ class ExceptionAnalyzer {
/// after filtering.
bool ContainsUnknown;
+ bool UnknownFromMissingDefinition = false;
+ bool UnknownFromKnownUnannotated = false;
+
/// 'ThrownException' is empty if the 'Behaviour' is either 'NotThrowing' or
/// 'Unknown'.
Throwables ThrownExceptions;
};
ExceptionAnalyzer() = default;
+ /// When enabled, treat any function that is not explicitly non-throwing
+ /// as potentially throwing, even if its body analysis finds no throw.
+ void assumeUnannotatedFunctionsThrow(bool Enable) {
+ AssumeUnannotatedThrowing = Enable;
+ }
void ignoreBadAlloc(bool ShallIgnore) { IgnoreBadAlloc = ShallIgnore; }
void ignoreExceptions(llvm::StringSet<> ExceptionNames) {
@@ -154,6 +178,7 @@ class ExceptionAnalyzer {
bool IgnoreBadAlloc = true;
llvm::StringSet<> IgnoredExceptions;
llvm::DenseMap<const FunctionDecl *, ExceptionInfo> FunctionCache{32U};
+ bool AssumeUnannotatedThrowing = false;
};
} // namespace clang::tidy::utils
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b982216297919..d671efa8b388f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -328,7 +328,11 @@ Changes in existing checks
where the check wouldn't diagnose throws in arguments to functions or
constructors. Added fine-grained configuration via options
`CheckDestructors`, `CheckMoveMemberFunctions`, `CheckMain`,
- `CheckedSwapFunctions`, and `CheckNothrowFunctions`.
+ `CheckedSwapFunctions`, and `CheckNothrowFunctions`; and added
+ ``KnownUnannotatedAsThrowing`` and ``UnknownAsThrowing`` to support
+ reporting for unannotated functions, enabling reporting when no explicit
+ ``throw`` is seen and allowing separate tuning for known and unknown
+ implementations.
- 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 7d724a4581715..66e8acaa242cf 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
@@ -71,3 +71,15 @@ Options
Comma separated list containing type names which are not counted as thrown
exceptions in the check. Default value is an empty string.
+
+.. option:: KnownUnannotatedAsThrowing
+
+ When `true`, treat calls to functions with visible definitions that are not
+ explicitly declared as non-throwing (i.e. lack ``noexcept`` or ``throw()``)
+ as potentially throwing, even if their bodies are visible and no explicit
+ throw is found. Default value is `false`.
+
+.. option:: UnknownAsThrowing
+
+ When `true`, treat calls to functions without visible definitions as
+ potentially throwing. Default value is `false`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-known-unannotated-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-known-unannotated-option.cpp
new file mode 100644
index 0000000000000..7f72870f43ea9
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-known-unannotated-option.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-exception-escape %t -- \
+// RUN: -config='{"CheckOptions": { \
+// RUN: "bugprone-exception-escape.KnownUnannotatedAsThrowing": true \
+// RUN: }}' -- -fexceptions
+
+void unannotated_no_throw_body() {}
+
+void calls_unannotated() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'calls_unannotated' which should not throw exceptions
+ unannotated_no_throw_body();
+}
+
+void extern_declared();
+
+void calls_unknown() noexcept {
+ // CHECK-MESSAGES-NOT: warning:
+ extern_declared();
+}
+
+void definitely_nothrow() noexcept {}
+
+void calls_nothrow() noexcept {
+ // CHECK-MESSAGES-NOT: warning:
+ definitely_nothrow();
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-unannotated-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-unannotated-option.cpp
new file mode 100644
index 0000000000000..c92eaa75c6adf
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-unannotated-option.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-exception-escape %t -- \
+// RUN: -config='{"CheckOptions": { \
+// RUN: "bugprone-exception-escape.UnknownAsThrowing": true \
+// RUN: }}' -- -fexceptions
+
+void unannotated_no_throw_body() {}
+
+void calls_unannotated() noexcept {
+ // CHECK-MESSAGES-NOT: warning:
+ unannotated_no_throw_body();
+}
+
+void extern_declared();
+
+void calls_unknown() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'calls_unknown' which should not throw exceptions
+ extern_declared();
+}
+
+void definitely_nothrow() noexcept {}
+
+void calls_nothrow() noexcept {
+ // CHECK-MESSAGES-NOT: warning:
+ definitely_nothrow();
+}
|
zeyi2
commented
Nov 17, 2025
|
|
||
| static ExceptionInfo createUnknown() { return {State::Unknown}; } | ||
| static ExceptionInfo createNonThrowing() { return {State::Throwing}; } | ||
| static ExceptionInfo createNonThrowing() { return {State::NotThrowing}; } |
Member
Author
There was a problem hiding this comment.
I understand there are valid reasons for this function to return Throwing, but it still feels a bit counter-intuitive for something named createNonThrowing to produce a Throwing.
If this change doesn't match your preference, I'm totally fine reverting it. Thanks in advance.
Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
🐧 Linux x64 Test Results
|
|
See #46282 (comment) for abandoned changes on a related issue. |
vbvictor
reviewed
Nov 18, 2025
vbvictor
reviewed
Nov 21, 2025
Comment on lines
+125
to
+126
| diag(MatchedDecl->getLocation(), "an exception may be thrown in function %0 " | ||
| "which should not throw exceptions") |
Contributor
There was a problem hiding this comment.
Did clang-format changed it? Seems unrelated
Comment on lines
+139
to
+140
| "frame #0: unhandled exception of type %0 may be thrown in function " | ||
| "%1 here", |
Comment on lines
+75
to
+85
| .. option:: KnownUnannotatedAsThrowing | ||
|
|
||
| When `true`, treat calls to functions with visible definitions that are not | ||
| explicitly declared as non-throwing (i.e. lack ``noexcept`` or ``throw()``) | ||
| as potentially throwing, even if their bodies are visible and no explicit | ||
| throw is found. Default value is `false`. | ||
|
|
||
| .. option:: UnknownAsThrowing | ||
|
|
||
| When `true`, treat calls to functions without visible definitions as | ||
| potentially throwing. Default value is `false`. |
Contributor
There was a problem hiding this comment.
I think instead of 2 options we should make 1 option with 3 different levels:
Option - TreatFunctionsWithoutSpecificationAsThrowing
Levels: All, OnlyUndefined, None.
Because i don't see a good usecase when we want to set KnownUnannotatedAsThrowing to true but UnknownAsThrowing to false.
WDYT?
CC @firewave.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #164795