Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly return finished #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

facontidavide
Copy link

Hi,

I think this change makes the behavior of resume more semantically correct.

Consider the following code

void routine_func()
{
    std::cout << "step1" << std::endl;
    coroutine::yield();

    std::cout << "step2" << std::endl;
    coroutine::yield();

    std::cout << "step3"<< std::endl;
    coroutine::yiel3();

     std::cout << "done" << std::endl;
}

int main()
{
    coroutine::routine_t rt = coroutine::create(routine_func);

    std::cout << coroutine::resume(rt) << std::endl;
    std::cout << coroutine::resume(rt) << std::endl;
    std::cout << coroutine::resume(rt) << std::endl;
    std::cout << coroutine::resume(rt) << std::endl;

   return 0;
}

I think it is reasonable to expect an output like

step1
0
step2
0
step3
0
done
-2

I would also suggest to add an enum like this

enum ResumeResult{
    INVALID = -1,
    FINISHED = -2,
    YIELD = 0
};

Cheers

Davide

@tonbit
Copy link
Owner

tonbit commented Aug 1, 2018

After co-routine is created, it will not start to run until thread owner resumes it. This behavior makes co-routine more flexible, splits object instantiating and routine running.

@facontidavide
Copy link
Author

The PR doesn't affect the behavior when routine is created

@tonbit
Copy link
Owner

tonbit commented Aug 3, 2018

PR means ?

@facontidavide
Copy link
Author

facontidavide commented Aug 3, 2018

PR = Pull request :)

What I mean, is that in the example I provided, the 4th resume would return 0, even if the end of the coroutine was reached.
I think it is more intuitive that -2 (finished) is returned instead... since the couroutine finished at resume number 4

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