On 4/8/23 06:45, Edward Catmur wrote:
>
> On Fri, 7 Apr 2023 at 18:54, Andrey Semashev via Std-Discussion
> <std-discussion_at_[hidden]
> <mailto:std-discussion_at_[hidden]>> wrote:
>
> On 4/8/23 01:49, Thiago Macieira via Std-Discussion wrote:
> > On Friday, 7 April 2023 14:31:46 -03 Andrey Semashev via
> Std-Discussion wrote:
> >> I think this issue needs to be addressed in the TS before
> inclusion into
> >> the main standard. As a possible solution, I proposed the
> following idea:
> >
> > The standard solution does not need your suggested implementation
> details. It
> > just needs to describe the expected behaviour and require that the
> > implementations do it properly. They may have other means of
> achieving the
> > same result.
>
> Sure, the standard may simply require a certain behavior, and I did not
> intend my proposed solution to be adopted or even be part of the
> standard wording. I simply indicated that there is a way to achieve
> this, which is something that is welcome for any proposal, isn't it?
>
> The point is, the current TS wording explicitly mentions
> unhandled_exceptions(), and it should not. Instead, it should describe
> the required behavior, including when used in conjunction with
> coroutines. Though I would still prefer if the behavior was
> implementable using the standard library API so that non-standard scope
> guard libraries like Boost.Scope could be implemented without resorting
> to compiler-specific intrinsics or private APIs.
>
> Your putative co_unhandled_exceptions() sounds like it would have pretty
> substantial overhead - an extra word-sized counter per coroutine, and
> the overhead of updating it whenever you resume a coroutine. It's a big
> ask compared to the plain unhandled_exceptions(), which is one word per
> thread and only updated when exceptions are thrown/caught, so lost in
> the noise.

The added cost may not be as high as it may seem.

1. The unhandled exception counter needs to be captured on coroutine
creation. This overhead is added to the existing overhead of dynamically
allocating the coroutine state and copying all the couroutine arguments
to it. Copying one more integer doesn't seem much in comparison.

2. Updating the coroutine-local counter needs to happen when an
exception is thrown/re-thrown/caught. This overhead is added to
allocating and copying the exception object and the associated stack
unwinding process. Depending on the exception type and the cost of
unwinding, this overhead may or may not be noticeable. However, this
overhead only exists on the exceptional code path.

3. When the exception passes the boundary of the coroutine and the
caller is also a coroutine, the caller's counter needs to be updated.
The compiler already generates an implicit try/catch block around the
coroutine body to catch any exceptions leaving it to call
promise_type::unhandled_exception(), which may capture the exception so
that it can be later rethrown in the caller. So this counter update is
actually implemented as part of #2 described above, no additional
overhead here.

In any case, I'm not insisting on this co_unhandled_exceptions()
implementation, or even on the particular co_unhandled_exceptions()
interface. If there is a more efficient way to discover whether an
exception is in flight, including in coroutines, that's great and let's
standardize that.

> I think it's a lot more likely that compilers will just create types
> whose destructors are marked as being called only either from scope exit
> or from unwind cleanup, and omitted in the other case. (At least, that's
> how I'd implement it.) So Boost.Scope etc. will need to either use those
> magic library types, or the attributes/intrinsics those library types
> use (if they aren't implemented with True Name magic).

Not calling the destructor on normal return (or on exception - we'd need
both for scope_success and scope_fail) will not work because even if you
don't invoke the scope guard action, you still want to destroy it.

I don't think skipping just the body of the class destructor but not
everything else makes much sense, nor is flexible to be useful beyond
the scope guard use case. E.g. why skip the whole body and not just part
of it? At this point you want an intrinsic or a library function that
tells you whether an exception is in flight. However the compiler
writers choose to implement this, my preference is that this mechanism
is portable across implementations and is documented in the standard.

Also, if you're thinking about marking the objects on construction to
skip their destructors upon different kinds of scope exiting, that
sounds like a more expensive solution compared to
co_unhandled_exceptions(). In this case, the overhead would be attached
to every non-trivially-destructible object created on the stack,
coroutines or not, compared to just a few key points in coroutines
creation and stack unwinding.

> I agree with Thiago that it's more important to establish what the
> correct behavior is around coroutines, so we can get it into wording as
> examples and as the basis of Library tests.

Not arguing with this.