Hi all,
and thank you to the authors of this well thought-out PEP.
https://www.python.org/dev/peps/pep-0573/
I read through the latest version and found it in a very good and
convincing shape. It has been resting for more than a year now, and some
things happened along the way, most notably PEP 590, to which I think it
should be adapted. Here are my comments.
I find it a bit sad that state lookups in slot methods are disadvantaged
compared to normal methods, given that their entire point is to provide a
performance _benefit_. However, for the time being, we do not know how
relevant and visible this will become in practice, so it is good that the
PEP does not propose a new/real/complex solution to it for now. Instead, it
resorts to the MRO lookup (which can always be done), and leaves any larger
change open for the future, when we know better if there really is a
problem to solve. Check from my side, this is the right trade-off.
I like the idea of immutable exception types.
I'm not entirely happy with the word "Prepare" vs. "New" in the function
name "PyErr_PrepareImmutableException", which is described to be similar to
"PyErr_NewExceptionWithDoc". I understand that it does not create a new
type everytime you call it, but then, it also only prepares something the
first time you call it. Can't say if there is a good way to express this
behaviour, but "prepare" seems worse to me than "new" or "create" IMHO, not
better.
Section [1] is the one on extension type methods, which IMHO should be
adapted to PEP 590. I think it should be based on the vectorcall/fastcall
calling conventions and not take tuple and dict but C arrays as arguments.
Alternatively, we could have two new signatures (old-style + new-style),
but that feels like overkill. the fastcall signatures are easy enough to
handle in C code, and in most cases easier than tuple+dict. The signature
of PyCMethod should become
(PyObject *obj, PyTypeObject *defining_class,
PyObject *const *args, size_t nargs,
PyObject *keywords)
The descriptor special casing [2] in PEP 590 is also worth mentioning in
the context of this PEP. As far as I can see it, the C method type should
implement the vectorcall protocol, and then pass the call through to the
underlying C function, adding the "defining_class" parameter along the way.
I've written up a PR [3] to change the PEP in that direction.
The "_PyMethodDef_RawFastCall*" functions may no longer need to be modified
after this, I think.
BTW, if there is "a new type PyCMethodObject", then there is probably also
a PyTypeObject for it, right? That should be mentioned somewhere.
The section on static/immutable exceptions [4] says "On deinitialization of
the exception type, *exc will be set to NULL". What sets them to NULL?
Under "Helpers", it says "PyType_GetModuleState … returns pointer to state
of the module". I assume that this refers to the PEP-3121 module state, but
an explicit reference would make it clearer.
It also says in the same section that "When a type without a module is
passed in, SystemError is set and NULL returned". Is that necessarily an
exception case? Why not simply return NULL and let the user deal with it? I
think the question is: is there a use case for getting the module state of
an arbitrary type? If so, passing a type that does not have a module
reference is fine and should not raise an exception. If that's not a use
case, then it _might_ be worth informing the user about their misuse, or
not. I don't know. What was the intention here?
Finally, the reference link to the patch set of the implementation does not
work for me. Maybe it should be this?
https://github.com/Dormouse759/cpython/compare/master...Dormouse759:pep-c
As you can see, I do not have any objections or major concerns about this
PEP. Next, I'll look through the implementation, and would also like to see
at least an attempt to implement it in Cython, to get an idea about how its
usage would look in the general case. But apart from that, I can see that a
lot of thinking has already gone into this PEP.
Stefan
[1]
https://www.python.org/dev/peps/pep-0573/#passing-the-defining-class-to-ext…
[2] https://www.python.org/dev/peps/pep-0590/#descriptor-behavior
[3] https://github.com/python/peps/pull/1159
[4] https://www.python.org/dev/peps/pep-0573/#static-exceptions
