This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: PyType_FromSpec API fails to use metaclass of bases
Type: crash Stage:
Components: C API, Interpreter Core Versions: Python 3.11
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: petr.viktorin, seberg
Priority: normal Keywords:

Created on 2021-10-05 21:51 by seberg, last changed 2022-04-11 14:59 by admin.

Pull Requests
URL Status Linked Edit
PR 28748 open seberg, 2021-10-05 21:51
Messages (9)
msg403273 - (view) Author: Sebastian Berg (seberg) * Date: 2021-10-05 21:51
The PyType_FromSpec fails to take care about MetaClasses.

     https://bugs.python.org/issue15870

Asks 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_Type` instances.
msg408532 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-12-14 14:43
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?
msg408720 - (view) Author: Sebastian Berg (seberg) * Date: 2021-12-16 16:13
Sorry, I need some time to dive back into this, so some things might be garbled :).  Yes, I do agree supporting a custom `tp_new` here 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_new` is 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_new` will 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_FromSpec` for metaclasses.
(I.e. an `PType_InitFromSpec()` doing most of what `PyType_FromSpec` does, 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...
msg408723 - (view) Author: Sebastian Berg (seberg) * Date: 2021-12-16 17:20
It 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_new` cannot 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?:
1. Force use of a custom class factory in Python (i.e. not through `__new__`).  But that seems hard, since I need to refuse `__new__()`!).
2. 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_FromSpec` then, or we need to tell it that we know what we are doing.
msg408790 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-12-17 15:18
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:
1) the metaclass uses default tp_new, or
2) 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).
msg408791 - (view) Author: Sebastian Berg (seberg) * Date: 2021-12-17 15:37
Fully, agree!  In the end, `PyType_FromSpec` replaces `type.__new__()` (and init I guess) when working in C.  In Python, we would call `type.__new__` (maybe via super) from the `metatype.__new__`, but right now, in C, the metatype cannot reliably use `PyType_FromSpec` in its own `metatype.__new__` to do the same.

I agree with the scenarios:
* If we do not have a custom `metatype.__new__` (init?) then `PyType_FromSpec` should have no reason to refuse doing the work, because nothing can go wrong.
* If we do have a custom `tp_new` the user has to provide C API to create the metaclass instance.  But they still need a way to call `type.__new__` in C (i.e. get what `PyType_FromSpec` does, and promising to do the rest).

`PyType_ApplySpec` would provide that way to create a custom `metatype.__new__` in C when `PyType_FromSpec()` would otherwise reject it to make the first scenario safe.
A flag probably can do the same.  I have no preference, `ApplySpec` seems great to me.
msg408793 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-12-17 15:53
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, `ApplySpec` seems great to me.

I didn't get what you mean here.
msg408797 - (view) Author: Sebastian Berg (seberg) * Date: 2021-12-17 16:34
Well, what we need is a way to say: I am calling `type.__new__` (i.e. PyType_FromSpec) on purpose from (effectively) my own `mytype.__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 calling `PyType_FromSpec` might refuse to work if it finds a custom `metaclass.__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 stuff
```
So, I thought telling `PyType_FromSpec` that 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_type` slot 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.)
msg408801 - (view) Author: Sebastian Berg (seberg) * Date: 2021-12-17 17:19
Btw. 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 :)!
History
Date User Action Args
2022-04-11 14:59:50adminsetgithub: 89546
2021-12-17 17:19:49sebergsetmessages: + msg408801
2021-12-17 16:34:28sebergsetmessages: + msg408797
2021-12-17 15:53:28petr.viktorinsetmessages: + msg408793
2021-12-17 15:37:58sebergsetmessages: + msg408791
2021-12-17 15:18:40petr.viktorinsetmessages: + msg408790
2021-12-16 17:20:02sebergsetmessages: + msg408723
2021-12-16 16:13:28sebergsetmessages: + msg408720
2021-12-14 14:43:01petr.viktorinsetmessages: + msg408532
2021-10-05 21:51:26sebergcreate