Re: [LEAF] A sort of review

From: Emil Dotchevski (emildotchevski_at_[hidden])
Date: 2020-06-04 21:59:54

On Thu, Jun 4, 2020 at 1:57 AM Andrzej Krzemienski via Boost <
boost_at_[hidden]> wrote:
> I find function preload() (https://zajo.github.io/leaf/#tutorial-preload)
> really great and novel. This is the first big C++ library that I know that
> implements it. This really makes the error reporting code brief, clean and
> bug-free.

Thanks for the review.

I remember when Boost Exception was discussed (ancient history now), David
Abrahams pointed out that it is unfortunate that it requires an exception
to be caught before it can be augmented. In terms of Boost Exception, you
must say:

try
{
  f();
}
catch( boost::exception & e )
{
  e << more_info{};
  throw;
}

He thought that we're not really doing error handling here, so the addition
of more_info should happen automatically. Unfortunately there is no good
way to do this with Boost Exception. In that design, you need to interact
with the exception object and the only way to get a reference to it is to
catch it. Perhaps it's possible to do some extreme acrobatics to hide the
catch from the user (e.g. a macro), but at that point it becomes
questionable whether that would be desirable at all. Further, e <<
more_info{} requires dynamic memory to be allocated, which could fail. As
ugly as it is, it's best to spell it all out, so we know what's going on.

I never forgot his feedback though. The analogous LEAF code is this:

auto p = leaf::preload(more_info{}); // Does not allocate dynamic memory
f();

So, thank you Dave. :)

> My only criticism of the library is that it makes a serious design choice
> to use a thread-local storage and fails to communicate it clearly to
> potential users in the documentation.

This is specified here: https://zajo.github.io/leaf/#context. There are 8
places in the documentation that mention thread local.

Perhaps it could be more prominent, I'm open to suggestions. IMO this isn't
something most users need to worry about in 2020. If I were one of the few
users who must worry about it, I'd search for "thread_local" in the source
code to be sure.

> Emil informs me that this problem could be
> worked around by manually propagating some error information at the point
> where coroutines are managed. And I trust him on that.

Don't. :)

I don't know if it's possible, I don't know coroutines enough. From what
I've read, it should be possible to store a leaf::context in an awaitable,
and activate/deactivate it as the coroutine is resumed/suspended (I plan,
in fact, to rename context::activate/deactivate to resume/suspend).

Thanks once again.