[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

Piotr Zegar via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 17 03:13:05 PDT 2023


PiotrZSL added a comment.

@isuckatcs
"Technically the exception is propagated through the function until a handler is found that catches it."
No because indirectly_recursive called from recursion_helper is noexcept, so there will be std::terminate called.

"Also we have cross translation unit analysis"
Not in clang-tidy, and that work based on AST merge, so even if someone manage to run it here, it will work fine.

"Please add a test case for this regardless of the behaviour to see that the checker handles exception propagation."
There is test for that. The one with recursion_helper + indirectly_recursive. Be more specific if you want something else.

"Because recursion_helper() propagates the thrown object it makes sense to emit a warning for that too."
No it's not propagating thrown object. Bug: https://github.com/llvm/llvm-project/issues/54956

As for warnings for forward declaration, what developer have to do with such information ? There is nothing he can change in forward declaration to fix this issue. And putting 2 or more NOLINTS to silence single issue is stupid. Other checks validate only definitions. Forward declarations are forward declarations, they transparent for an exception propagation. And current behaviour introduce also performance penalty, as same thing is done multiple times. If you somehow want to point to developer issue about function declaration, there are notes for that, but still developer know about declarations, and to fix issue He/She don't need to touch this, only definition need to be changed (unlles decides to remove noexcept keyword, but then compiler will tell him about issues). Same goes for pure virtual functions, check does not emit warnings for them just because some implementation throw exception.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148462/new/

https://reviews.llvm.org/D148462



More information about the cfe-commits mailing list