Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-42100: Add _PyType_GetModuleByDef#22835
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
encukou commented Oct 20, 2020 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
encukou commented Oct 20, 2020
See #22838 for example usage. |
corona10 left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit comment.
IMHO, we can use C99 features since some of the module already use other c99 features.
(for example, struct init)
| assert(PyType_Check(type)); | ||
| assert(type->tp_mro); | ||
| int i; | ||
| for (i = 0; i < PyTuple_GET_SIZE(type->tp_mro); i++){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| for (i=0; i<PyTuple_GET_SIZE(type->tp_mro); i++){ | |
| for (inti=0; i<PyTuple_GET_SIZE(type->tp_mro); i++){ |
| { | ||
| assert(PyType_Check(type)); | ||
| assert(type->tp_mro); | ||
| int i; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| int i; |
encukou commented Oct 26, 2020
IMO that's a code style issue, and PEP7 has nothing against it. I'd rather keep it as is. |
shihai1991 left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, petr. I have no any other comment in here.
PS: I have reviewed it months ago in fork repo~
encukou commented Nov 3, 2020
Thanks! And thanks for the other review – this PR includes the changes you pointed out :) |
vstinner commented Nov 4, 2020
When a type has no subclass but a long MRO, _PyType_GetModuleByDef() has to iterate on the N parent classes before reaching the last one which will match. Would it be more efficient to iterator on the MRO in the reverse order? It would make the function slower for subclasses, but faster for direct instance of the type. |
vstinner commented Nov 4, 2020
Usually, when I read "get", I expect a O(1) operation. I'm fine if the first call fills a cache and is slower. But here, every call has a complexity of O(n) where n is the length of the MRO tuple. Would it make sense to rename the function to Anyway, thanks for adding this function! It will unblock porting many extension modules to multi-phase init and heap types in https://bugs.python.org/issue1635741 ! |
vstinner commented Nov 4, 2020
About _PyType_FindModuleByDef() name, here is a more concrete example: the PR #23124 converts the array extension to multi-phase init. The current PR defines: I would prefer to use "find" in the first macro, to better highlight that "get_array_state_by_type()" is a fast attribute access: |
encukou commented Nov 4, 2020
Hm, I don't expect that. "get" can mean many things :) I'll keep it in mind for if/when it becomes public API. I don't think it's worth changing the name now, after all the discussion. |
* master: bpo-42260: Add _PyInterpreterState_SetConfig() (pythonGH-23158) Disable peg generator tests when building with PGO (pythonGH-23141) bpo-1635741: _sqlite3 uses PyModule_AddObjectRef() (pythonGH-23148) bpo-1635741: Fix PyInit_pyexpat() error handling (pythonGH-22489) bpo-42260: Main init modify sys.flags in-place (pythonGH-23150) bpo-1635741: Fix ref leak in _PyWarnings_Init() error path (pythonGH-23151) bpo-1635741: _ast uses PyModule_AddObjectRef() (pythonGH-23146) bpo-1635741: _contextvars uses PyModule_AddType() (pythonGH-23147) bpo-42260: Reorganize PyConfig (pythonGH-23149) bpo-1635741: Add PyModule_AddObjectRef() function (pythonGH-23122) bpo-42236: os.device_encoding() respects UTF-8 Mode (pythonGH-23119) bpo-42251: Add gettrace and getprofile to threading (pythonGH-23125) Enable signing of nuget.org packages and update to supported timestamp server (pythonGH-23132) Fix incorrect links in ast docs (pythonGH-23017) Add _PyType_GetModuleByDef (pythonGH-22835) Post 3.10.0a2 bpo-41796: Call _PyAST_Fini() earlier to fix a leak (pythonGH-23131) bpo-42249: Fix writing binary Plist files larger than 4 GiB. (pythonGH-23121) bpo-40077: Convert mmap.mmap static type to a heap type (pythonGH-23108) Python 3.10.0a2
See https://mail.python.org/archives/list/[email protected]/thread/T3P2QNLNLBRFHWSKYSTPMVEIL2EEKFJU/ for discussion.
https://bugs.python.org/issue42100