Repository navigation
PyType_FromSpec API fails to use metaclass of bases #89546
Description
Activity
seberg commented
on Oct 5, 2021 sebergmannequinMannequinAuthorThe PyType_FromSpec fails to take care about MetaClasses.
https://bugs.python.org/issue15870Asks to create a new API to pass in the MetaClass. This issue is only about "inheriting" the metaclass of the bases correctly. Currently, Python fails to take into account that the bases may be MetaClass and not
PyType_Typeinstances.I haven't forgotten this issue, but when I get to it it always leads to a rabbit hole. Sometimes just chasing refleaks, but there are deeper issues as well.
AFAICS, there's no way to call metatype.tp_new for such a class. I guess the safest option is to fail when the metaclass has a custom tp_new -- because that means the metaclass is requesting that it wants to allocate/initialize its types itself.
We can call metatype.tp_init, and probably should. And __init_subclass__ too, I suppose.
But at that point, this is duplicating a lot of existing functionality, and I'm starting to wonder if this wouldn't all be better with calling the metaclass instead.
What's missing?- basicsize/itemsize could be allowed with __basicsize__/__itemsize__ in the dict.
- flags: we could add mechanisms to set individual flags after the type is created, as needed.
- slots can usually be applied after the class is created; maybe there should be a public function for this.
- members could theoretically be copied to individual descriptors; there doesn't seem much need for keeping tp_members around.
Does that seem like a more reasonable direction to explore?
seberg commented
on Dec 16, 2021 sebergmannequinMannequinAuthorSorry, I need some time to dive back into this, so some things might be garbled :). Yes, I do agree supporting a custom
tp_newhere seems incredibly tricky. I have not thought about the implications of this, though.guess the safest option is to fail when the metaclass has a custom tp_new
That would seem OK, but I can't quite judge it. It may mean that I have to do a custom factory to create new metaclass instances from Python, but that is probably for the better anyway.
Now, calling
tp_newis a bit useless, since from C we don't have a dict anyway (at least not really). So yeah, this does not make sense at all for most Python defined metaclasses... (they may want to inspect/mutate the dict)But at that point, this is duplicating a lot of existing functionality, and I'm starting to wonder if this wouldn't all be better with calling the metaclass instead.
I do not think I am following :(. My worry is that I want people to create a MetaClass instance through C (but not be locked into static types forever).
My current thought is that I would like it be possible to do something like:
/* Create a new type object */ type_spec = {stuff}; newtype = PyType_FromSpec(&type_spec); /* Finalize the MetaClass */ metatype_spec = {more_stuff}; PyArray_InitDTypeFromSpec(newtype, &metatype_spec);
Of course I can move this into a single function and create the class for the user. But I am uncertain that piping everything through
tp_newwill help? At some point I thought that the user should create a subclass, and I create another class inheriting it. But it also seems convoluted and complicated.I have no idea right now, but maybe there could also be a way to make creating a metaclass-factory in C easier, rather than supporting
PyType_FromSpecfor metaclasses.
(I.e. anPyType_InitFromSpec()doing most of whatPyType_FromSpecdoes, but really meant to be only used be such metaclass factories.)- basicsize/itemsize could be allowed with __basicsize__/__itemsize__ in the dict.
Do we need this? I need the basicsize of the metaclass, but that of the class seems fixed/OK?
- flags: we could add mechanisms to set individual flags after the type is created, as needed.
Seems fine, yeah.
- slots can usually be applied after the class is created; maybe there should be a public function for this.
- members could theoretically be copied to individual descriptors; there doesn't seem much need for keeping tp_members around.
But a Python MetaClass (that the author may not even realize about) might need access to these to work properly?
A bit far fetched, but...seberg commented
on Dec 16, 2021 sebergmannequinMannequinAuthorIt is probably early, but I am starting to like the idea of a "C MetaClass factory" helper/indicator.
It seems to me that yes, at least
tp_newcannot be called reasonable for a class that is created in C, it is just too confusing/awkward to try to push the C stuff through the Python API. (And the Python API is the typical one that should not be inconvenienced)Which gives two possibilities if I want this capability?:
- Force use of a custom class factory in Python (i.e. not through
__new__). But that seems hard, since I need to refuse__new__()!). - Use a class factor in C which never calls
__new__and knows that this is OK.
This is not my turf, so I like unholy, but maybe pragmatic things :). Would a slot/flag indicating
Py_using_metaclass_cinit_pretty_promise_please"solve" these issues?I mean, we have two ways to create classes (C and Python), I am not sure it is plausible to untangle this on the MetaClass level, so maybe the only way forward is to embrace it: Some Python MetaClasses just can't be instantiated from C, because we don't know it will work. If we want to allow instantiating the MetaClass from C, we need some way to set this up. And either we cannot use
PyType_FromSpecthen, or we need to tell it that we know what we are doing.- Force use of a custom class factory in Python (i.e. not through
I don't see how instantiating a metaclass with non-default tp_new would work if you don't know some details about the specific metaclass. So IMO we can we limit ourselves to scenarios where either:
- the metaclass uses default tp_new, or
- the code that creates the class knows about the metaclass
For case 2), we could leave allocation to the calling code, and add an API function that does the rest of the work of applying the spec. Something like:
/* Create a metaclass */ metatype_spec = {stuff}; metatype = PyType_FromSpecAndBases(&metatype_spec, &PyType_Type); /* Create a type */ type_spec = {other_stuff}; newtype = alloc_metatype(); PyType_ApplySpec(newtype, &type_spec);
PyType_ApplySpec would assert PyType_Ready wasn't called before, and call it after filling in the name, slots, etc.
The metatype could disable its tp_new to disallow PyType_FromSpec (and Python __new__), effectively enforcing "using_metaclass_cinit_pretty_promise_please".There could be a slot for code to run at the end of PyType_ApplySpec -- the "PyArray_InitDTypeFromSpec" in your pseudocode.
That seems better than calling the metaclass, but to answer questions on that idea:
> - basicsize/itemsize could be allowed with __basicsize__/__itemsize__ in the dict.
Do we need this? I need the basicsize of the metaclass, but that of the class seems fixed/OK?That's the size of the instance. One more level down :)
> - members could theoretically be copied to individual descriptors; there doesn't seem much need for keeping tp_members around.
But a Python MetaClass (that the author may not even realize about) might need access to these to work properly?
A bit far fetched, but...Seems very far-fetched to me. IMO these, like e.g. tp_methods, should only be used as arguments for the code that puts the descriptors in __dict__. How the descriptors got there is a detail, the class doesn't need to use tp_members (nor advertise that it does).
seberg commented
on Dec 17, 2021 sebergmannequinMannequinAuthorFully, agree! In the end,
PyType_FromSpecreplacestype.__new__()(and init I guess) when working in C. In Python, we would calltype.__new__(maybe via super) from themetatype.__new__, but right now, in C, the metatype cannot reliably usePyType_FromSpecin its ownmetatype.__new__to do the same.I agree with the scenarios:
- If we do not have a custom
metatype.__new__(init?) thenPyType_FromSpecshould have no reason to refuse doing the work, because nothing can go wrong. - If we do have a custom
tp_newthe user has to provide C API to create the metaclass instance. But they still need a way to calltype.__new__in C (i.e. get whatPyType_FromSpecdoes, and promising to do the rest).
PyType_ApplySpecwould provide that way to create a custommetatype.__new__in C whenPyType_FromSpec()would otherwise reject it to make the first scenario safe.
A flag probably can do the same. I have no preference,ApplySpecseems great to me.- If we do not have a custom
Nice! It's starting to look reasonable, I'll try an implementation when I get some focus time. (Sadly I can't promise it'll be this year.)
Just one detail:
A flag probably can do the same. I have no preference,
ApplySpecseems great to me.I didn't get what you mean here.
seberg commented
on Dec 17, 2021 sebergmannequinMannequinAuthorWell, what we need is a way to say: I am calling
type.__new__(i.e. PyType_FromSpec) on purpose from (effectively) my ownmytype.__new__?That is, because right now I assume we want to protect users from calling PyType_FromSpec thinking that it is equivalent to calling
class new(base)when it may not be if base is a metaclass. So callingPyType_FromSpecmight refuse to work if it finds a custommetaclass.__new__(init?).I don't really see that it matters if we only support effectively this from C:
class MyMetaClass(type): def __new__(cls, *args, **kwargs): self = type.__new__(...) # this is PyType_FromSpec # more stuffSo, I thought telling
PyType_FromSpecthat we are "inside" a custom__new__is sufficient and that even as a flag passed as part of the spec could be enough.
But... I agree that I do not quite see that it would be pretty, so it probably was a bad idea :).Plus, if you add a new method it should also solves the issue of setting the
tp_typeslot to the metaclass explicitly when it is not implicit by inheritance (which is the only thing I care about).
(PyType_FromSpec and PyType_ApplySpec will still need to do the work of resolving the metaclass from the base classes, though.)seberg commented
on Dec 17, 2021 sebergmannequinMannequinAuthorBtw. huge thanks for looking into this! Let me know if I can try to help out (I can make due with static metatypes, but my story seems much clearer if I could say: Well with Py 3.11 you can, and probably should, do it dynamically.). I had lost a lot of time chasing "metaclass should just work" followed by "but I can't get it right without bad hacks".
And now the solution seems fairly clear, which is amazing :)!
After
PyType_From*functions are enhanced in #93012 and #28748, I'd like to take a moment to clean it up:- Minimize calling Python code and raising exceptions between
tp_allocandPyType_Ready - Make the refcounting more regular
- Check that base & offsets fit in the basicsize
Some of this is already entangled in #28748, but still, a separate cleanup PR is best practice.
Reacted by Erlend E. Aasland- Minimize calling Python code and raising exceptions between
Thanks for fixing up the PR and pushing this through Petr! Sounds like this can be closed then :).
- added a commit that references this issue
on Jun 14, 2022 - added a commit that references this issue
on Jun 26, 2022
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:
bugs.python.org fields: