Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -563,16 +563,6 @@ ExceptionAnalyzer::throwsException(const Stmt *St,
}
}
Results.merge(Uncaught);
} else if (const auto *Call = dyn_cast<CallExpr>(St)) {
if (const FunctionDecl *Func = Call->getDirectCallee()) {
ExceptionInfo Excs =
throwsException(Func, Caught, CallStack, Call->getBeginLoc());
Results.merge(Excs);
}
} else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) {
ExceptionInfo Excs = throwsException(Construct->getConstructor(), Caught,
CallStack, Construct->getBeginLoc());
Results.merge(Excs);
} else if (const auto *DefaultInit = dyn_cast<CXXDefaultInitExpr>(St)) {
ExceptionInfo Excs =
throwsException(DefaultInit->getExpr(), Caught, CallStack);
Expand Down Expand Up @@ -601,10 +591,25 @@ ExceptionAnalyzer::throwsException(const Stmt *St,
Results.merge(Excs);
}
} else {
// Check whether any of this node's subexpressions throws.
for (const Stmt *Child : St->children()) {
ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
Results.merge(Excs);
}

// If this node is a call to a function or constructor, also check
// whether the call itself throws.
if (const auto *Call = dyn_cast<CallExpr>(St)) {
if (const FunctionDecl *Func = Call->getDirectCallee()) {
ExceptionInfo Excs =
throwsException(Func, Caught, CallStack, Call->getBeginLoc());
Results.merge(Excs);
}
} else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) {
Comment on lines +602 to +608
Copy link
Contributor

@vbvictor vbvictor Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking out loud, I think it will be more readable if instead of placing dyn_cast<CallExpr> in "main" else branch, we would extract "check for children" part into a separate function and use it like this:

  } else if (const auto *Call = dyn_cast<CallExpr>(St)) {
    Results.merge(CheckChildren(St)); // New code!
    if (const FunctionDecl *Func = Call->getDirectCallee()) {
      ExceptionInfo Excs =
          throwsException(Func, Caught, CallStack, Call->getBeginLoc());
      Results.merge(Excs);
    }
  } else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) {
    Results.merge(CheckChildren(St)); // New code!
    ExceptionInfo Excs = throwsException(Construct->getConstructor(), Caught,
                                         CallStack, Construct->getBeginLoc());
    Results.merge(Excs);
  }

I'd rather see this more straightforward solution instead of twisting order of branches, which seem to me more bugprone and harder to read, WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, I'd really like to see this big function separated in N smaller with each function corresponding to each type of node we process.

ExceptionInfo Excs = throwsException(Construct->getConstructor(), Caught,
CallStack, Construct->getBeginLoc());
Results.merge(Excs);
}
}
return Results;
}
Expand Down
4 changes: 3 additions & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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. Additionally, fixed an issue
where the check wouldn't diagnose throws in arguments to functions or
constructors.

- Improved :doc:`bugprone-infinite-loop
<clang-tidy/checks/bugprone/infinite-loop>` check by adding detection for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -948,11 +948,62 @@ const auto throw_in_noexcept_lambda = [] () noexcept { throw 42; };
// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
// CHECK-MESSAGES: :[[@LINE-2]]:56: note: frame #0: unhandled exception of type 'int' may be thrown in function 'operator()' here

void thrower() {
int thrower() {
throw 42;
}

const auto indirect_throw_in_noexcept_lambda = [] () noexcept { thrower(); };
// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
// CHECK-MESSAGES: :[[@LINE-5]]:3: note: frame #0: unhandled exception of type 'int' may be thrown in function 'thrower' here
// CHECK-MESSAGES: :[[@LINE-3]]:65: note: frame #1: function 'operator()' calls function 'thrower' here

int f(int);
void throw_in_function_arg() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_in_function_arg' which should not throw exceptions
f(false ? 0 : throw 1);
}
// CHECK-MESSAGES: :[[@LINE-2]]:17: note: frame #0: unhandled exception of type 'int' may be thrown in function 'throw_in_function_arg' here

int g(int, int, int);
void throw_in_last_function_arg() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_in_last_function_arg' which should not throw exceptions
g(42, 67, false ? 0 : throw 1);
}
// CHECK-MESSAGES: :[[@LINE-2]]:25: note: frame #0: unhandled exception of type 'int' may be thrown in function 'throw_in_last_function_arg' here

void indirect_throw_in_function_arg() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_throw_in_function_arg' which should not throw exceptions
f(thrower());
}
// CHECK-MESSAGES: :[[@LINE-26]]:3: note: frame #0: unhandled exception of type 'int' may be thrown in function 'thrower' here
// CHECK-MESSAGES: :[[@LINE-3]]:5: note: frame #1: function 'indirect_throw_in_function_arg' calls function 'thrower' here

void indirect_throw_from_lambda_in_function_arg() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_throw_from_lambda_in_function_arg' which should not throw exceptions
f([] { throw 1; return 0; }());
}
// CHECK-MESSAGES: :[[@LINE-2]]:10: note: frame #0: unhandled exception of type 'int' may be thrown in function 'operator()' here
// CHECK-MESSAGES: :[[@LINE-3]]:30: note: frame #1: function 'indirect_throw_from_lambda_in_function_arg' calls function 'operator()' here

struct S {
S(int) noexcept {}
};

void throw_in_constructor_arg() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_in_constructor_arg' which should not throw exceptions
S s(false ? 0 : throw 1);
}
// CHECK-MESSAGES: :[[@LINE-2]]:19: note: frame #0: unhandled exception of type 'int' may be thrown in function 'throw_in_constructor_arg' here

void indirect_throw_in_constructor_arg() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_throw_in_constructor_arg' which should not throw exceptions
S s = thrower();
}
// CHECK-MESSAGES: :[[@LINE-50]]:3: note: frame #0: unhandled exception of type 'int' may be thrown in function 'thrower' here
// CHECK-MESSAGES: :[[@LINE-3]]:9: note: frame #1: function 'indirect_throw_in_constructor_arg' calls function 'thrower' here

void weird_throw_in_call_subexpression() noexcept {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'weird_throw_in_call_subexpression' which should not throw exceptions
(false ? []{} : throw 1)();
}
// CHECK-MESSAGES: :[[@LINE-2]]:19: note: frame #0: unhandled exception of type 'int' may be thrown in function 'weird_throw_in_call_subexpression' here