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.

Sign up for GitHub

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

Jump to bottom

bpo-32357: Use PySet_GET_SIZE macro in _is_coroutine() from _asynciomodule.c #4990

Merged
merged 3 commits into from
Dec 23, 2017
Merged

bpo-32357: Use PySet_GET_SIZE macro in _is_coroutine() from _asynciomodule.c #4990

merged 3 commits into from
Dec 23, 2017

Conversation

asvetlov
Copy link
Contributor

@asvetlov asvetlov commented Dec 23, 2017 •
edited by bedevere-bot
Loading

PySet_Size can fail if passed object is not a set.
It's not the case, pretty sure -- but let's add a check for better code readability.

https://bugs.python.org/issue32357

@asvetlov asvetlov requested a review from 1st1 as a code owner December 23, 2017 16:51
@1st1
Copy link
Member

1st1 commented Dec 23, 2017

It can't happen, as the set object is created in _asynciomodule.c. Let's change it to PySet_GET_SIZE

@1st1 1st1 changed the title Fix unhandled exception in _asyncio._is_coroutine bpo-32357: Fix unhandled exception in _asyncio._is_coroutine Dec 23, 2017
@1st1
Copy link
Member

1st1 commented Dec 23, 2017

If you want put an assert(PySet_Check(iscoroutine_typecache)) before PySet_GET_SIZE

@asvetlov
Copy link
Contributor Author

I like PySet_GET_SIZE, assertion is redundant.
Initial motivation was checking the error if PySet_GetSize() can return it, now the check is not needed.

@asvetlov asvetlov changed the title bpo-32357: Fix unhandled exception in _asyncio._is_coroutine bpo-32357: Use PySet_GET_SIZE macro in _is_coroutine() from _asynciomodule.c Dec 23, 2017
@1st1
Copy link
Member

1st1 commented Dec 23, 2017

👍

@1st1 1st1 merged commit 0f47fa2 into python:master Dec 23, 2017
@1st1
Copy link
Member

1st1 commented Dec 23, 2017

(merged it myself as I'm working on _asynciomodule.c right now)

@asvetlov
Copy link
Contributor Author

Cool!

@asvetlov asvetlov deleted the handle-asyncio-exc branch December 23, 2017 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@1st1 1st1 1st1 approved these changes

Assignees
No one assigned
Labels
skip issue skip news topic-asyncio
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@asvetlov @1st1 @the-knights-who-say-ni @bedevere-bot