[mikael-s-persson]

This is a refactor that's been gnawing at me for a while. It feels wrong that toolbelt would depend on coroutine, just to inject a couple of poll-like functions. So, this PR replaces the co::Coroutine* parameters in the various toolbelt classes to use its own toolbelt::Poller abstract base class, which co::Coroutine inherits from (there will be a corresponding PR in coroutines repo too).

I also add, for convenience, a PosixPoller class that implements the interface with vanilla Posix poll calls.

The motivation, other than the mere ideal of separating concerns, is to make toolbelt lighter and not joined-at-the-hip with coroutines, for when they are not used.

It creates a slightly weird situation where both repos depend on each other, but that works because only toolbelt's test targets depend on coroutines.

[dallison]

I think this will be hard to merge. Anything that uses the toolbelt functions that take a Coroutine*, like subspace will not work until both this and the co change are in.

If we could move to C++20 we could possibly use concepts to make this easier. Like adding a "Pollable" concept?

[mikael-s-persson]

I don't think it's that hard to merge. But yeah, both PRs have to be merged in close succession and there will technically be a short window where updating toolbelt only will not work. Currently, in my code-base, I put to the latest commits on both PRs, and build subspace against those versions of toolbelt and coroutines (and my subspace version is quite old), and it all works fine. I think the merge procedure boils down to these simple steps (in quick succession):

1. Merge this toolbelt-PR
2. Bump up the toolbelt version number (in its MODULE.bazel)
3. Update the co-PR to depend on the new toolbelt version, using a git_override that points to main-head-commit
4. Merge the co-PR
5. Bump up the co version number (in its MODULE.bazel)

At this point, they are coherent. Later, you can publish the new toolbelt version on BCR, remove the git_override from coroutines, and publish that snapshot of coroutines to BCR. Anyone who updates their coroutine dep will be warned / required to update their toolbelt dep too. Anyone who only uses toolbelt won't have any issue.

[dallison]

I was thinking a little about this and I don't like the name "Poller". From the toolbelt's point of view, it's really a "Waitable" object, in that we are using an object that we can use to wait for an event to occur. It's not really polling anything, but it's waiting.

How do you feel about replacing "Poller" with "Waitable"?

[mikael-s-persson]

What about "Waiter"?

What I don't like about "waitable" is that it sounds more like the thing you can wait for (aka an event), rather than the thing that does the waiting.

[dallison]

How about "AsyncWaiter"?

[mikael-s-persson]

Alright, AsyncWaiter it is.