Skip to content

Abstract Coroutine interface - #36

Open
mikael-s-persson wants to merge 5 commits into
dallison:mainfrom
mikael-s-persson:feature/abstract_coroutine
Open

Abstract Coroutine interface#36
mikael-s-persson wants to merge 5 commits into
dallison:mainfrom
mikael-s-persson:feature/abstract_coroutine

Conversation

@mikael-s-persson

Copy link
Copy Markdown
Contributor

This PR (along with dallison/cpp_toolbelt#27) is about decoupling the coroutine interface that functions/lambdas that run in a coroutine need from the actual scheduler implementation. This is mostly separating concerns and to some extent helps hide implementation details and flags that don't affect the code that runs in a coroutine context, only how the coroutines are scheduled and switched.

This could be controversial change because it breaks users since the concrete coroutine class that you construct and tie to a scheduler is now called ScheduledCoroutine, while Coroutine is just an abstract class now.

@dallison

dallison commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Would it be better to rename Coroutine to CoroutineBase keeping that abstract and then Coroutine is a concrete instance of it. Any functions that take a Coroutine* or Coroutine& would still work, and any code that creates Coroutine instances can still compile.

Unless that breaks you separation of concerns issue you are addressing. I am concerned about breaking existing code.

@mikael-s-persson

Copy link
Copy Markdown
Contributor Author

Yeah, that might make more sense now. Originally, I tried to make Coroutine abstract so that all the code in toolbelt that takes Coroutine* parameters wouldn't have to depend on the full coroutine scheduler implementation. But, after that, I introduced the "Poller" abstraction in toolbelt, which effectively cuts that dependency out.

A potential "redux" version of this PR could simply be to make the original Coroutine class inherit from Poller and split the toolbelt/coroutine dependency that way.

Splitting the Coroutine class into two parts (in this PR: Coroutine and ScheduledCoroutine) is mostly nice in principle (and cleaner) but it's a hard breaking change with little practical effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants