bpo-46417: Add _PyType_CAST() macro - #30760
Conversation
Sorry, something went wrong.
In debug mode, the macro makes sure that its argument is a type using an assertion.
erlend-aasland
left a comment
There was a problem hiding this comment.
I see you left some ordinary type casts. Was that deliberate, or did you miss some _PyType_CASTs?
Sorry, something went wrong.
| type->tp_name, Py_TYPE(ob)->tp_name); | ||
| return -1; | ||
| } | ||
| PyTypeObject *base = (PyTypeObject*)ob; |
There was a problem hiding this comment.
Use _PyType_CAST here as well?
Sorry, something went wrong.
There was a problem hiding this comment.
We just checked PyType_Check() at runtime (even if assertions are removed by the compiler), using the macro is redundant (it would call PyType_Check() twice in debug mode).
Sorry, something went wrong.
| Py_TYPE(obj)->tp_name); | ||
| return -1; | ||
| } | ||
| PyTypeObject *base = (PyTypeObject*)obj; |
There was a problem hiding this comment.
Ditto
Sorry, something went wrong.
| return NULL; | ||
| } | ||
| base_i = (PyTypeObject *)base_proto; | ||
| PyTypeObject *base_i = (PyTypeObject *)base_proto; |
There was a problem hiding this comment.
Ditto
Sorry, something went wrong.
| Py_TYPE(value)->tp_name); | ||
| return -1; | ||
| } | ||
| PyTypeObject *newto = (PyTypeObject *)value; |
There was a problem hiding this comment.
Ditto
Sorry, something went wrong.
| return NULL; | ||
| } | ||
| type = (PyTypeObject *)self; | ||
| PyTypeObject *type = (PyTypeObject *)self; |
There was a problem hiding this comment.
Ditto
Sorry, something went wrong.
| return NULL; | ||
| } | ||
| subtype = (PyTypeObject *)arg0; | ||
| PyTypeObject *subtype = (PyTypeObject *)arg0; |
There was a problem hiding this comment.
Ditto
Sorry, something went wrong.
| if ((PyObject *)subclass == Py_None) | ||
| PyObject *obj = PyWeakref_GET_OBJECT(ref); | ||
| assert(obj != NULL); | ||
| if (obj == Py_None) { |
There was a problem hiding this comment.
| if (obj == Py_None) { | |
| if (Py_IsNone(obj)) { |
Sorry, something went wrong.
There was a problem hiding this comment.
Trust me, the temptation to refactor everything in this old giant C file is very high :-D But then it leads to PR which is impossible to review :-( I prefer to leave this test unchanged.
Sorry, something went wrong.
I left unchanged the raw type casts which are just after a PyType_Check() test. Locally, I have a local Git branch to cleanup way more casts: Modules/_csv.c | 20 ++++++++++---------- Modules/_datetimemodule.c | 8 ++++---- Modules/_pickle.c | 20 +++++++++++--------- Modules/_randommodule.c | 11 ++++++----- Modules/_sqlite/connection.c | 5 +++-- Modules/_sqlite/cursor.c | 5 +++-- Modules/_sqlite/prepare_protocol.c | 5 +++-- Modules/_sqlite/row.c | 5 +++-- Modules/_sqlite/statement.c | 5 +++-- Modules/_testcapimodule.c | 12 ++++++------ Modules/_zoneinfo.c | 3 +-- Objects/complexobject.c | 2 +- Objects/dictobject.c | 3 +-- Objects/enumobject.c | 7 ++----- Objects/exceptions.c | 3 +-- Objects/floatobject.c | 2 +- Objects/listobject.c | 3 +-- Objects/setobject.c | 8 ++++---- Objects/structseq.c | 3 +-- Objects/tupleobject.c | 2 +- Objects/typeobject.c | 65 ++++++++++++++++++++++++++++++----------------------------------- Python/bltinmodule.c | 4 ++-- Python/ceval.c | 22 +++++++++++----------- Python/specialize.c | 13 ++++++------- 24 files changed, 115 insertions(+), 121 deletions(-) I prefer to create a separated PR for these ones :-) |
Sorry, something went wrong.
Ok, I assumed you had a good reason for it :) |
Sorry, something went wrong.
In debug mode, the macro makes sure that its argument is a type using
an assertion.
https://bugs.python.org/issue46417