Skip to content

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

Merged
1st1 merged 3 commits into
python:masterfrom
asvetlov:handle-asyncio-exc
Dec 23, 2017
Merged

bpo-32357: Use PySet_GET_SIZE macro in _is_coroutine() from _asynciomodule.c #4990
1st1 merged 3 commits into
python:masterfrom
asvetlov:handle-asyncio-exc

Conversation

@asvetlov

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

Copy link
Copy Markdown
Contributor

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

1st1 commented Dec 23, 2017

Copy link
Copy Markdown
Member

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

1st1 commented Dec 23, 2017

Copy link
Copy Markdown
Member

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

@asvetlov

asvetlov commented Dec 23, 2017

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

1st1 commented Dec 23, 2017

Copy link
Copy Markdown
Member

👍

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

1st1 commented Dec 23, 2017

Copy link
Copy Markdown
Member

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

@asvetlov

asvetlov commented Dec 23, 2017

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