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:
> > 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.
> It looks like the function `foo` can throw until the first suspension point
> in the coroutine. If `promise::initial_suspend` is `std::suspend_always`,
> then it will not throw. Of course, determining this statically is quite
> complicated.
> But I also think that this is a rather niche example, it looks like
> clang-tidy already warns with `bugprone-exception-escape` on the destructor
> of `Evil` even when it is marked `noexcept(false)`. I assume this is due to
> the other
> complications brought by throwing from destructors. Would that not be the
> appropriate place to warn about this anyway?
>
> For example, the code below terminates because the destructor of `Evil` gets
> called while there is an active exception.
> ```
> task foo() { // all promise's method of task wouldn't throw
> Evil e;
> throw 1;
> co_return;
> }
> ```
If we've handled the case, the strategy makes sense to me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147417/new/
https://reviews.llvm.org/D147417
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
