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

bpo-41756: Add PyIter_Send function to allow sending value into generators/coroutines/iterators #22443

Merged
merged 3 commits into from Oct 10, 2020

Conversation

vladima
Copy link
Contributor

@vladima vladima commented Sep 28, 2020

Add PyIter_Send function to allow sending value into generators/coroutines/iterators and receive results without raising StopIteration exception.

I anticipate more changes related to naming so deferring updating docs before consensus is reached.

https://bugs.python.org/issue41756

…tines/iterators and receive results without raising StopIteration exception
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

LGTM. Please add a C API documentation and a What's New entry.

} else {
if (is_gen_or_coro) {
retval = _PyGen_Send((PyGenObject *)receiver, v);
Copy link
Member

@serhiy-storchaka serhiy-storchaka Sep 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_PyGen_Send() can be removed now, is not?

@@ -0,0 +1,3 @@
Add PyIter_Send function to allow sending value into
Copy link
Member

@serhiy-storchaka serhiy-storchaka Sep 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Add PyIter_Send function to allow sending value into
Add :c:func:`PyIter_Send` function to allow sending value into

@@ -0,0 +1,3 @@
Add PyIter_Send function to allow sending value into
generator/coroutine/iterator without raising StopIteration exception to
signal return
Copy link
Member

@serhiy-storchaka serhiy-storchaka Sep 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
signal return
signal return.

Modules/_asynciomodule.c Show resolved Hide resolved
Objects/abstract.c Show resolved Hide resolved
if (tstate->c_tracefunc == NULL && is_gen_or_coro) {
gen_status = PyGen_Send((PyGenObject *)receiver, v, &retval);
PySendResult gen_status;
if (tstate->c_tracefunc == NULL) {
Copy link
Member

@1st1 1st1 Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we fold the tracing into the genobject/PyIter_Send inmplementation to avoid code duplication?

Copy link
Member

@1st1 1st1 Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would also make ceval a tad smaller which is always a good thing.

Copy link
Member

@serhiy-storchaka serhiy-storchaka Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it is feasible.

Copy link
Member

@1st1 1st1 Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate?

Copy link
Member

@serhiy-storchaka serhiy-storchaka Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a call of the trace function in between calls of __next__/send and _PyGen_FetchStopIterationValue. It cannot be called after PyIter_Send because the original exception (which can be a subclass of StopIteration or StopIteration with additional attributes) is already silenced in PyIter_Send. Therefore we should pass the trace function to PyIter_Send, and this would complicate the API.

We can introduce a private function _PyIter_SendWithTrace which accepts also the trace function argument. But would not it make the code more complicated? In any case it is better to left to a separate PR or separate issue.

Copy link
Member

@1st1 1st1 Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about exporting the call_exc_trace function and calling it directly from genobject.c; the trace function can easily be read from the thread state.

1st1
1st1 approved these changes Oct 9, 2020
Copy link
Member

@1st1 1st1 left a comment

LGTM.

@serhiy-storchaka Serhiy, can you review again? If you're OK I think this is ready to be merged.

Please also read https://github.com/python/cpython/pull/22443/files#r502624137.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

LGTM.

The remaining question: do we still need PyGen_Send() in the public API?

if (tstate->c_tracefunc == NULL && is_gen_or_coro) {
gen_status = PyGen_Send((PyGenObject *)receiver, v, &retval);
PySendResult gen_status;
if (tstate->c_tracefunc == NULL) {
Copy link
Member

@serhiy-storchaka serhiy-storchaka Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it is feasible.

@1st1
Copy link
Member

1st1 commented Oct 9, 2020

The remaining question: do we still need PyGen_Send() in the public API?

I actually don't think we need it. And if we discover a use case we can always (re-)expose it.

@1st1
Copy link
Member

1st1 commented Oct 9, 2020

@vladima Let's drop PyGen_Send() for now and merge this.

@1st1
Copy link
Member

1st1 commented Oct 10, 2020

@vladima Let's drop PyGen_Send() for now and merge this.

Actually, to avoid the churn, let's merge this as is and continue the discussion. We can drop it in a follow up pr.

@1st1 1st1 merged commit 037245c into python:master Oct 10, 2020
3 checks passed
@bedevere-bot
Copy link

bedevere-bot commented Oct 10, 2020

@1st1: Please replace # with GH- in the commit message next time. Thanks!

@vladima vladima deleted the iter_send branch Oct 10, 2020
xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
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.

None yet

5 participants