gh-91603: Speed up isinstance/issubclass on union types #91631
Conversation
|
@JelleZijlstra I have unified logic for |
Misc/NEWS.d/next/Core and Builtins/2022-04-17-11-03-45.gh-issue-91603.hYw1Lv.rst
Outdated
Show resolved
Hide resolved
| @@ -348,12 +294,6 @@ static PyMemberDef union_members[] = { | |||
| {0} | |||
| }; | |||
|
|
|||
| static PyMethodDef union_methods[] = { | |||
| {"__instancecheck__", union_instancecheck, METH_O}, | |||
There was a problem hiding this comment.
I think we may still want to keep those methods too in case people call them directly. Let me think about it some more.
There was a problem hiding this comment.
On second thought, I don't think we need to keep these. The language reference (https://docs.python.org/3/library/stdtypes.html#types-union) explicitly says that isinstance() works with union objects, so we don't need __instancecheck__ to be present to indicate that they work with isinstance().
So no change needed here.
…e-91603.hYw1Lv.rst Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
| @@ -348,12 +294,6 @@ static PyMemberDef union_members[] = { | |||
| {0} | |||
| }; | |||
|
|
|||
| static PyMethodDef union_methods[] = { | |||
| {"__instancecheck__", union_instancecheck, METH_O}, | |||
There was a problem hiding this comment.
On second thought, I don't think we need to keep these. The language reference (https://docs.python.org/3/library/stdtypes.html#types-union) explicitly says that isinstance() works with union objects, so we don't need __instancecheck__ to be present to indicate that they work with isinstance().
So no change needed here.
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
There was a problem hiding this comment.
Looks good to me, but since it's a C change I'm going to run it through the buildbots. Hopefully @Fidget-Spinner can take a look too.
|
🤖 New build scheduled with the buildbot fleet by @JelleZijlstra for commit 20c44f8 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
|
@Fidget-Spinner It will be great if you can review this PR. |
|
The |
#91603