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

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs added a comment.

> "The warning was emitted at every occurence of the function. It might be confusing if it's only emitted for the definition."
> Why ? Issue is in definition, not declaration.

For me it would be confusing, because the forward declaration is naming the same function as the definition.
If I see that something is reported for the definition but not for the declaration I might think it's a bug in the 
tool, like once there is a problem with the function and the other time there isn't. Note that this particular 
warning is reported for the function and not for something inside the definition.

Also we have cross translation unit analysis, though I'm not sure this particular checker works there too.
Assuming it does, what happens if I forward declare the function in one translation unit and define it in
a different one? With this change the warning will only be output in the translation unit,the function is
defined in and this might silently hide some other problems in the TU the function is forward declared in.

> recursion_helper does not throw, it crashes.

Technically the exception is propagated through the function until a handler is found that catches it.

> Example with indirectly_recursive & recursion_helper behave in same way, only difference is that warning is emitted only for definition.

Please add a test case for this regardless of the behaviour to see that the checker handles exception propagation.

> This is other bug that I'm fixing (not under this patch) related to properly handling noexcept keyword.

I'm not sure what you mean by bug here.

  int recursion_helper(int n) noexcept {
    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in
    // function 'recursion_helper' which should not throw exceptions
    indirectly_recursive(n);
  }
  
  int indirectly_recursive(int n) noexcept {
    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in
    // function 'indirectly_recursive' which should not throw exceptions
    if (n == 0)
      throw n;
    return recursion_helper(n);
  }

Because `recursion_helper()` propagates the thrown object it makes sense to emit a warning for that too.
Also because the warning is emitted upon every propagation it is easy to trace where the exception actually
comes from. Think of it like a stack trace for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148462