GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — bpo-42100: Add _PyType_GetModuleByDef by encukou · Pull Request #22835 · python/cpython Conversation
|
|
See #22838 for example usage. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| 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.
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 (int i = 0; i < PyTuple_GET_SIZE(type->tp_mro); i++) { |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| { | ||
| assert(PyType_Check(type)); | ||
| assert(type->tp_mro); | ||
| int i; |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
| int i; |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
IMO that's a code style issue, and PEP7 has nothing against it. I'd rather keep it as is. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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~
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks! And thanks for the other review – this PR includes the changes you pointed out :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 ! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
* 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
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.