Skip to content

Correctly return finished#6

Open
facontidavide wants to merge 1 commit intotonbit:masterfrom
facontidavide:master
Open

Correctly return finished#6
facontidavide wants to merge 1 commit intotonbit:masterfrom
facontidavide:master

Conversation

@facontidavide
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Author

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

@tonbit
Copy link
Copy Markdown
Owner

tonbit commented Aug 3, 2018

PR means ?

@facontidavide
Copy link
Copy Markdown
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