Skip to content

[WIP] bpo-1635741: Port _collections module to multiphase initialization. - #19071

Closed
corona10 wants to merge 1 commit into
python:masterfrom
corona10:bpo-1635741-_collectionsmodule
Closed

[WIP] bpo-1635741: Port _collections module to multiphase initialization.#19071
corona10 wants to merge 1 commit into
python:masterfrom
corona10:bpo-1635741-_collectionsmodule

Conversation

@corona10

@corona10 corona10 commented Mar 19, 2020

Copy link
Copy Markdown
Member
Comment thread Modules/_collectionsmodule.c Outdated

@corona10 corona10 Mar 19, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@vstinner I'd like to create a new helper function for this.

This logic can be reused.
https://github.com/python/cpython/blob/514c469719f149e1722a91a9d0c63bf89dfefb2a/Modules/itertoolsmodule.c#L4733L4747

What do you think?

@vstinner vstinner Mar 19, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you mean combine PyType_Ready + _PyType_Name + PyModule_AddObject? Yeah, maybe it's worth it, but I would prefer to add it once this PR lands.

@corona10 corona10 changed the title bpo-1635741: Port _collections module to multiphase initialization. Mar 19, 2020
@corona10
corona10 force-pushed the bpo-1635741-_collectionsmodule branch from 58e7344 to fb35d81 Compare March 19, 2020 14:46
&PyODict_Type,
&dequeiter_type,
&dequereviter_type,
&tuplegetter_type,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Either you use Py_ARRAY_COUNT() to get the list length, or you need to add a NULL terminator. Currently, you rely on an undefined behavior.

@shihai1991

Copy link
Copy Markdown
Member
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment