[WIP] bpo-35059: Add assertion to _PyObject_CAST() #10649
Conversation
|
This change introduces two backward incompatible changes:
See my commit b37672d for other examples of regressions. So I don't think that this change should be merged. |
Include/object.h
Outdated
| const char *func(PyObject *item) { return Py_TYPE(&item)->tp_name; } | ||
| */ | ||
| #define _PyObject_CAST(op) \ | ||
| ((PyObject*)(op) + Py_BUILD_ASSERT_EXPR(sizeof(*op) != sizeof(op))) |
There was a problem hiding this comment.
if op is an expression, then your dereferencing is likely to fail, better use *(op).
There was a problem hiding this comment.
And you don't have the issue of op being evaluated three times because sizeof does not evaluate the expression \o/
_PyObject_CAST() and _PyVarObject_CAST() macros now require that the type of the argument has a size. For example, _PyObject_CAST(frame) now raises a compilation error if frame type is "struct _frame*" but "struct _frame" is not defined. _PyObject_CAST() and _PyVarObject_CAST() macros now raise a compilation error if the argument is a "PyObject" or a "PyObject**". Partially revert b37672d to show that _PyObject_CAST() accepts non-trivial expressions.
IMHO it's a blocker issue, it's a backward incompatible change. At least, _PyObject_CAST() doesn't require 'op' to have a size of Py_LIMITED_API is defined. |
|
cc @nascheme: you might like this experiment :-) |
|
@serhiy-storchaka, @pablogsal: I experimented a C macro to detect "Py_TYPE(&obj)" bug (with "PyObject *obj"), but it doesn't seem possible to write a magic macro because the Python C API is a mess :-) For example, _PyObject_CAST() should also accept PyTupleObject* which contains a "PyObject ob_base". But there are other type monsters like:
|
|
I did not know about |
I added it yesterday :-) commit 2ff8fb7 I wrote it for C macros to avoid "(PyObject*)ob" mistake: missing parenthesis around "ob". By the way, I should fix Include/odictobject.h in Python 3.6 and 3.7. I have to confess that I also wrote the macro to be able to make such experiment. More generally, it's a hint on cast between object pointer types which helps me on my work on the C API. |
|
Ok, this PR was fun to write, but since it's breaking the backward compatibility, it's really a no-go and so I close it. |
_PyObject_CAST() and _PyVarObject_CAST() macros now require that the
type of the argument has a size. For example, _PyObject_CAST(frame)
now raises a compilation error if frame type is "struct _frame*" but
"struct _frame" is not defined.
_PyObject_CAST() and _PyVarObject_CAST() macros now raise a
compilation error if the argument is a "PyObject" or a "PyObject**".
Partially revert b37672d to show
that _PyObject_CAST() accepts non-trivial expressions.
https://bugs.python.org/issue35059