Skip to content

[C API] Introduce a new slot in PyModuleDef to hold the classes #88265

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
shreyanavigyan mannequin opened this issue May 10, 2021 · 12 comments
Closed

[C API] Introduce a new slot in PyModuleDef to hold the classes #88265

shreyanavigyan mannequin opened this issue May 10, 2021 · 12 comments
Labels
extension-modules C modules in the Modules dir topic-C-API type-feature A feature request or enhancement

Comments

@shreyanavigyan
Copy link
Mannequin

shreyanavigyan mannequin commented May 10, 2021

BPO 44099
Nosy @vstinner, @tiran, @encukou, @erlend-aasland, @shreyanavigyan

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-05-10.09:49:59.507>
labels = ['extension-modules', 'expert-C-API', 'type-feature']
title = '[C API] Introduce a new slot in PyModuleDef to hold the classes'
updated_at = <Date 2021-05-20.15:22:46.540>
user = 'https://github.com/shreyanavigyan'

bugs.python.org fields:

activity = <Date 2021-05-20.15:22:46.540>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules', 'C API']
creation = <Date 2021-05-10.09:49:59.507>
creator = 'shreyanavigyan'
dependencies = []
files = []
hgrepos = []
issue_num = 44099
keywords = []
message_count = 11.0
messages = ['393374', '393890', '393956', '393957', '394014', '394022', '394023', '394024', '394025', '394030', '394032']
nosy_count = 5.0
nosy_names = ['vstinner', 'christian.heimes', 'petr.viktorin', 'erlendaasland', 'shreyanavigyan']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue44099'
versions = []

@shreyanavigyan
Copy link
Mannequin Author

shreyanavigyan mannequin commented May 10, 2021

It's tedious to add classes one by one using PyModule_AddObject in PyInit_module function. Wouldn't it be much easier if there's a slot that has an array of all the classes in their PyObject form and it'll automatically add those classes. Since this is a backwards compatibility change it'll be best if a macro (like PY_AUTO_OBJECT maybe?) is defined then it'll add those classes automatically. If the slot is NULL or 0 then don't add anything. But yeah they may add more classes if they want but it doesn't matter much.

Thanking you,

With Regards,
Shreyan Avigyan

@shreyanavigyan shreyanavigyan mannequin added extension-modules C modules in the Modules dir topic-C-API type-feature A feature request or enhancement labels May 10, 2021
@shreyanavigyan shreyanavigyan mannequin changed the title Introduce a new slot in PyModuleDef to hold the classes [c-api] Introduce a new slot in PyModuleDef to hold the classes May 11, 2021
@shreyanavigyan shreyanavigyan mannequin changed the title Introduce a new slot in PyModuleDef to hold the classes [c-api] Introduce a new slot in PyModuleDef to hold the classes May 11, 2021
@shreyanavigyan
Copy link
Mannequin Author

shreyanavigyan mannequin commented May 18, 2021

A friendly ping

@shreyanavigyan shreyanavigyan mannequin changed the title [c-api] Introduce a new slot in PyModuleDef to hold the classes [C API] Introduce a new slot in PyModuleDef to hold the classes May 19, 2021
@shreyanavigyan shreyanavigyan mannequin changed the title [c-api] Introduce a new slot in PyModuleDef to hold the classes [C API] Introduce a new slot in PyModuleDef to hold the classes May 19, 2021
@encukou
Copy link
Member

encukou commented May 19, 2021

First off, note that you can use PyModule_AddType rather than PyModule_AddObject to avoid repeating the name.

Adding this *correctly* will be somewhat involved: slots take function pointers, not data pointers which are incompatible according to standard C. The changes will also need to keep backwards compatibility.

@shreyanavigyan
Copy link
Mannequin Author

shreyanavigyan mannequin commented May 19, 2021

Yes and it wouldn't be a problem. If struct member is 0 or NULL then use default behavior. If not then apply PyModule_AddType or PyModule_AddObject on all of those. Then after finishing set slot to NULL again since we don't want to add a type more than once. And yes after this also anyone can add more types. This will just introduce a handy way to add types.

@erlend-aasland
Copy link
Contributor

See also bpo-42376

@shreyanavigyan
Copy link
Mannequin Author

shreyanavigyan mannequin commented May 20, 2021

I'm not sure if bpo-42376 is same as this one. Yes the basic idea to provide abstraction and make it easy for the users to add type is same but the approaches suggested are completely different. I propose a new structure member in PyModuleDef be added where we can just specify the address of the types and then in PyModule_Create add those automatically and set the member back to NULL. This will not be a performance improvement just an enhancement to allow adding types more easily. The only thing I'm concerned about is that will it break the Stable ABI? It'll not break C extensions though.

@erlend-aasland
Copy link
Contributor

How will you differentiate which types should be added to the module dict and which not to add. How will you map the instantiated type objects to the respective module state members?

@shreyanavigyan
Copy link
Mannequin Author

shreyanavigyan mannequin commented May 20, 2021

In PyModule_Create -> PyModule_Create2 -> SomeFunc... it would loop through the array of object addresses and call PyModule_AddType on each and every one of them. This will not change behavior or control flow only implement a handy way. This I've found very distrubing,

PyModule_AddType(&Example1);
PyModule_AddType(&Example2);
PyModule_AddType(&Example3);

The proposed way will be

PyModuleDef examplemodule = {
...
.types = {Example1, Example2, Example3};
...
}

Now just calling PyModule_Create (not PyModule_Init or any other initialization function) would add those types automatically.

@tiran
Copy link
Member

tiran commented May 20, 2021

I suggest you try to implement your proposal first before you post it. (spoilers: you will notice that it doesn't work for non-trivial heap types.)

@shreyanavigyan
Copy link
Mannequin Author

shreyanavigyan mannequin commented May 20, 2021

Yes. It's becoming hard to implement both static and heap types using one struct member. Since this is all about implementing a helping hand when it comes to types I think two different members one for static and another one for heap would be fine. And if someone then comes up with even a better idea, then I'll implement that. I'm now working on implementing them.

@vstinner
Copy link
Member

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@erlend-aasland
Copy link
Contributor

Suggesting closing this as out-of-date, or perhaps as a duplicate of gh-86542.

@erlend-aasland erlend-aasland added the pending The issue will be closed if no feedback is provided label Jun 22, 2023
@encukou encukou closed this as completed Jun 22, 2023
@erlend-aasland erlend-aasland closed this as not planned Won't fix, can't repro, duplicate, stale Jun 22, 2023
@erlend-aasland erlend-aasland removed the pending The issue will be closed if no feedback is provided label Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants