Skip to content

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

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

Closed

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Mar 19, 2020

&tuplegetter_type,
};

for (int i = 0; typelist[i] != NULL; i++) {
Copy link
Member Author

@corona10 corona10 Mar 19, 2020

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?

Copy link
Member

@vstinner vstinner Mar 19, 2020

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. [WIP] 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
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
Member

FWIW, some comments in https://github.com/python/cpython/pull/18066/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants