Message324303
I was also talking about static C types, it is possible to write types with custom meta types in C code (even if that is even less common than using meta types in the first place). I have extensions where I use PyVarObject_HEAD_INIT without the first argument being &PyType_Type or NULL and this patch would break those.
This part of your patch silently changes the API for CPython in a way that is confusing. With your patch this code changes behaviour:
static PyTypeObject my_type = {
PyVarObject_HEAD_INIT(&my_meta_type, 0),
...
};
This is currently valid code and I use this pattern in my code. It is easy enough to adjust for your patch (just add "Py_TYPE(&my_type) = &my_meta_type;" before the call to PyType_Ready), but that is just a change for changes sake.
Why not just change all invocations of PyVarObject_HEAD_INIT in the CPython tree, if any?
I do agree that explicitly calling PyType_Ready for all types would be better, but that is a separate issue from changing the way the static PyTypeObject is initialised.
A patch that does add calls to PyType_Ready should also ensure that this is done before types are actually used (otherwise there is a nonzero chance that the explicit calls don't actually do anything due to the calls to PyType_Ready in strategic places in the interpreter).
Note that your patch tries to accomplish two things:
1) PyType_Ready should be called to initialize types
2) Explicitly setting the type of PyTypeObject instances through PyVarObject_HEAD_INIT is not necessary in the common case.
I agree with the first item, and am -1 on a change for the second item, but -1 on the current patch for this. |
|
Date |
User |
Action |
Args |
2018-08-29 07:13:28 | ronaldoussoren | set | recipients:
+ ronaldoussoren, twouters, benjamin.peterson, eelizondo |
2018-08-29 07:13:28 | ronaldoussoren | set | messageid: <1535526808.31.0.56676864532.issue34522@psf.upfronthosting.co.za> |
2018-08-29 07:13:28 | ronaldoussoren | link | issue34522 messages |
2018-08-29 07:13:27 | ronaldoussoren | create | |
|