[PATCH] D147417: [clang-tidy] Do not emit bugprone-exception-escape warnings from coroutines

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp:75-79
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:11: warning: an exception may be thrown in function 'b_ShouldNotDiag' which should not throw exceptions
+  if (b == 0)
+    throw b;
+
+  co_return a / b;
----------------
denizevrenci wrote:
> ChuanqiXu wrote:
> > I don't understand why we shouldn't emit the warning here. Since the function is marked `noexcept` but it may throw actually in `unhandled_exception`. I think it is meaningful to warn for this.
> Right, I now see that this behavior is different between Clang's `-Wexceptions` and Clang Tidy's `bugprone-exception-escape`. The former does not warn on this code, the latter does.
> 
> ```
> int foo() {
>   throw 1;
> }
> 
> int bar() noexcept {
>   return foo();
> }
> ```
> 
> We need to treat coroutines differently and check whether `task::task`, `promise::promise`,  `promise::initial_suspend`, `promise::get_return_object`, and `promise::unhandled_exception` can throw instead of the body of the coroutine.
I investigated the exception issue in coroutines before: https://reviews.llvm.org/D108277. And it is much more complex than I thought. The short conclusion here is that the coroutine is still may throw even if all the promise's method wouldn't throw. For example:

```
struct Evil {
    ~Evil() noexcept(false) { throw 32; }
};

task foo() noexcept { // all promise's method of task wouldn't throw
    throw Evil;
}
```

And in the above example, foo() may throw actually. Although the implicit `catch` block of `foo()` will catch `Evil`, the exception in the destructor of `Evil` will be thrown again.

So we can't be sure that a coroutine wouldn't throw even if all of its promise's method wouldn't throw.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147417